adafruit / Adafruit_LED_Backpack

Adafruit LED Backpack Library for our various LED backpacks.
MIT License
299 stars 192 forks source link

Add setDisplayState() to turn display on and off #78

Closed jonnybergdahl closed 9 months ago

jonnybergdahl commented 9 months ago

Thank you for creating a pull request to contribute to Adafruit's GitHub code! Before you open the request please review the following guidelines and tips to help it be more easily integrated:

I added a setState(bool state) call that you can use to turn the display on or off.

The change exposes an existing command in the HT16K33 chip.

This does not affect existing code.

dhalbert commented 9 months ago

OK, looking at the blinkRate() documentation, it's just wrong.

jonnybergdahl commented 9 months ago

Except the code does not actually do that, it makes sure it is on by hardcoding the ON value in the payload.

uint8_t buffer = HT16K33_BLINK_CMD | HT16K33_BLINK_DISPLAYON | (b << 1);

Also - an on/off command does not really fit in a method named blinkRate, right?

I changed the comment to reflect what values you can actually use.

dhalbert commented 9 months ago

I was looking at this at midnight previously, sorry. My only comment then is that setState() is vague, since state could mean a lot of things.

I haven't found a really good name, but setDisplayOn(), setIlluminated, setDisplayState, are all candidates. Do you have a suggestion?

jonnybergdahl commented 9 months ago

Agreed. setDisplayState() sounds good to me.

jonnybergdahl commented 9 months ago

I am working on finalizing this - https://github.com/jonnybergdahl/Arduino_JBWopr_Library Found the brightness issue while doing the finals tests.