Implement pulseio.PulseIn and PulseOut for ESP8266 #716 #926
Conversation
Switch to ets_delay_us but the PWM is still way too slow to be useful.
…ython-nickzoic-716-pulseio-esp8266
There was a problem hiding this comment.
@nickzoic looks good! I bring up GPIO16 mostly because I worked the DigitalInOut issue for it. Also I don't have the SDK/datasheet handy at the day job, so disregard if I'm off base here.
For PulseIn, I could see avoiding it since there is no interal pull up?
@ALL, maybe we could make the DigitalInOut functions extendable without requiring the DigitalInOut object? Or, just handle the cases situationally.
|
|
||
| void common_hal_pulseio_pulsein_construct(pulseio_pulsein_obj_t* self, | ||
| const mcu_pin_obj_t* pin, uint16_t maxlen, bool idle_state) { | ||
| mp_raise_NotImplementedError(""); | ||
| if (pin->gpio_number == NO_GPIO || pin->gpio_function == SPECIAL_CASE) { |
There was a problem hiding this comment.
Can we extend this to GPIO16? It is the only pin with gpio_function == SPECIAL_CASE.
There was a problem hiding this comment.
Unfortunately, GPIO16 has no IRQ capability so we can't practically use it for PulseIn ... it's a bit of a lame duck pin.
There was a problem hiding this comment.
Ahhh...that's right. The curse of being attached to the RTC register.
| #include "shared-bindings/pulseio/PulseOut.h" | ||
|
|
||
| void pulseout_set(pulseio_pulseout_obj_t *self, bool state) { | ||
| PIN_FUNC_SELECT(self->pin->peripheral, state ? self->pin->gpio_function : 0); | ||
| } |
There was a problem hiding this comment.
Same here with GPIO16. Will need special handling since it doesn't work with PIN_FUNC_SELECT.
There was a problem hiding this comment.
... and the esppwm.c implementation doesn't support GPIO16 either (only pins 0, 2, 4, 5, 12, 13, 14, 15), so you can't get a PWM object for it and thus can't construct a PulseOut object either. There probably should be an explicit check for SPECIAL_CASE in common_hal_pulseio_pulseout_construct though, I'll add that in.
@tannewt and I discussed adding the ability to construct PulseOut around a DigitalInOut instead, which would be useful for external modulation and could be made to work on GPIO16. It's also possible we could substantially rewrite esppwm.c to work a lot better but that's beyond the scope of this PR :-)
There was a problem hiding this comment.
PS: Allowing DigitalInOut would require changes to the shared-bindings/pulseio/PulseOut.c so I wasn't sure that was in scope either.
There was a problem hiding this comment.
Agree that the larger change would be better outside this PR. If it is desired... 😄
There was a problem hiding this comment.
Overall, this looks good to me!
I prefer this interrupt registration approach just a bit because it'll be easier to update the micropython parts when we merge. However, I could be convinced that the other approach is better.
I'll merge after @sommersoft's approval as well.
|
@tannewt the IRQ thing I'm still experimenting with but the version in this PR has the advantage that it'll merge cleanly with upstream. It's the YAGNI version :-) It looks silly but works fine if you register a callback with (and it'd be easy enough to transition to a more flexible approach if this is warranted by some other library later ... those experimental branches still need some fixing up anyway.) |
|
Thanks all! |
Implements pulseio.PulseIn and pulseio.PulseOut for ESP8266 as per #716:
PulseIn:
PulseOut;
It also implements a generic gpio interrupt callback registration mechanism under microcontroller.Pin: this would allow the other *io libraries to register to receive GPIO interrupts. This possibly should be moved elsewhere, see also https://github.com/nickzoic/micropython/tree/circuitpython-nickzoic-rethink-gpio-interrupts
I'd be interested in feedback on this approach.