brightness instance variable is broken in some way

[kevinjwalters]

There is something odd going on in the code with a mix of the use of self.brightness and self._brightness - it won't work in its current form and it's confusing.

On minor issues, there's no use of the python * pseudo-argument in constructor to force use of named arguments for last two args. And the example in docs uses an integer 1 for brightness rather than floating point 1.0

[kevinjwalters]

I based the WS2801 code on this so inherited the brightness issue, see https://github.com/kevinjwalters/Adafruit_CircuitPython_WS2801 and https://github.com/adafruit/Adafruit_CircuitPython_WS2801

[tannewt]

In what way doesn't brightness work? Are you assuming that it'll update the pixels when its set?

[mattytrentini]

Heya @kevinjwalters, would you be able to clarify what you meant about brightness/_brightness not working? Maybe I can help fix it.

@tannewt PR #12 has a small fix for the issue you mentioned; ie modifying brightness will now trigger a call to show() if auto_write is True.

[mattytrentini]

While we're talking brightness, the scale of 0.0-1.0 looks like it could use some work...I find that I can set it as low as 0.004 (any lower appears to turn the LED off). Values from 0.1 down to 0.004 are quite discernible.

Could adjust the scale, perhaps from 0.1 to 1.0? Or even 1-100. In both cases though it may affect existing code...

[tannewt]

Rounding makes it tricky. I think 1.0 to 0.0 is fine because its normally used programmatically. Thanks for the auto-write fix!

[kevinjwalters]

Been distracted by snow.

I hadn't run the code, was just inspecting it and didn't understand the role of the two instances variables _brightness and brightness. Inspecting 503e854:

• constructor places the value in self.brightness (and sets self._brightness to a fixed value of 1.0)
• getter/setter reads/writes to self._brightness - differs to constructor
• show() conditional for using brightness tests self.brightness but the value used to scale values is self._brightness

OH. I am new to Python and may have misunderstood how Python treats self.brightness when brightness() is defined in the code. Yes, this is the source of my confusion, I wasn't aware of the @Property decorator/synatic sugar drop. What's the purpose of the self._brightness = 1.0 assignment in constructor? Is there a reason for the mixing of techniques for reading the value in show()?

[kevinjwalters]

I'll close this as this looks ok.

[tannewt]

@kevinjwalters The assignment to _brightness is done because pylint checks for assignments to instance variables in the constructor. I agree its a little weird.