bitfasching / AS5601

Library for driving the sensor AS5601 over I²C on an Arduino
Other
16 stars 6 forks source link

Misreads #5

Open arpruss opened 1 year ago

arpruss commented 1 year ago

This is a great library, but I'm having trouble with some misreads. I rotate the magnet (it's set in a skate bearing, so I can do it smoothly) clockwise and read getAngle(), and normally the data is increasing, but every so often I get one piece of data that seems to be behind.

My current workaround is to read three angles in sequence, and then return one of the pair that are closest together, but that's really kludgy (plus a slowdown).

I am using 4.7K pull-up resistors on SDA and SCL, and using a 3.3v board (stm32f103c8t6 blue pill clone).

Would you have any suggestions?

Here's my filtering code, which pretty much solves the problem, but introduces some latency.

        unsigned int distance(unsigned a, unsigned b) {
          int delta = a-b;
          if (delta & 0x800) {
            delta |= ~0xFFF;
          }
          if (delta < 0)
            return -delta;
          else
            return delta;
        }

        unsigned int getAngleFiltered()
        {            
            unsigned int angle1 = this->readRaw16( AS5601::WordRegister::ANGLE );
            unsigned int angle2 = this->readRaw16( AS5601::WordRegister::ANGLE );
            unsigned int angle3 = this->readRaw16( AS5601::WordRegister::ANGLE );
            unsigned d12 = distance(angle1,angle2);
            unsigned d23 = distance(angle2,angle3);
            unsigned d13 = distance(angle1,angle3);
            if (d13 < d12)
              return angle3;
            if (d12 < d23)
              return angle2;
            return angle3;
        }
wilba commented 1 year ago

I've been experimenting with this chip (not using this code, though), and encountered the same problem, I think.

I think the ANGLE registers can change between the read of the high and low byte, which is a big problem if both change, e.g. between 0x00FF and 0x0100... the read might result in 0x01FF or 0x0000.

My workaround is reading high byte twice, before and after low byte, and then when using the 1st read of high byte, detecting an absolute difference of around 255, and then using the 2nd read of high byte instead. Seems to work.

Here's my filtering code, FWIW it's running on an ATMEGA32U4, compiled C, QMK project:

int16_t as5601_get_angle(void) {
    // The angle registers can change between reading the low and high byte.
    // If this happens while both bytes are changing, the only way
    // to tell is by comparing the new value with the last value,
    // and observe an absolute difference of around 255.
    static int16_t last_angle = -1;
    uint8_t        angle_hi1  = 0;
    uint8_t        angle_hi2  = 0;
    uint8_t        angle_lo   = 0;

    bool s1 = as5601_read_register(AS5601_REG_ANGLE_HI, &angle_hi1);
    bool s2 = as5601_read_register(AS5601_REG_ANGLE_LO, &angle_lo);
    bool s3 = as5601_read_register(AS5601_REG_ANGLE_HI, &angle_hi2);

    if (!s1 || !s2 || !s3) {
        return -1;
    }

    int16_t angle1 = (angle_hi1 << 8) | angle_lo;
    int16_t angle2 = (angle_hi2 << 8) | angle_lo;

    // The first known good angle will be when the high byte didn't
    // change between two reads of it.
    if (last_angle == -1) {
        if (angle1 == angle2) {
            last_angle = angle1;
            return angle1;
        }
        return -1;
    }

    int16_t delta_abs1 = abs(angle1 - last_angle);
    int16_t delta_abs2 = abs(angle2 - last_angle);
    if (delta_abs1 > 127 && delta_abs1 < 511) {
        if (delta_abs2 > 127 && delta_abs2 < 511) {
            return -1;
        } else {
            last_angle = angle2;
            return angle2;
        }
    }

    last_angle = angle1;
    return angle1;
}
arpruss commented 1 year ago

Very nice diagnosis of the problem!

And the solution is nice, in that it requires only half of the reads that mine does. But this solution only work for continuous reading, since it depends on past data.

Looking at the datasheet, the AS5601 supports sequential reading. I wonder if the rollover problem would occur if we requested a two-byte read instead of two one-byte reads.

wilba commented 1 year ago

The reason I was reading each byte separately was because the datasheet says automatic increment of the address pointer is suppressed specifically for the ANGLE high byte register.

Automatic increment of the address pointer for ANGLE, RAW ANGLE, and MAGNITUDE registers:

These are special registers which suppress the automatic increment of the address pointer on reads, so a re-read of these registers requires no I²C write command to reload the address pointer. This special treatment of the pointer is effective only if the address pointer is set to the high byte of the register.

But your suggestion make me realize it only applies to the high byte register.

I just tested doing a 3-byte sequential read from the register before the high byte register, and it appears to be stable and not needing the workaround. Maybe the registers are stable during a single I2C transaction.

If you want to try the same thing, you'll need to change readRaw16() to not do two calls to readRaw8() as this is not a real I2C sequential read.

arpruss commented 1 year ago

A two-byte sequential read of RAWANGLE works just fine for me with an STM32F103C8T6 clone and a bitbanged I2C library (SoftWire.h). No need to decrement the address and read three bytes.

Oddly, I get the device hanging on the requestFrom() call if I use the hardware-based I2C library while to read more than one byte at a time. I have no idea why. (The hardware-based I2C library works with the AS5601 for single-byte reads, and it also works with my Wii Nunchuck for six-byte reads.) It could be that this is a bug in the STM clone (I don't have another AS5601 on a breakout board right now to test with a genuine STM chip).

I don't really know how I2C works below the Arduino abstraction, but there seems to be a difference between the auto-increment and the use of requestFrom() with more than one byte requested. In fact, I am seeing convenient no-auto-increment behavior in the following context. After first setting up the reading of the angle register via wireChannel->beginTransmission( this->address ); wireChannel->write( registerAddress ); wireChannel->endTransmission( false ); I can just call wireChannel->requestFrom( (uint8_t) this->address, (uint8_t) 2, (uint8_t) true ) as many times as I like to request more and more words from the angle register.

wilba commented 1 year ago

I changed it to reading two bytes in a single read (send address, read 2 bytes) from the ANGLE high register and it works.

I think I just misinterpreted the datasheet... "reading multiple bytes" is a different kind of incrementation of the address. The datasheet is saying if you send the ANGLE high byte address once, then you can do repeated 2 byte reads and the address is not auto-incremented and thus doesn't need to be sent again.

arpruss commented 1 year ago

I asked AMS about this. Here's what they said:

The ANGLE, RAWANGLE and MAGNITUDE registers are ‘hold’ registers.

According to the digital designer of the AS5600/5601, the sequence of reading those registers is of importance – Read High byte first, Low byte second --

Whenever you read the High Byte, the Low Byte will be on hold/frozen in order to guarantee a consistent dataset.

There is no danger to get a mix of old and new data during one sequential read if you follow this sequence.

I have asked the team to add this detail to the datasheet – Thank you for highlighting the need for it. This functionality has always been native to these Output Registers on these products (Rotary Magnetic Position Sensors). It was, unfortunately, not explicitly noted it in the documentation for newer users’ reference.

wilba commented 1 year ago

Oh that's great. Thanks for your help.

bitfasching commented 1 year ago

Hey guys,

very interesting discussion!

Whenever you read the High Byte, the Low Byte will be on hold/frozen in order to guarantee a consistent dataset. There is no danger to get a mix of old and new data during one sequential read if you follow this sequence.

Alright – since this is what the library currently does, I guess there is no need for changes then?

Could you find the cause of your inconsistent readings meanwhile?

I just wanted to add that I've never noticed such problem back then when using an Arduino Nano. And we queried these sensors a lot in our haptic paddle controllers for Pong. ;)

Cheers, Nick

arpruss commented 1 year ago

As I understand it, to get this feature, you need to issue a two-byte read command to the I2C controller, rather than just a sequence of two one-byte reads.

Yes, I was getting bad data with this library (with minor patches to get it working on my stm32f1) when spinning fast (faster than one would in playing pong).

wilba commented 1 year ago

Inconsistent readings could happen even if spinning slowly, i.e. any time both bytes change, such as 0x00FF to 0x0100, two separate one-byte reads might result in 0x0000 or 0x01FF. It has to be a two-byte read, i.e. send address, read byte, read byte.

arpruss commented 1 year ago

AMS also told me this about the hold feature for the reads: "Yes, it does require 2-byte read sequence (unitized) vs. having a STOP bit in-between reads… as you note. I’ll drive the message home with the associate who is to adjust the datasheet."