#3875 fix for rotary reverse direction dead spot #3895
Conversation
It is possible that the `quarter` counter can get out of sync with the quadrature pins. By checking the new_state, instead of just the quarter counter, it stays in sync with the rotary encoding. adafruit#3875
|
Thank you for working on this. Happy to review and test when you're done. |
|
all done except that one test failed, humph !? |
That was a network problem at GitHub; we can ignore that. |
| @@ -51,10 +51,10 @@ static void _intr_handler(nrfx_gpiote_pin_t pin, nrf_gpiote_polarity_t action) { | |||
|
|
|||
| // logic from the atmel-samd port: provides some damping and scales movement | |||
| // down by 4:1. | |||
| if (self->quarter >= 4) { | |||
| if (self->quarter > 0 && new_state == 2) { | |||
There was a problem hiding this comment.
Oh a finer point might be to refactor this to be ever so slightly more efficient:
- switch the order of the conditional
(new_state == 2 && self->quarter > 0)because thequarterwill be non-zero more often than the state is 2 - I guess to go even further it could be refactored with an outer if clause and an inner case, perhaps like this:
if (new_state == 2 && self->quarter != 0) {
if (self->quarter > 0) { // increment
...
} else { // decrement
...
}
}
What do you think @dhalbert or anyone else?
|
Take a look also at https://github.com/adafruit/circuitpython/blob/48de52c8ab62fae93e1382cb774802828cf92edd/ports/atmel-samd/common-hal/rotaryio/IncrementalEncoder.c, which implements the same thing (the nrf version is a rewrite; I wrote the |
|
nice, I am looking and learning vocab (like detent) and will keep looking at them both to compare, fun. |
|
closing this PR for now. will be taking another tact on this fix to handle inverted pins etc. |
|
I am going to look at this whole issue again, reviewing the current implementation, and I hope come up with a reference implementation that can be ported easily. |
It is possible that the
quartercounter can get out of sync with the quadrature pins. By checking the new_state, instead of just the quarter counter, it stays in sync with the rotary encoding.During testing I was able to recreate the bug and saw that the
quartercount would sometimes count (from a settled click position to next encoder position) 2, 3, 4, 1 and back to 2. In this mode it does a position increment mid way through the quadrature cycle and it works fine if it is always increasing or always decreasing. When the encoder reverses it does not pass 4, instead it counts down 2, 1, 0, -1, -2 (which will not decrement the position).This PR changes the conditions for a
positionincrement or decrement, to only fire when the encoder is at the settled state.