-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
getCumulativePosition not using direction #65
Comments
Thanks for this issue, I need to refresh my mind with the code details but it could be a good catch. (Note for myself) |
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. |
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? |
think ist should be zero while in -4095 .. 4095 |
and why not keep the readReg2(AS5600_ANGLE) & 0x0FFF; |
Then I will patch that function. |
True, |
I copied the snipplet into my code, seems to work. |
If these bits are not zero, they will be masked correctly by the value &= 0x0FFF; line. A test saved 10 bytes with the patch on an UNO. So I'll keep it |
mmm, if the direction is "reverse" there should be a & 0x0FFF as 4096 - 0 = 4096 which should be 0. |
whats about introducing an update method which calculates speed and position? So the I2C register has only to be read once? |
& 0x0FFF Should be fixed in setOffset() too. |
The idea is interesting however not clear how it affects performance and footprint. |
Created the develop branch for this issue (and some pending backlog). |
If you have time, can you verify that the develop branch fixes the use of direction for you? |
Merged PR and published 0.6.2 release. |
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.
The text was updated successfully, but these errors were encountered: