getCumulativePosition not using direction

[Chris-42]

getCumulativePosition does not use the direction set in initialization. better replace int16_t value = readReg2(AS5600_ANGLE) & 0x0FFF; by int16_t value = readAngle(); which is the same with taking direction into account.

[RobTillaart]

Thanks for this issue, Assuming you have tested this, did it gave better results? as I have no hardware nearby to verify.

I need to refresh my mind with the code details but it could be a good catch.

---

(Note for myself) When looking at readAngle() to check the direction code I noticed the mod 4096 (& 0x0FFF) is done too often. rawAngle() idem.

[RobTillaart]

@Chris-42

Can you verify this modification with your setup?

uint16_t AS5600::rawAngle()
{
  int16_t value = readReg2(AS5600_RAW_ANGLE);
  if (_offset > 0) value += _offset;
  value &= 0x0FFF;

  if ((_directionPin == AS5600_SW_DIRECTION_PIN) &&
      (_direction == AS5600_COUNTERCLOCK_WISE))
  {
    value = (4096 - value);
  }
  return value;
}

uint16_t AS5600::readAngle()
{
  uint16_t value = readReg2(AS5600_ANGLE);
  if (_offset > 0) value += _offset;
  value &= 0x0FFF;

  if ((_directionPin == AS5600_SW_DIRECTION_PIN) &&
      (_direction == AS5600_COUNTERCLOCK_WISE))
  {
    value = 4096 - value;
  }
  return value;
}

Will make a develop branch to add your patch.

[RobTillaart]

@Chris-42

This is a related open issue.

Currently the function getRevolutions() returns -1 directly when the cumulative angle goes below zero. And -2 -3 etc

Should it be more correct or intuitive to return zero until the first (negative) revolution has been made?

[Chris-42]

think ist should be zero while in -4095 .. 4095

[Chris-42]

and why not keep the readReg2(AS5600_ANGLE) & 0x0FFF; I found no register description stating the upper bits are always zero. Just to be on the safe side...

[RobTillaart]

think ist should be zero while in -4095 .. 4095

Then I will patch that function.

[RobTillaart]

and why not keep the readReg2(AS5600_ANGLE) & 0x0FFF; I found no register description stating the upper bits are always zero. Just to be on the safe side...

True, it only safes a few bytes and performance wise the gain is a few micros, where the I2C transaction is in the millis range.

[Chris-42]

I copied the snipplet into my code, seems to work.

[RobTillaart]

and why not keep the readReg2(AS5600_ANGLE) & 0x0FFF; I found no register description stating the upper bits are always zero. Just to be on the safe side...

True, it only safes a few bytes and performance wise the gain is a few micros, where the I2C transaction is in the millis range.

If these bits are not zero, they will be masked correctly by the value &= 0x0FFF; line. Also if the adding of the offset causes the value to be above 4095 it will be clipped correctly.

A test saved 10 bytes with the patch on an UNO. So I'll keep it

[RobTillaart]

mmm, if the direction is "reverse" there should be a & 0x0FFF as 4096 - 0 = 4096 which should be 0.

[Chris-42]

whats about introducing an update method which calculates speed and position? So the I2C register has only to be read once? then one can have update called in loop and use siple getters for getCumulativePosition and getAngularSpeed.

[RobTillaart]

& 0x0FFF Should be fixed in setOffset() too.

[RobTillaart]

whats about introducing an update method which calculates speed and position? So the I2C register has only to be read once? then one can have update called in loop and use siple getters for getCumulativePosition and getAngularSpeed.

The idea is interesting however not clear how it affects performance and footprint. Please open a a new issue as I do not want to put too much new code into one version.

[RobTillaart]

@Chris-42

Created the develop branch for this issue (and some pending backlog). Build has started.

[RobTillaart]

@Chris-42

If you have time, can you verify that the develop branch fixes the use of direction for you?

[RobTillaart]

Merged PR and published 0.6.2 release. (only #67 pending, will pick that up asap)