Skip to content
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.

Sign up for GitHub

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

drivers/led/neopixel: Add brightness control. #739

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

drivers/led/neopixel: Add brightness control. #739

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2d9486c
drivers/led/neopixel: Add brightness to class init
tmountjr Sep 28, 2023
94e3dab
drivers/led/neopixel: Calculate the new values once.
tmountjr Sep 29, 2023
8105f2d
drivers/led/neopixel: Increment version number
tmountjr Sep 29, 2023
69f6d61
drivers/led/neopixel: Add set_brightness method
tmountjr Sep 29, 2023
47c5dbc
drivers/led/neopixel: Clamp brightness between 0.0 and 1.0.
tmountjr Sep 29, 2023
92967e0
drivers/led/neopixel: Also clamp constructor brightness
tmountjr Sep 29, 2023
6f3ebea
drivers/led/neopixel: Optimize for speed on non-FP builds.
tmountjr Oct 4, 2023
f683608
drivers/led/neopixel: Fix lint issue.
tmountjr Oct 4, 2023
0014eac
drivers/led/neopixel: Continue lint fixes.
tmountjr Oct 4, 2023
866fd2f
drivers/led/neopixel: code size tweaks
tmountjr Oct 6, 2023
9e80530
drivers/led/neopixel: Refactor brightness() for speed.
tmountjr Oct 6, 2023
9b5be62
drivers/led/neopixel: Add tests for new features.
tmountjr Oct 6, 2023
6ca933b
drivers/led/neopixel: Fix formatting.
tmountjr Oct 6, 2023
1f1f51d
drivers/led/neopixel: Fix test formatting.
tmountjr Oct 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion micropython/drivers/led/neopixel/manifest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
metadata(description="WS2812/NeoPixel driver.", version="0.1.0")
metadata(description="WS2812/NeoPixel driver.", version="0.2.0")

module("neopixel.py", opt=3)
22 changes: 21 additions & 1 deletion micropython/drivers/led/neopixel/neopixel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@ class NeoPixel:
# G R B W
ORDER = (1, 0, 2, 3)

def __init__(self, pin, n, bpp=3, timing=1):
def _clamp(self, v, c_min, c_max):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely stylistically better, but functions are expensive in terms of code size (and therefore memory usage).

In this case, it's actually cheaper to just use min(max(brightness), 0, 1) directly in both places. Note also changing 0.0 to 0 etc is also helpful for size (and doesn't change the behaviour).

return min(max(v, c_min), c_max)

def __init__(self, pin, n, bpp=3, timing=1, brightness=None):
self.pin = pin
self.n = n
self.bpp = bpp
self.brightness = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should rename this to self.b -- class members end up allocating qstrs, so using small names is better.

It's also smaller to write

self.b = None if brightness is None else min(max(brightness), 0, 1)

rather than

self.b = None
if brightness is not None:
   self.b = min(max(brightness), 0, 1)

if brightness is not None:
self.brightness = self._clamp(float(brightness), 0.0, 1.0)
self.buf = bytearray(n * bpp)
self.pin.init(pin.OUT)
# Timing arg can either be 1 for 800kHz or 0 for 400kHz,
Expand All @@ -22,11 +28,17 @@ def __init__(self, pin, n, bpp=3, timing=1):
else timing
)

def _apply_brightness(self, v):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to _b (to save qstr usage). It's a private method, so better to add a comment than to waste bytes on the name. (Unfortunately this is par for the course with MicroPython... there are lots of ugly things we have to do, ideally in places the user doesn't see, in order to save code size. Every byte counts).

if self.brightness is not None:
return tuple(round(c * self.brightness) for c in v)
return v

def __len__(self):
return self.n

def __setitem__(self, i, v):
offset = i * self.bpp
v = self._apply_brightness(v)
for i in range(self.bpp):
self.buf[offset + self.ORDER[i]] = v[i]

Expand All @@ -35,6 +47,7 @@ def __getitem__(self, i):
return tuple(self.buf[offset + self.ORDER[i]] for i in range(self.bpp))

def fill(self, v):
v = self._apply_brightness(v)
b = self.buf
l = len(self.buf)
bpp = self.bpp
Expand All @@ -45,6 +58,13 @@ def fill(self, v):
b[j] = c
j += bpp

def set_brightness(self, b: float):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to brightness (now that that name isn't used for the field). It's more in line with what we do elsewhere (e.g. pin.value()).

Also I think, similarly, it should probably return the current brightness too, e.g.

    def brightness(self, brightness):
        if brightness is not None:
            self.b = min(max(brightness, 0), 1)
            # This may look odd but because __getitem__ and __setitem__ handle all the
            # brightness logic already, we can offload the work to those methods.
            for i in range(self.n):
                self[i] = self[i]
        return self.b

which is +10 bytes, but I think useful for people to write e.g. strip.brightness(strip.brightness() + 0.1).

self.brightness = self._clamp(b, 0.0, 1.0)
# This may look odd but because __getitem__ and __setitem__ handle all the
# brightness logic already, we can offload the work to those methods.
for i in range(self.n):
self[i] = self[i]

def write(self):
# BITSTREAM_TYPE_HIGH_LOW = 0
bitstream(self.pin, 0, self.timing, self.buf)