Access to read / write calibration offsets

[caternuson]

There are two "calibration" related registers:

• Status - read only, via the CALIB_STAT register at 0x35
• Offsets - read and write, via the 22 registers 0x55 - 0x6A

Access to the status related one was implemented in #15 as a fix for #11. That issue was perhaps intended to address the offsets, but might have been confused for simply the status.

Access to the offset related ones are still not implemented. For ref, here the related funcs from the older library:
https://github.com/adafruit/Adafruit_Python_BNO055/blob/master/Adafruit_BNO055/BNO055.py#L527
https://github.com/adafruit/Adafruit_Python_BNO055/blob/master/Adafruit_BNO055/BNO055.py#L542

[JNatael]

FYI, I needed this functionality so I went ahead and ported pieces of the old code to accomplish this in the short term. Code is here in case it's useful to anyone, and if anyone finds bugs please let me know. Of note, a few places have hardcoded variables or refer to parent class variables; those would need to get updated for any implementation that doesn't have this as a child class.

Sorry for the weird pasting below, github seems to be segmenting out pieces to count as code and others not. Everything below this paragraph is one file though, including the extension and testing at the end, and if anyone wants a cleaner version of it you can find it at https://forums.adafruit.com/viewtopic.php?f=60&t=146334&p=723726#p723726

import adafruit_bno055
"""This is a class extender of the bno055 adafruit class. Most of the functionality remains in the parent class,
but functionality here has been ported from an old driver in order to set and get calibration values from the device."""

class BNO055_extended(adafruit_bno055.BNO055):
    def get_calibration(self):
        """Return the sensor's calibration data and return it as an array of
        22 bytes. Can be saved and then reloaded with the set_calibration function
        to quickly calibrate from a previously calculated set of calibration data.
        """
        # Switch to configuration mode, as mentioned in section 3.10.4 of datasheet.
        self.mode = adafruit_bno055.CONFIG_MODE
        # Read the 22 bytes of calibration data and convert it to a list (from
        # a bytearray) so it's more easily serialized should the caller want to
        # store it.
        cal_data = list(self._read_registers(0X55, 22))
        # Go back to normal operation mode.
        self.mode = adafruit_bno055.NDOF_MODE
        return cal_data

    def set_calibration(self, data):
        """Set the sensor's calibration data using a list of 22 bytes that
        represent the sensor offsets and calibration data.  This data should be
        a value that was previously retrieved with get_calibration (and then
        perhaps persisted to disk or other location until needed again).
        """        
        # Check that 22 bytes were passed in with calibration data.
        if data is None or len(data) != 22:
            raise ValueError('Expected a list of 22 bytes for calibration data.')
        # Switch to configuration mode, as mentioned in section 3.10.4 of datasheet.
        self.mode = adafruit_bno055.CONFIG_MODE
        # Set the 22 bytes of calibration data.
        self._write_register_data(0X55, data)
        # Go back to normal operation mode.
        self.mode = adafruit_bno055.NDOF_MODE

    def _write_register_data(self, register, data):
        write_buffer = bytearray(1)
        write_buffer[0] = register & 0xFF
        write_buffer[1:len(data)+1]=data
        with self.i2c_device as i2c:
            i2c.write(write_buffer, start=0, end=len(write_buffer))

    def _read_registers(self, register, length):
        read_buffer = bytearray(23)
        read_buffer[0] = register & 0xFF
        with self.i2c_device as i2c:
            i2c.write(read_buffer, start=0, end=1)
            i2c.readinto(read_buffer, start=0, end=length)
            return read_buffer[0:length]

if __name__ == "__main__":
    import time
    import board
    import busio
    # if run directly we'll just create an instance of the class and output the current readings
    i2c = busio.I2C(board.SCL, board.SDA)
    sensor = BNO055_extended(i2c)

    print("Calibration Status",sensor.calibrated)
    print("Calibration:",sensor.get_calibration())

    sensor.set_calibration([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 252, 255, 251, 255, 1, 0, 232, 3, 0, 0])
    print("Calibration Status",sensor.calibrated)
    print("Calibration:",sensor.get_calibration())

    for x in range(0,20):
        print('Temperature: {} degrees C'.format(sensor.temperature))
        print('Accelerometer (m/s^2): {}'.format(sensor.accelerometer))
        print('Magnetometer (microteslas): {}'.format(sensor.magnetometer))
        print('Gyroscope (deg/sec): {}'.format(sensor.gyroscope))
        print('Euler angle: {}'.format(sensor.euler))
        print('Quaternion: {}'.format(sensor.quaternion))
        print('Linear acceleration (m/s^2): {}'.format(sensor.linear_acceleration))
        print('Gravity (m/s^2): {}'.format(sensor.gravity))
        print()

        time.sleep(1)

    print("Calibration Status",sensor.calibrated)
    print("Calibration:",sensor.get_calibration())```

[tannewt]

@JNatael I updated your comment to use three backticks on each side to designate the block code.

Would you like to add a calibration property in the top class that takes the byte string and then make a pull request? @caternuson or I can help you through that process.

[JNatael]

Weird; I even tried that and it didn't seem to work. Regardless, thanks for the fix; looks much better now!

Actually, I assumed that my fix probably didn't conform to the new standards you're porting the libraries to so I didn't think a pull request was entirely appropriate here. Figured the longer term solution would need to use only new approaches and function calls; I posted this more in case any of the above was useful to you all in moving that way and to anyone else who needed to use this functionality in the meantime.

Of course, if you all want to integrate this directly I'd be happy to add the property and go through that process, but would want someone on the team to first ensure that was actually desired here and to verify the code functions correctly (is that part of a pull request?).

[caternuson]

Please do! We either want this in or discuss here why we don't. But I'm pretty sure we do.

Specifics would be part of the pull request process. It would get some automated checks via Travis which all need to pass. Then we would code review the PR. Ideally there would be hardware testing done at this point also.

We can also have preliminary discussion about the approach you want to take here.

• Should there be only one property? There are separate offsets for the accelerometer, magnetometer, and gyroscope. And also "offset" and "radius" items.
• What should the property(s) return? One big bytestring? Or something more user friendly?

I'll put this here for ref:

[JNatael]

Well the core reason I'd assumed you wouldn't want it in was that I reworked the register reading and writing functions. Based on other work you all have done it seemed like part of the point of moving to the circuitpython library was to consolidate a lot more of those calls and/or make them consistent across devices. I did base them on functions from other drivers in the library so they're similar, but not the same.

So I suppose the first part of the discussion about the inclusion should be a discussion about that; what's the goal of moving the library in the first place and what's the plan with the read/write functions for the new approach?

As for the offset point, that makes sense and on reflection it seems to me like it might actually make more sense to split the calibration or have it be more nuanced so that it could be set by device or all together.

And to be honest, while I'm happy to have contributed, my needs were met with the code I wrote above so I'm not sure I have the time to break from my main project to work on generalizing at the moment. :-/ Maybe that'll change in a week or three, but at the moment generalizing that in the way that would likely be best is more effort than I currently have free to put in here. Sorry; I hope at the least having tested code like the above is a useful starting point to you all.

[JNatael]

Hmm, maybe a quick addition, if you all would like the code as is to be integrated I'd be happy to do a pull for that and then you could develop further from there, that shouldn't take long. If you split into multiple properties though then it might make more sense to leave that to the person who develops that capacity to add in. Up to you all; if it would be useful to integrate the above as a starting point let me know.

[JNatael]

And because I apparently can't resist 😏, one more thought. I'd advocate strongly against returning a bytestring; seems to me the whole purpose of such drivers is to abstract away the specifics of the physical device from the usage of that device. Users shouldn't need to understand the details of what's going on at the register level to use it. For that purpose letting the user do the reading and writing with lists of numbers, as I did above, and absorbing any byte conversion into the library itself seems vastly preferable.

That is of course a broader design choice for the whole library though so while I think that's better, it's likely more important to be in sync with the library standard.

[caternuson]

Thanks, but I think we would want it a little different. One of the changes we are moving toward in the newer Python libraries is the use of @property style properties instead of getter/setter functions. It's generally the same things, just with a different syntax, esp. in user code.

Agree - I think moving the entire bytestring around is very clunky and not at all user friendly.

[caternuson]

Some options for discussion sake. @tannewt You're better at this than I am, what'd you think?

Option 1 - a property for each

bno.accelerometer_offset_X
bno.accelerometer_offset_Y
bno.accelerometer_offset_Z
bno.magnetometer_offset_X
bno.magnetometer_offset_Y
bno.magnetometer_offset_Z
bno.gyro_offset_X
bno.gyro_offset_Y
bno.gyro_offset_Z
bno.accelerometer_radius
bno.magnetometer_radius

Option 2 - use 3-tuple for offsets

bno.accelerometer_offsets
bno.magnetometer_offsets
bno.gyro_offsets
bno.accelerometer_radius
bno.magnetometer_radius

Option 3 - a struct like class to hold all this

[tannewt]

I'd probably go for option 2. It's a good balance between verbosity and limiting memory impact.

The Adafruit_CircuitPython_Register library could be useful because the accessors can share code then.

[kattni]

@caternuson Was this resolved by #30? I'm unsure whether it covered your entire initial post or not - it looks like it included part of it, but not all of it.

[caternuson]

@kattni Yah, I think it was. Closing.