Fix hall sensor velocity estimation - #469
Conversation
Sorry, something went wrong.
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.
|
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. |
Sorry, something went wrong.
|
Thank you very much for contributing this. I can't easily test it at the moment, but we'll review it and then include it in an upcoming release. |
Sorry, something went wrong.
|
Good catch! I think the integer wrapping behavior will work just as well with signed or unsigned, but setting pulse_diff = 0 is indeed necessary to prevent returning the previous valid velocity reading if it sits still for half a _micros() wrap. I think it would even continue to return that reading for the next 36 minutes until _micros() fully wraps back to where it started. |
Sorry, something went wrong.
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:
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.