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

Misreads #5

Open
arpruss opened this issue Feb 13, 2023 · 11 comments
Open

Misreads #5

arpruss opened this issue Feb 13, 2023 · 11 comments

Comments

@arpruss
Copy link

arpruss commented Feb 13, 2023

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
Copy link

wilba commented Mar 21, 2023

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
Copy link
Author

arpruss commented Mar 21, 2023

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
Copy link

wilba commented Mar 21, 2023

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
Copy link
Author

arpruss commented Mar 22, 2023

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
Copy link

wilba commented Mar 22, 2023

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
Copy link
Author

arpruss commented Mar 23, 2023

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
Copy link

wilba commented Mar 24, 2023

Oh that's great. Thanks for your help.

@bitfasching
Copy link
Owner

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
Copy link
Author

arpruss commented Apr 12, 2023

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
Copy link

wilba commented Apr 13, 2023

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
Copy link
Author

arpruss commented Apr 13, 2023

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."

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

3 participants