Skip to content

Fix hall sensor velocity estimation #469

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

ahughesattomra
Copy link

The hall sensor velocity estimation could end up returning a nonzero value if the timer "wrapped" between runs, even if the motor was actually stationary. If using the velocity regulator the motor may then fail to start.

To solve this:

  • Switch to using unsigned types so that wrapping is defined.
  • When timing out the pulse value, also reset it so that the velocity won't be briefly treated as valid again when the timer wraps.

Seen on a platform with 32 bit long, so running the motor and then waiting ~40 minutes is enough to observe this bug. Note that the pulse_diff variable is reset if the hall sensor direction changes, which may happen naturally when the motor stops, depending on what it is attached to.

The hall sensor velocity estimation could end up returning a nonzero
value if the timer "wrapped" between runs, even if the motor was
actually stationary. If using the velocity regulator the motor may then
fail to start.

To solve this:
- Switch to using unsigned types so that wrapping is defined.
- When timing out the pulse value, also reset it so that the velocity
  won't be briefly treated as valid again when the timer wraps.

Seen on a platform with 32 bit long, so running the motor and then
waiting ~40 minutes is enough to observe this bug.
Note that the pulse_diff variable is reset if the hall sensor direction
changes, which may happen naturally when the motor stops, depending on
what it is attached to.
@ahughesattomra
Copy link
Author

Only tested on v2.3.1 on a non-Arduino platform.

The bug is real though, so it'd be nice if it was fixed upstream.

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

Successfully merging this pull request may close these issues.

1 participant