Closed hpreston closed 2 years ago
Hello @hpreston,
First of all thank you for the contribution, and the fact that the library is suitable for you waiting, however, I do not really understand how RGBLed::WHITE
can be in conflict with another arduino library since we use the name of the library to retrieve the value WHITE
, could you give me an example with the library in question?
If it is really a problem, I will put a BREAKING CHANGE in the description of the new version.
For brightness, it is indeed a good idea that I did not think of. And for the management of RGBW LEDs I am not against a contribution, because I do not have much time to spend at the moment.
@wilmouths thanks for the quick reply. I wasn't happy having to make a change that was breaking.
As I was putting together the details for you here, I actually found an alternative to fix the conflict that did NOT require changing the value here. I will update my PR to restore the value to RGBLed::WHITE
to remove this breaking change.
But, in case you are interested in what I ran into, here are more details:
Here is what I get when I try to compile when I have RGBLed::WHITE
defined.
Verifying...
In file included from /Users/hpreston/code/arduino/feather_led_strip/code.ino:9:
/Users/hpreston/Documents/Arduino/libraries/Adafruit_SSD1306/Adafruit_SSD1306.h:69:23: error: expected unqualified-id before numeric constant
69 | #define SSD1306_WHITE 1 ///< Draw 'on' pixels
| ^
/Users/hpreston/Documents/Arduino/libraries/Adafruit_SSD1306/Adafruit_SSD1306.h:64:15: note: in expansion of macro 'SSD1306_WHITE'
64 | #define WHITE SSD1306_WHITE ///< Draw 'on' pixels
| ^~~~~~~~~~~~~
/Users/hpreston/Documents/Arduino/libraries/RGBLed-hank/src/RGBLed.h:40:13: note: in expansion of macro 'WHITE'
40 | static int WHITE[3];
| ^~~~~
exit status 1
I interpreted the above output to indicate that the use of the name WHITE
in a #define
within the Adafruit_SSD1306
library as well as a public class attribute of the RGBLed
library is causing some collision. I would have expected the fact they they are contained within separate libraries to keep them isolated, but unfortunately that doesn't seem to be the case.
I am not using the RGBLed::White
in my actual code file. However I am using SSD1306_WHITE
in my code to set the color of the text on the OLED display I am using.
I dropped into GitHub code for the Adafruit library where the conflicting macro existed. It is here: https://github.com/adafruit/Adafruit_SSD1306/blob/master/Adafruit_SSD1306.h#L67
When I checked, I found the comment that the macro WHITE
is part of a set of macro names kept for compatibility. And they could be disabled before including the library. I added this to the top of my code file:
// Disable "old" color codes from library to avoid conflit with name WHITE
#define NO_ADAFRUIT_SSD1306_COLOR_COMPATIBILITY 1
And now when I try to verify my code I get NO error.
Okay, I have updated the PR, restoring the WHITE
name.
I did add a .gitignore
file as well. The only thing being ignored right now is the .vscode
directory so I don't mistakingly commit my IDE settings.
Thanks @hpreston for this, as this is a feature I was looking for also. Hope to see these changes merged as they bring really welcome improvements to an excellent library.
First, this is great Library. Exactly what I was hoping existed to save me time from having to start from scratch for my building of some LED light kits for my own use.
As I was working with it, I found I wanted to "store" the brightness level setting on the LED between changes. Kind of like a "master brightness knob" on a physical controller. Then any setting for a color would be automatically adjusted to the brightness level stored.
Most of what was needed for this already existed in the library, I just added in a new private attributed to the LED object for
_brightness
, and then updated the other methods to leverage theintensity
function instead ofcolor
, and use the_brigthness
property. This will make more sense when you look at the code submission.I have tested this change for the basic
setColor
,blink
, andfade
functions. And the existingbrightness
functions continue to work, and the setting forbrightness
is saved for use on later calls/changes. A newbrightness
method that just takes abrightness
value has been added to JUST set the brightness level.Another change you'll see in the PR is a rename of the
RGBLed::WHITE
toRGBLed::RGB_WHITE
. I made this change for 2 reasons. First, and most pressing is that the use of justWHITE
for the static conflicted with a defined macro in another Library I'm using to control an LED screen. That library usesWHITE
as the font color.I'm also looking at updating this library to support
RGBW
LEDs, so renaming theWHITE
color to indicate that it is created by combining RGB LEDs to create White vs a "native white LED" seemed a reasonable change.I hope you find this enhancement useful. If there are any changes you'd like to see before merging in, please just let me know.