Go Down

Topic: A new library to control leds with your Arduino (Read 482 times) previous topic - next topic

BiagioMkr

Hi all,
I have made a new library to control one or more leds attached to an Arduino board.
Use this link to go to the Arduino-Led library repo.
You can download the library by using the Clone or download button on the Github page and to use the library extract its source code from the zipped tarball, rename it to Arduino-Led and place it in your libraries directory inside your Arduino sketch folder.
You can find some examples that use the library APIs in the examples directory inside the library directory and to get more info about the library methods see the Led.cpp file.
If you have suggestion to improve the library or if you find any problem, write them under this post.

septillion

One thing I already notice, the library should have the same name as the library. So simply Led.h is asking for trouble... So call it Arduino-Led :)

And blinking a led with delay() is the first thing newbies should stop doing, let alone grab a library that does that...

And a big error
Code: [Select]

  SERIAL.print("Led connected to pin ");

Will cause an error when SERIAL_DEBUG isn't defined (instead of not printing it). A way to do it is like:
Code: [Select]

#if defined(SERIAL_DEBUG)
  #define DEBUG_PRINT(x) Serial.print(F(x))
  #define DEBUG_PRINTLN(x) Serial.println(F(x))
#else
  #define DEBUG_PRINT(x)
  #define DEBUG_PRINTLN(x)
#endif


You could have save yourself a lot of typing..
Code: [Select]

if(initState == ON) {
if(pull == PULL_DOWN)
_pinState = HIGH;
else
_pinState = LOW;

_state = initState;
_brightness = checkBrightness(_state);
}
else if(initState == OFF) {
if(pull == PULL_DOWN)
_pinState = LOW;
else
_pinState = HIGH;

_state = initState;
_brightness = checkBrightness(_state);
}
else {
_pinState = LOW;
_state = WRONG_STATE;
_brightness = 0;
}

==>
Code: [Select]

if(initState == WRONG_STATE){
  _pinState = LOW;
_state = WRONG_STATE;
_brightness = 0;
}
else{
  _pinState = initState ^ pull; //pull is a weird variable name...
  _state = initState;
  _brightness = checkBrightness(_state);
}


And you could save more by simply removing the WRONG_STATE from the enum... And assume every state other then OFF to be ON.
Code: [Select]

_pinState = !!initState ^ pull; //pull is a weird variable name...
_state = initState;
_brightness = checkBrightness(_state);


PS. Damn, heavy indentation :p
Use fricking code tags!!!!
I want x => I would like x, I need help => I would like help, Need fast => Go and pay someone to do the job...

NEW Library to make fading leds a piece of cake
https://github.com/septillion-git/FadeLed

BiagioMkr

Hi septillion,
thanks a lot for your review. Regarding your comment about the use of SERIAL macro, if you look at the Led.h file you can see that if SERIAL_DEBUG is not defined, the SERIAL macro and the methods of Led class that use Serial communication are not defined so that is not an error.
I have updated the methods documentation, the constructor and the library name.
If you want you can test the new library version

septillion

I have updated [...] the library name.
You indeed did. But the wrong way around... A library name should not have a name that is very generic. Because you can't have two libraries with the same name and the changes are pretty big somebody else made the same mistake and gave a library a generic name as Led. Or pick a more creative name or prefix it, like Biagiom-Led.

Regarding your comment about the use of SERIAL macro, if you look at the Led.h file you can see that if SERIAL_DEBUG is not defined, the SERIAL macro and the methods of Led class that use Serial communication are not defined so that is not an error.
I saw the define etc. But what I missed is that you only used the SERIAL macro in #ifdef blocks. I was just looking at it in GitHub, not an editor. As long as you do that it will indeed work. I thought the SERIAL.print() where just in the normal code. Then it would not work of course but my method would.

But not to be rude, a library that uses delay() to blink a led (ignoring the overhead) is not very useful in my opinion.

PS What you call pull(Type) has nothing to do with pulling. It's the driveType ;) High side or low side switching.
PSS Like the Doxygen documentation :)
Use fricking code tags!!!!
I want x => I would like x, I need help => I would like help, Need fast => Go and pay someone to do the job...

NEW Library to make fading leds a piece of cake
https://github.com/septillion-git/FadeLed

BiagioMkr

Hi septillion,
thanks for your suggestions. I decided to call the library as Arduino-Led so I have updated the documentation, the class name and all the code that used to the old library name.
Best regards

Go Up