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

[Backport v3.4-branch] drivers: sensor: ams_as5600: Fix calculation of fractional part #70143

Closed

Conversation

yashi
Copy link
Collaborator

@yashi yashi commented Mar 13, 2024

commit 98903d4 upstream.

The original calculation has two bugs. One is the calculated value, and the other is that the value is not in one-millionth parts.

What the original calculation does is compute a scaled position value by multiplying the raw sensor value (dev_data->position) by AS5600_FULL_ANGLE, which represents a full rotation in degrees. It then subtracts the product of the whole number of pulses (val->val1) and AS5600_PULSES_PER_REV from this scaled position value.

((int32_t)dev_data->position * AS5600_FULL_ANGLE)
- (val->val1 * AS5600_PULSES_PER_REV);

What you actually need is to extract the fractional part of the value by taking the modulo of AS5600_PULSES_PER_REV from the scaled value of the position.

(((int32_t)dev_data->position * AS5600_FULL_ANGLE)
% AS5600_PULSES_PER_REV)

Then convert the value to one-millionth part.

  • (AS5600_MILLION_UNIT / AS5600_PULSES_PER_REV);

Fixes #70142

commit 98903d4 upstream.

The original calculation has two bugs. One is the calculated value, and the
other is that the value is not in one-millionth parts.

What the original calculation does is compute a scaled position value by
multiplying the raw sensor value (`dev_data->position`) by
`AS5600_FULL_ANGLE`, which represents a full rotation in degrees. It then
subtracts the product of the whole number of pulses (`val->val1`) and
`AS5600_PULSES_PER_REV` from this scaled position value.

    ((int32_t)dev_data->position * AS5600_FULL_ANGLE)
    - (val->val1 * AS5600_PULSES_PER_REV);

What you actually need is to extract the fractional part of the value by
taking the modulo of AS5600_PULSES_PER_REV from the scaled value of the
position.

   (((int32_t)dev_data->position * AS5600_FULL_ANGLE)
   % AS5600_PULSES_PER_REV)

Then convert the value to one-millionth part.

   * (AS5600_MILLION_UNIT / AS5600_PULSES_PER_REV);

Signed-off-by: Yasushi SHOJI <yashi@spacecubics.com>
@kartben
Copy link
Collaborator

kartben commented Mar 13, 2024

@yashi looks like you've opened the same PR thrice -- could you please check/clean up? Thanks!

@henrikbrixandersen
Copy link
Member

@yashi looks like you've opened the same PR thrice -- could you please check/clean up? Thanks!

They are targeting different branches.

@yashi In the future, please just use the backport labels on the original PR.

@henrikbrixandersen henrikbrixandersen changed the title drivers: sensor: ams_as5600: Fix calculation of fractional part [Backport v3.4-branch] drivers: sensor: ams_as5600: Fix calculation of fractional part Mar 15, 2024
@henrikbrixandersen
Copy link
Member

Zephyr v3.4 is EOL,

@henrikbrixandersen henrikbrixandersen added the Backport Backport PR and backport failure issues label Mar 15, 2024
@henrikbrixandersen henrikbrixandersen added this to the v3.4.1 milestone Mar 15, 2024
@yashi
Copy link
Collaborator Author

yashi commented Mar 16, 2024

Zephyr v3.4 is EOL,

OK. I'm closing this, then. Thanks.

@yashi yashi closed this Mar 16, 2024
@yashi yashi deleted the ams_as5600-for-v3.4 branch March 20, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Sensors Sensors Backport Backport PR and backport failure issues
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants