micro:bit dal accelerometer LSB handling

03 May 2016

I was noodling around the microbit-dal to see how it worked and noticed there might be some issues with the optional LSB handling. It currently (master/eead320) uses a multiply by 8 on MSB followed by an optional addition of the MSB with a divide by 64 both operating on a signed 8bit integer quantity.

https://github.com/lancaster-university/microbit-dal/blob/master/source/drivers/MicroBitAccelerometer.cpp

The first question is whether the LSB data stored in bits 7 and 6 should be shifted 5 places rather than 6? The multiply by 8 is a 3bit shift so this appears to be leaving an incorrect 1 bit "gap" between MSB and LSB data. Worth checking whether 5:0 bits are guaranteed to be zero, of course.

For the LSB (only) I'm also puzzling over whether this should be processed using signed (presumably twos complement) representation. I think this means it will incorrectly be interpreted as a negative value rather than just trailing bits? Perhaps using shift operations on an unsigned integer would be correct here, e.g. >> 5 and then use &= to fill in lsb.

Do the divides get optimised away to become shift instructions?

I can see some derived code here too where the comments question the cast. I don't see the cast as an issue, just the LSB portion needs more careful treatment wrt bit positioning and sign.

https://github.com/web-bluetooth/juggling/blob/master/firmware/source/Accelerometer.h

(I've only inspected code and not written any tests here to prove this.)

07 Oct 2016

Hi Kevin,

It seems I have come to a similar conclusions to you, I am using a microbit as a 'peripheral' to an Arduino, supplying accelerometer data via SPI, so I wanted the 'raw' accelerometer data, but with the full resolution available.

To see what was going on, I was displaying the raw bits of the values of interest on the microbit's LED matrix, and monitoring the sent value via the Arduino's serial monitor.

The accelerometer MSB is a signed byte, representing bits 15:8 of the 16-bit signed result. The LSB just has the top two bits of the byte active (bits 7:6), and as these represent bits 7:6 of the signed 16-bit result, will be unsigned when considered as a stand-alone byte.

As I just wanted to construct the 10-bit result without any additional scaling, the operation I wanted was (MSB * 4) (== shift left 2 bits), and add in (LSB >> 6), such that bits 7:6 of the LSB are moved to bits 1:0.

Looking at MicroBitAccelerometer.cpp in the microbit-dal, the accelerometer MSB and LSB are read into an array of type int8_t, i.e. signed bytes, and the result e.g. sample.x is of type int16_t. So the MSB multiply by 4 is 'safe' and will be sign extended as required. However to shift the LSB right correctly, I first cast it to a uint8_t unsigned byte, so that the bits 7:2 of the result are zero-filled rather than sign extended.

Monitoring the resultant 16-bit signed result, I can see that the code follows the proper binary sequence when tilting the microbit (gently!), so am fairly happy that I'm getting the correct result.

Here is the resulting modified code that worked for me:

modified MicroBitAccelerometer.cpp snippet

int MicroBitAccelerometer::updateSample()
{
    if(!(status & MICROBIT_ACCEL_ADDED_TO_IDLE))
    {
        fiber_add_idle_component(this);
        status |= MICROBIT_ACCEL_ADDED_TO_IDLE;
    }

    // Poll interrupt line from accelerometer.
    // n.b. Default is Active LO. Interrupt is cleared in data read.
    if(!int1)
    {
        int8_t data[6];
        int result;

        result = readCommand(MMA8653_OUT_X_MSB, (uint8_t *)data, 6);
        if (result !=0)
            return MICROBIT_I2C_ERROR;

        // read MSB values...
        sample.x = data[0];
        sample.y = data[2];
        sample.z = data[4];

        // Normalize the data in the 0..1024 range.
        sample.x *= 4;
        sample.y *= 4;
        sample.z *= 4;

//#if CONFIG_ENABLED(USE_ACCEL_LSB)
        // Add in LSB values.
        sample.x += (uint8_t)data[1] >> 6;
        sample.y += (uint8_t)data[3] >> 6;
        sample.z += (uint8_t)data[5] >> 6;
//#endif

        // Scale into millig (approx!)
//        sample.x *= this->sampleRange;
//        sample.y *= this->sampleRange;
//        sample.z *= this->sampleRange;

        // Indicate that pitch and roll data is now stale, and needs to be recalculated if needed.
        status &= ~MICROBIT_ACCEL_PITCH_ROLL_VALID;

        // Update gesture tracking
        updateGesture();

        // Indicate that a new sample is available
        MicroBitEvent e(id, MICROBIT_ACCELEROMETER_EVT_DATA_UPDATE);
    }

    return MICROBIT_OK;
};