drivers icon indicating copy to clipboard operation
drivers copied to clipboard

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

Open soypat opened this issue 4 years ago • 2 comments

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.

soypat avatar Sep 09 '21 02:09 soypat

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.

ysoldak avatar Sep 09 '21 14:09 ysoldak

@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).

soypat avatar Sep 09 '21 20:09 soypat