search

adafruit / Adafruit_CircuitPython_Pypixelbuf

Pure python implementation of _pixelbuf for smaller boards
Other
4 stars 11 forks source link

Discrepancy in setting dotstar brightness byte #25

Closed dunkmann00 closed 4 years ago

dunkmann00 commented 4 years ago

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: https://github.com/adafruit/Adafruit_CircuitPython_Pypixelbuf/blob/e52c672196102e362ddc8b94dbe7fda6a9c95a49/adafruit_pypixelbuf.py#L255-L257

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!

tannewt commented 4 years ago

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

dunkmann00 commented 4 years ago

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 commented 4 years ago

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