-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
raspberrypi: Implement rotaryio, improve StateMachine #4329
Conversation
I named the property `in_available` because it is similar to pyserial. However, it indicates the number of words in the fifo, not the number of bytes.
This can be used where the standard API calls for a list of pins, to check that they satisfy the requirements of the rp2pio state machine, e.g., ```python def __init__(self, pin_a, pin_b): if not rp2pio.pins_are_sequential([pin_a, pin_b]): raise ValueError("Pins must be sequential") ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I think you need a make translate
. Code looks good otherwise!
@tannewt done -- if you get a chance to check again, that'd be dandy |
Any two consecutive pins can be used for an IncrementalEncoder Testing performed: Put a synthesized (few hundred counts per second) quadrature signal into GP2/3 and read the encoder out. Performed filesystem operations at the same time to stress test it. The reasons for not using common_hal_rp2pio_statemachine_readinto are commented on.
This example requires adafruit/circuitpython#4329 to be merged in order to work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested with a volume-control program I use daily on a Trinket M0. It works just fine!
Did you diagnose the "dead spot" problem from #3967? I don't see that issue immediately.
No, I used the same quadrature decoder as the samd port. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One style thing. Thanks for doing this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me! Thank you! I think @DavePutz will want to integrate this into the PulseIn work too.
@@ -71,4 +71,6 @@ bool common_hal_rp2pio_statemachine_get_rxstall(rp2pio_statemachine_obj_t* self) | |||
void common_hal_rp2pio_statemachine_clear_rxfifo(rp2pio_statemachine_obj_t *self); | |||
size_t common_hal_rp2pio_statemachine_get_in_waiting(rp2pio_statemachine_obj_t *self); | |||
|
|||
void common_hal_rp2pio_statemachine_set_interrupt_handler(rp2pio_statemachine_obj_t *self, void(*handler)(void*), void *arg, int mask); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For next time, I'd put this in ports/raspberrypi/common-hal/rp2pio/StateMachine.h
because it's a port-specific API that is unrelated to the Python API. It's a bit weird since this module is itself port-specific.
Fixes #4226.
Now rotaryio is implemented in this port. For the attached example, either connect a rotary encoder at GP2/3, or connect GP2/3 and GP20/21 to use a synthesized signal.
Click to show Python code
The following functionality was added to StateMachine:
Also a few other things "crept in":
With the Python API improvements, it's also possible to implement a form of IncrementalEncoder in pure Python:
Click to show Python code