So...what happens now? Do I need to tell someone about this PR? Do I need to do a thing?

The code looks in good shape - and you've clearly taken some care with the timings - though I haven't yet had a chance to try it out.

However, I've been investigating a different method - using the RMT module. I've been working to expose the RMT tx capabilities and believe that this would be a better approach than relying on timing. RMT is independent of the processor and highly accurate - and it should be quite straightforward to create a NeoPixel driver completely in Python using the RMT to send the pulses. You can see the start of the work on my fork on the esp_rmt branch. It's worth noting that this isn't a unique idea: FastLED have switched their ESP32 implementation to using RMT too - and it seems successful.

Anyway, I like your work but I'm hoping that using RMT might be a better way to implement NeoPixel driving.

Nice @mattytrentini , I'll check your stuff out!

@mattytrentini , so I'm looking through things, and I think while this may work for the ESP32, which would be great, I'd still like to keep it functional with the pure-timing stuff, because I'd like to also put this into the other ports, if this one gets approved.

I'd like to keep it, shall we say, portable.

https://www.youtube.com/watch?v=7uW47jWLMiY

Thanks for the contribution. If it's possible to do proper neopixel timing in software that would be convenient. Using the RMT could also be an option, and having both ways to do it (software and RMT) would be ideal if they both work correctly.

because I'd like to also put this into the other ports

+1 to have a portable implementation.

I was actually working on a similar thing for stm32 with the following signature:

pixelbitstream(pin, timing_ns, buf)

Where timing_ns is a 4-tuple of nanosecond timing values, 0-high, 0-low, 1-high, 1-low. Typical values would be (450, 800, 850, 400) for standard neopixel, and can be changed for other variants.

@maludwig do you think you can modify your implementation to take in this 4-tuple of nanosecond timing values?

Hey @dpgeorge , I'd be more than happy to have it accept a 4-tuple of timing values, but I don't have an STM32 to test it with. I'd also like to leave the existing CPU tick implementation to allow for the finest grained control. So if you think it's a good idea, let's merge this particular PR, and then I'll make another PR that moves the timing stuff into the Python, and allows the user to pass in a 4-tuple.

I think that would be better too, because then we could put the timing into the Python side and then future devs wouldn't need to brush up on C++ to add a new timing spec for like, the SK#### neopixels.

I also want to add the concept of "double-buffering", like, VSync, so that a FreeRTOS task can run concurrent to the main Python task, so that your Python code can put stuff into buffer B while concurrently, the neopixels are streaming buffer A, and then put stuff into buffer A when the other core is streaming our buffer B to the pixels. But maybe that's a future future PR.

Micropython is gonna have just BALLER neopixel support.

See #5294 for an stm32 implementation.

Looks like our two implementations are compatible. Same high level view. You've made yours in a bit of a different way. I've maintained backwards compatibility with the old ways, but your code is smaller and looks cleaner, and is inside machine, where mine is inside neopixel.

I don't have an STM to test with or I'd help you merge yours.

My only suggestion for yours, would be to make the pixelbitstream be a part of neopixel, rather than part of machine. I think of machine like, things specific to that hardware.

I'll leave comments on your PR. Got anything more you'd like me to do before we merge?

@dpgeorge I've added pixelbitstream for you, kept the tick-based version too, but the pixelbitstream implementation will correct for CPU freq for those who want to just use timing.

I've also wiped out my previous fixes to the docs, since I think it was you actually who added support for the latest ESP-IDF.

Can we merge this now?

@mattytrentini I just found this, thought you might find it interesting:
https://github.com/espressif/esp-idf/tree/f91080637/examples/peripherals/rmt/led_strip

You've probably already seen it, for I thought, just in case!

@dpgeorge Should I be losing hope on merging this guy?

@dpgeorge How's this looking to you? Looks good to me and seems to take in the tuple as you requested...

@maludwig Sorry about the delay on this PR. For v1.17 we merged the bitstream feature mentioned in #5117 (comment) for stm32/esp32/esp8266 and in the process of adding this to ESP32 I fixed the same overflow bug as this PR and made the timing customisable from Python. I have not been able to see any glitching with this driver. Subsequently, bitstream has been added for more ports.

Further to this, last week I implemented an RMT driver to replace the bit banging implementation and so far sounds like it further improves reliability -- see #7985