Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Update method call to read sensor data from a bus #310

Open
soypat opened this issue Sep 9, 2021 · 2 comments
Open

Proposal: Update method call to read sensor data from a bus #310

soypat opened this issue Sep 9, 2021 · 2 comments

Comments

@soypat
Copy link
Contributor

soypat commented Sep 9, 2021

Background

Sensors acquire measurements from their surrounding and store them as bytes in their registers for us to read via a communications bus, such as (but not limited to) i2c and SPI. Reading sensor data using these protocols carries some overhead that should be minimized if possible. This proposal seeks to provide TinyGo with a lean foundation to build sensing libraries upon.

The Update method

The benefits of having an Update method to schedule sensor measurements was discussed in PR #298 and #299.

Put simply, the Update method reads all requested sensor values into memory for later use using Measurement methods. Subsequent calls to update replace measurements.

The goal of this proposal is to clearly separate the I/O from the data. Most ICs work with auto-increment when reading registers which allows reading many sensor values in one transaction.

The benefits are many:

Advantages

  • Better framework for libraries. Without this change if many libraries use the same sensor then they'd all be fighting for the communications bus. On top of that, this bus abuse gets worse if there are many sensors on the same bus. Splitting I/O and data responsibilities lets the user decide if they delegate I/O to the library, if they call it once every loop() or if they call it every couple minutes
  • Error management separate from "business logic". So far we've been creating these sensor handlers which have data only methods, such as an ADC's Get() uint16, an IMU's acceleration and rotation methods. We've been discarding all bus errors doing this and that is objectively not good design. These errors should be handled somewhere in the stack. By using an Update() method the error can be handled explicitly and we can still have a clean API on our sensors.
  • Less overhead of read/write function call and protocol transaction means better performance, more sensor data per unit time
  • Less chance of a failed read or timeout due to interference. The MPU6050 is notorious for this at high request rates.
  • This is how most ICs are meant to be used. i.e: Some ADC's work faster in continuous conversion mode, but this means that the channel you desire might not be the one you get when reading the data register so for loops are used to request data until the desired channel is read, which is not guaranteed to happen in a fixed amount of time.

Disadvantage

  • Peripheral structure consumes data equivalent to total sensor data.
  • A call to Update must be done before calling a measurement function though this could be remedied by embedding the type within a sample rate limiter.

@aykevl Showed an example of what the API could look like

// in the drivers package
type Measurement uint32
const (
    Temperature = 1 << iota
    Humidity
    Rotation
    Acceleration
    MagneticField // not sure about this name
    // ...etc
)

// In a driver
// Update reads the various measurements in one batch, as far as possible.
func (d *Device) Update(which Measurement) error {
    if which & drivers.Temperature != 0 {
        // read temperature
    }
}

// In a user package
device := ...
for {
    err := device.Update(drivers.Acceleration | drivers.Rotation)
    if err != nil { handle }
    ax, ay, az := device.Acceleration()
    rx, ry, rz := device.Rotation()
    time.Sleep(time.Second)
}

TBD

Side effects of Update

It might be worthwhile to decide if Update has side effects or not before writing the implementations.

  1. Update reads sensor data requested. Does not guarantee only the sensors requested are read. When calling Measurement, data is read straight from the buffer
  2. Update reads only sensor data requested.. This probably means you have to create struct field for each sensor data and only save to it the values requested. This takes up more space if also storing the buffer inside the struct. Goes from 24 bytes to 48 or so. A small benefit of this is that there need not be byte shifting and ORing on Measurement call since Update already does that for us.
@ysoldak
Copy link
Contributor

ysoldak commented Sep 9, 2021

I'll read this more carefully later, but my initial thought: would be nice to see a PR demonstrating this in action, i.e. refactor existing (not necessary all, couple is enough) IMU drivers to satisfy common interface.
There can be insights that very easy miss designing top-down.

@soypat
Copy link
Contributor Author

soypat commented Sep 9, 2021

@ysoldak Here's a PR that partially implements this design choice. The approach of this PR was to implement a type embedding the existing peripheral handle and add the new Update, Acceleration, Temperature, and Rotation methods. The PR has been tested and works fine. I've also written drivers for the MPU6050 for OS targets and this approach reduces "failed read" rates drastically for high request rates (4 failures/minute --> 1 failure every 6 minutes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants