Skip to content
This repository was archived by the owner on Apr 20, 2022. It is now read-only.

Discrepancy in setting dotstar brightness byte #25

Closed
dunkmann00 opened this issue Jul 9, 2020 · 3 comments · Fixed by #26
Closed

Discrepancy in setting dotstar brightness byte #25

dunkmann00 opened this issue Jul 9, 2020 · 3 comments · Fixed by #26
Labels
enhancement New feature or request

Comments

@dunkmann00
Copy link
Contributor

The dotstar brightness byte is exposed to the user as a float, but then internally converted into an integer, so it needs to be rounded.

In pypixelbuf the brightness is always rounded up using an approach equivalent to math.ceil:

# same as math.ceil(brightness * 31) & 0b00011111
# Idea from https://www.codeproject.com/Tips/700780/Fast-floor-ceiling-functions
w = (32 - int(32 - w * 31) & 0b00011111) | DOTSTAR_LED_START

However, in pixelbuf the brightness is rounded down by truncating:
https://github.com/adafruit/circuitpython/blob/41fe62929f268bfc1f2b06be8b649409234d9d1a/shared-module/_pixelbuf/PixelBuf.c#L181-L185

If rounding down is desired I can make a pull request to correct this? Or if not, I can file an issue in CP and update pixelbuf to round up!

@evaherrada evaherrada added the enhancement New feature or request label Jul 10, 2020
@tannewt
Copy link
Member

tannewt commented Jul 10, 2020

Consistency is the most important to me. Does rounding up prevent setting brightness to zero?

@dunkmann00
Copy link
Contributor Author

No, if you set the brightness to 0.0 it will be set to 0.

The rounding, is just happening because of the float needing to become a whole number (a 5 bit int for the dotstar). So in the pypixelbuf code, it really is just equivalent to a math.ceil, so 0 still yields a 0.

As an example:

User enters: 0.0  -->  Multiply by 31 (5 bits): 0     -->  Set brightness with: 0 (same with pixelbuf)

User enters: 0.4  -->  Multiply by 31 (5 bits): 12.4  -->  Set brightness with: 13 (pixelbuf would set to 12)

Either way is equally useful, really just a matter of preference!

@tannewt
Copy link
Member

tannewt commented Jul 13, 2020

I'd round down to be consistent with _pixelbuf because it is harder to change.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants