Do all implementations now have floating point enabled? (#define MICROPY_FLOAT_IMPL MICROPY_FLOAT_IMPL_FLOAT )

I had just assumed that float was a built-in type. It seems to be referenced that way here - https://docs.micropython.org/en/latest/genrst/builtin_types.html.

If that's not the case, what's the process for handling floating point numbers on systems that don't have that type?

Thanks @tmountjr

@ricksorensen is correct that not all boards have floating point enabled. The bigger issue though is that using floating point is very expensive in MicroPython because floats get heap-allocated so you want to absolutely avoid using them unless necessary.

For example, on a PYBV11, the following line:

n.fill((255,0,0,0))

takes 2058 microseconds before this PR, but now takes 24686 microseconds.

But I think this can probably be almost entirely addressed:

• Make brightness optional (and None if not set).
• Only calculate the adjusted tuple once (right now it's doing it for every pixel).
• Keep fill as it was (implementing it in terms of __setitem__ is slow).

See e.g. https://gist.github.com/jimmo/472bcf9213bae97fc915fd71d85048b9 which is back to ~2048 microseconds when brightness is not set and 2238 microseconds when set to 0.5.

I'm also not sure about the behavior of __getitem__ though. It's a bit weird that if you set a pixel and read it back, you get the adjusted value? I realise it's difficult to make this work any other way though.

Good idea only conditionally applying brightness if it's already set. Also didn't consider that I'd be recalculating the values for every single pixel if I simplified the fill() method.

The use of __getitem__ in the set_brightness method was a bit of a shot in the dark for me too - I basically thought of it from a high level - when you set the brightness, you need to iterate over each pixel and recalculate what the new tuple should be based on the new brightness. __getitem__ already handles getting the right component value for each "pixel" in the bytearray, so the left half takes care of itself; and __setitem__ handles calculating the brightness as a function of setting the component values, so that's the right half too. I agree though that self[i] = self[i] seems kinda redundant but unless there are performance concerns with using the under methods it may qualify as elegant. 😄 It could use some documentation though, if only to acknowledge the fact that it looks really odd.

@jimmo Thanks for all your feedback! You seem to have access to some hardware that I don't that catches corner cases...would you be able to take a look at the latest version of this PR on that hardware and let me know if the performance is where it should be?

You seem to have access to some hardware

Nope just any board running MicroPython:

t = time.ticks_us()
... do some code ...
print(time.ticks_diff(time.ticks_us(), t))

catches corner cases

Nothing special going on here, was just calling fill() for neopixel objects with and without the brightness set.

It would be also worth testing code that sets pixels directly (which I imagine is more representative of what people typically do with neopixels).

I'm on the fence about this... I agree it's a useful feature -- it's nice to be able to e.g. fade in/out a strip or just separate your pixel generating code from brightness control. And it's good that it doesn't affect performance significantly if you don't use it (although set pixel will now be slower because it has to check self.brightness). I am still not super excited about the fact that set and get pixel are no longer symmetric (but again I don't have a better suggestion... so maybe it's a reasonable compromise).

The other thing to think about is code size increase. I spent a bit of time trying to minimise neopixel.py as much as possible (which is why the style is a bit weird). neopixel.mpy is currently 616 bytes. This PR increases it to 925. I think we can get it down to 819 without any functional changes. I'll add some comments to the PR to explain.

Did some tests on my Pico W...

• fill() time with the changes: 658us without a brightness, 1093us with a brightness
• changing brightness with brightness(): 4294us <-- that felt a little concerning to me so I'm going to refactor and post the "after" (it's way faster now)

(See related micropython/micropython#3623 and micropython/micropython#4815)

What are the next steps with this PR? I see the two closed PRs on the main micropython repo but I'm not clear if I need to make any changes in light of those. Thanks!

Hi,
I don't understand something...
I have just add a brightness function to your lib, and I have only 173 us for a fill() and a write() without brightness function, and 256 with brightness function, on a pyboard v1.1...
On a feather M4, I have 249 us without brightness function, and 283 with it.
On other way, I have problems to use strip, which contains severals neopixels, with the classical ws2812 lib (spi) too... Electrical problems I think...
Your lib with the brightness function is there https://www.cosmocratie.fr/data.html

@IHOXOHI any chance you can put that code in a gist for me? the link you provided looked a little sketchy.

Is your question why the timing is different? I'm not sure I know what you're asking for help with.

Just commenting that I went looking for this functionality today only to wind up at this PR. It isn't too much of a lift to simply implement equivalent functionality for myself for the time being, but I would love to see this PR eventually get merged.

Ok, I will post It on GitHub... M'y first.
M'y question is about the color which depends of a frequency, interfere with the frequency which controls each place of each neopixel... I presume.
..
Is there anybody who succeeded to use a big strip of bigs neopixels? More than 5...

The lib modified: github.com/IHOXOHI/neopixel-brightness-function/blob/main/README.md