micropython / micropython

MicroPython - a lean and efficient Python implementation for microcontrollers and constrained systems
https://micropython.org
Other
19.29k stars 7.72k forks source link

1.18/esp32: neopixel and bitstream() changes #8198

Closed cederom closed 2 years ago

cederom commented 2 years ago

Hello word :-)

I noticed that neopixel low level driver on ESP32 has changed at bitstream() level in 1.18. It now seems to use RMT block.

Is call to old bitstream() still available? My custom device driver was depending on that one. It works on 1.17 and stopped working on 1.18.

According to 1.18 Manual (https://docs.micropython.org/en/latest/esp32/quickref.html) it should be possible with code below:

import esp
esp.neopixel_write(pin, grb_buf, is800khz)

But 1.18 release for ESP32 does not have this call available:

MPY: soft reboot
MicroPython v1.18 on 2022-01-17; ESP32 module with ESP32
Type "help()" for more information.
>>> import esp
>>> esp.neopixel_write()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'module' object has no attribute 'neopixel_write'
>>> dir(esp)
['__class__', '__name__', 'LOG_DEBUG', 'LOG_ERROR', 'LOG_INFO', 'LOG_NONE', 'LOG_VERBOSE', 'LOG_WARNING', 'dht_readinto', 'flash_erase', 'flash_read', 'flash_size', 'flash_user_start', 'flash_write', 'gpio_matrix_in', 'gpio_matrix_out', 'osdebug']

In the source tree micropython/ports/esp32/machine_bitstream.c I can see various implementations of machine_bitstream_high_low() but the question is how to access and call them?

Any hints appreciated :-)

cederom commented 2 years ago

Okay, I found some hints in file https://github.com/micropython/micropython/blob/a3bbd5332bff0a67204196d2e365ad10ff773d58/docs/library/esp32.rst that:

Passing in None disables the use of RMT and instead selects a bit-banging implementation for machine.bitstream.

The quick fix on 1.18 is:

from esp32 import RMT
RMT.bitstream_channel(None)
bitstream(self.pin, _BITSTREAM_TYPE_HIGH_LOW, self.timing, self.buf) # this now uses old bitbang.

But that does not work on 1.17:

AttributeError: type object 'RMT' has no attribute 'bitstream_channel'
MicroPython v1.17 on 2021-09-02; ESP32 module with ESP32

Looks like the bitstream itself seems better place to select its underlying driver implementation selected with additional optional named parameter..

dpgeorge commented 2 years ago

esp.neopixel_write() was actually removed in v1.17 by this commit: 71f4faac2732d932dcc03bdbc9f80434e5757edb. But that failed to remove the reference from the quickref, so that needs to be fixed.

machine.bitstream() can be used instead of esp.neopixel_write().

The machine.bitstream() implementation in v1.17 was bit-banging. This was reported to have timing errors, see #7985 and #8161, and so was replaced with an RMT implementation in v1.18.

The bit-banging implementation was reinstated to give the user more control, see #8158. These changes were pointed out in the release notes https://micropython.org/resources/micropython-ChangeLog.txt. It's highly recommended to read them when upgrading!

This is not yet part of the official documentation

Which part is not documented? It should all be there.

This seems to be in conflict when someone wants so use RMT (or its channel) for other tasks.

There are many channels and bitstream uses the last channel as a default. And you can disable this if needed via esp32.RMT.bitstream_channel(None).

Why not use bitstream(... , driver='RMT') or bitstream(... , driver='bitbang') instead?

That is an option, but the idea was to set it globally once and then not need to change any other code. And the default is RMT because that should be more reliable than bit-banging for more use cases, so that it works "out of the box" for more users.

cederom commented 2 years ago

Thank you @dpgeorge :-)

esp.neopixel_write() was actually removed in v1.17 by this commit: 71f4faa. But that failed to remove the reference from the quickref, so that needs to be fixed.

Yup :-)

machine.bitstream() can be used instead of esp.neopixel_write().

Hmm, I was using exactly this way (machine.bitstream), but on ESP32 seems to be set by default to use RMT in 1.18, so esp32.RMT.bitstream_channel(None) needs to performed in the first place anyways. This is why choosing the underlying driver in bistream constructor could be more convenient and result in self-describing code :-)

I was using this old machine.bitstream bitbang in my custom driver that performed some additional operations before and after bitbang. Maybe this would be also possible to achieve with RMT but I would have to get familiar with peripheral and the current code. It broke after switch to 1.18 but now I know how to fix it by switching back to bitbang, so the problem is somehow solved :-)

Do you know is it at least possible to set default line state of the RMT output to high or low after the stream? Is it always High-To-Low streaming? Can be inverted? For some reason using Pin.value() before and after RMT controls pin does not work the same way as it was working with using Pin.value() with bitbang.

The machine.bitstream() implementation in v1.17 was bit-banging. This was reported to have timing errors, see #7985 and #8161, and so was replaced with an RMT implementation in v1.18.

The bit-banging implementation was reinstated to give the user more control, see #8158. These changes were pointed out in the release notes https://micropython.org/resources/micropython-ChangeLog.txt. It's highly recommended to read them when upgrading!

Yup, I figured that out already by searching the source code. Thank you for the Changelog reference will remember about this one :-) Quickref update would be nice to have as this is the first place I (and maybe some other folks) search :-)

This is not yet part of the official documentation

Which part is not documented? It should all be there.

Quickref www does not seem to yet contain info from https://github.com/micropython/micropython/blob/a3bbd5332bff0a67204196d2e365ad10ff773d58/docs/library/esp32.rst :-)

This seems to be in conflict when someone wants so use RMT (or its channel) for other tasks.

There are many channels and bitstream uses the last channel as a default. And you can disable this if needed via esp32.RMT.bitstream_channel(None).

Ah, so this will only disable bitstream driver using RMT and switch to old bitbang. No interference with other RMT operations. Now the idea is clear. Thanks! :-)

Why not use bitstream(... , driver='RMT') or bitstream(... , driver='bitbang') instead?

That is an option, but the idea was to set it globally once and then not need to change any other code. And the default is RMT because that should be more reliable than bit-banging for more use cases, so that it works "out of the box" for more users.

I understand that RMT seems more reliable and that approach with esp32.RMT.bitstream_channel(None) is the least invasive method. On the other hand other ports may utilize other underlying hardware, so choosing driver in the bitstream constructor seems more coherent.. at least for me :-)

Thank you :-)

dpgeorge commented 2 years ago

Do you know is it at least possible to set default line state of the RMT output to high or low after the stream?

I think you can do that with the idle_level argument to the RMT constructor.

Quickref update would be nice to have as this is the first place I (and maybe some other folks) search :-)

Now done in 326b2c79dfaf472d67e5e3fee5868e78ccc6be73

On the other hand other ports may utilize other underlying hardware, so choosing driver in the bitstream constructor seems more coherent.. at least for me

Yes, I can see the advantages to doing that. It might make sense to support both the esp32.RMT.bitstream_channel() function and specifying the implementation in the call to machine.bitstream() itself. That would at least be necessary if you had multiple threads and wanted the separate threads to use different schemes.