Pages: [1]   Go Down
Author Topic: New Library: DycoLed for Arduino  (Read 2038 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 10
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi,

long time ago I got some nice rgb leds which can be connected in series. Data is sent in for all leds by a microcontroller (e.g. Arduino).
During the last weeks I spent some time to get this running.
Now I present to you the first version of my library to control these leds (see attachment).

Installation:
copy the DycoLed folder into ...\Arduino\libraries\ (where all custom libraries are)

Usage:
1. Import the library in your sketch
Code:
#include <DycoLed.h>

2. generate an object for your leds
Code:
DycoLed myLeds;

3. in setup(), initialize the object with begin()
Code:
void setup() {
  myLeds.begin(3, 10);  // three leds, 10 ms refresh rate
}

4. use setLed() with 3 colors or an byte array with three colors. Values between 0 and 31 can be used to set the PWM
Code:
// first led to red
  myLeds.setLed(0, RED);
  delay(500);

to reset all leds, use resetAll()
Code:
//reset all, second led green
  myLeds.resetAll();
  myLeds.setLed(1, GREEN);
  delay(500);

and so on...

At the moment this library only works on an ATtiny85, using the USI interface. But I will update it soon so you can use it with your Arduino board, other AT-chips or whatever.
Also there is a limitation to 32 leds. If you want to set this limit up, go to the header file of the lib and change this code line to your needs:
Code:
//########################################################
#define MAXLEDS 32
//########################################################

Please let me know if you have questions, bug reports and so on. Have fun smiley

* DycoLed.zip (4.09 KB - downloaded 18 times.)
Logged

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 220
Posts: 13846
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


Thanks for sharing,


just commenting on all what I see:
If things are unclear let me know.


Code:
if (_ledCount <= MAXLEDS && _ledCount >= 1) {
LEDCOUNT = _ledCount;
}
else {
LEDCOUNT = MAXLEDS;
}

so if I fill in 0 I get MAXLEDS?  Why???

[improve]
The type of _ledcount can be  (byte)  -- idem for setLED()

[missing]
It is not possible to set the pins, now you are using 1 and 2  where pin 1 is allready part of the serial port....

[missing]
no method to request the #leds or the interval set in the constructor.

[style]
The usage of capital like LEDCOUNT is in C++ primary used for #defines like you did MEXLEDS
for variables lowerCase is normally used

[bug 2x]
void DycoLed::setLed(unsigned int led, byte value[3]) {
   if (led <= LEDCOUNT) {

if LEDCOUNT == 1 the only valid number is 0

[typo]
claculating

[shorter]
Code:
                bitSet(tmp_ledMessage, 15);
// set each 'dyco color word' to specific color value
tmp_red   = dycoLed[i][r] << 10;       // shift it 10 bits, making 0000000000011111 now 0111110000000000.
tmp_green = dycoLed[i][g] <<  0;      // this is redundant but here for readability; stays as 0000000000000111.
tmp_blue  = dycoLed[i][b] <<  5;    // shift it 5 bits, making 0000000000001111 now 0000000111100000.

// merge color data to one word
tmp_ledMessage = tmp_ledMessage | tmp_red | tmp_blue | tmp_green;

==>

uint16_t  tmp_ledMessage = 1 << 15;

tmp_ledMessage |= dycoLed[i][r] << 10;
tmp_ledMessage |= dycoLed[i][g] <<  0;   
tmp_ledMessage |= dycoLed[i][b] <<  5;   

[improvement?]
no need for currentMmillis.
Code:
void DycoLed::send()
{
  if (millis()- previousMillis > interval)
  {
     previousMillis = millis();
     sendNow();
   }
}


Code:
// send data for black
for (int i=0; i<256; i++) {
spi_transfer(B10000000);
spi_transfer(B00000000);
}
WHY 256 ???  LEDCOUNT should do

[idea]
I would add a boolean called changed; in the class that is set to true if setLed() was called and reset if values were written. If changed == false you dont need to send all the data as it was not changed.

I would merge clearAll() and resetAll() to
Code:
void DycoLed::clearAll()
{
  for(int i=0; i<LEDCOUNT; i++)
  {
    dycoLed[i][0] = 0;
    dycoLed[i][2] = 0;
    dycoLed[i][1] = 0;
  }
  update();
  sendNow();
}


is sendNow() missing in the constructor?


You have a double administration one for easy setting and one for sending.
You can win 60+bytes footprint by removing the ledMessage Array's. You could then also remove the complete update() function.

sendNow() could become something like

[code]
void DycoLed::sendNow() {
// send start data
initLeds();

// send color data
for (int i=0; i<LEDCOUNT; i++)
        {
             uint16_t ledMessage = 1 << 15;

             ledMessage |= dycoLed[i][r] << 10;
             ledMessage |= dycoLed[i][g] <<  0;   
             ledMessage |= dycoLed[i][b] <<  5; 

spi_transfer( highByte(ledMessage) );
spi_transfer( lowByte(ledMessage) );
}

// send stop data (all leds change color to sent value)
assumeData();
}

missing the function getLed(&r &g & b);

Hope this helps to improve your library!

[/code]
Logged

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Offline Offline
Newbie
*
Karma: 0
Posts: 10
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hello robtillaart, thank you for the review of my code. Here are the answers you are (hopefully) looking for smiley-wink

Quote
so if I fill in 0 I get MAXLEDS?  Why???
The answer is easy. What sense would it make to control zero leds? I agree with you that MAXLEDS is a bit oversized, could change this to 1 or leave it like so.

Quote
[improve]
The type of _ledcount can be  (byte)  -- idem for setLED()
Agreed.

Quote
[missing]
It is not possible to set the pins, now you are using 1 and 2  where pin 1 is allready part of the serial port....
This is the price of an early release ... I've tested the code only on one ATtiny85 right now. Last week I changed from software bit-banging to hardware output (USI). In a later (the next?) version of the library I will add support for real SPI and software mode.

Quote
[missing]
no method to request the #leds or the interval set in the constructor.
"get...()" methods will be added. The thing with the interval is not fully featured at the moment (delay() does it's job, but not very efficient).

Quote
[style]
The usage of capital like LEDCOUNT is in C++ primary used for #defines like you did MEXLEDS
for variables lowerCase is normally used
Tried to get the quantity of leds by template, but it didn't work out. Therefore I set this as a #define some days ago an didn't change it back after the usage as a variable. Is fixed.

Quote
[bug 2x]
void DycoLed::setLed(unsigned int led, byte value[3]) {
   if (led <= LEDCOUNT) {

if LEDCOUNT == 1 the only valid number is 0
Not right, 1 would be true, too. (smaller or equal to LEDCOUNT)

Quote
[shorter]
void DycoLed::update() { .........
The shorter version does it's job very well smiley Was so long during testing the basic commands for the leds, temp vars are not needed anymore.

Quote
begin() ...
WHY 256 ???  LEDCOUNT should do
Had a problem whlie testing the leds. I changed the quantitiy to a smaller one than connected to my ATtiny85 ... the DycoLeds have the "feature" that they go to maximum brightness at start ... without initialization, all leds without this first command would drain 60 mA (for each led). Therefore this huge initialization in begin()

Quote
[idea]
I would add a boolean called changed; in the class that is set to true if setLed() was called and reset if values were written. If changed == false you dont need to send all the data as it was not changed.
Same as for the interval ... will be added when usage without delay() is fully featured.

Quote
I would merge clearAll() and resetAll() to
Not sure why I left both in the code ... maybe it is a remain from testing the code. clearAll() could be useful for a fluid color changing. resetAll() may be used for shutting down everything. Will search for the reason in my notes.

Quote
You have a double administration one for easy setting and one for sending.
You can win 60+bytes footprint by removing the ledMessage Array's. You could then also remove the complete update() function.
Yes and no smiley-wink
I had the just-in-time calculation in a very early stage of writing the code.  But I thougt that pre-calculating all values and just sending them out at the desired time would be better. I will think about this again later.

Quote
missing the function getLed(&r &g & b);
What do you mean exactly? To get the values of a specific led ( getLed(byte led) with an array return)? Or to search for one led with the matching color values (returning index of led)?
Logged

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 220
Posts: 13846
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

responding only at some issues,

Quote
The answer is easy. What sense would it make to control zero leds? I agree with you that MAXLEDS is a bit oversized, could change this to 1 or leave it like so.
I would make it explicit zero because this value is used for testing in other functions. setLEd() in particular


Quote
It is not possible to set the pins, now you are using 1 and 2  where pin 1 is allready part of the serial port....
This is the price of an early release ... I've tested the code only on one ATtiny85 right now. Last week I changed from software bit-banging to hardware output (USI). In a later (the next?) version of the library I will add support for real SPI and software mode.
OK, fair enough - however you can easily create a constructor with params and set these default to 1 and 2 (so UNO people can overrule)

Quote
[bug 2x]
void DycoLed::setLed(unsigned int led, byte value[3]) {
   if (led <= LEDCOUNT) {
if LEDCOUNT == 1 the only valid number is 0
Not right, 1 would be true, too. (smaller or equal to LEDCOUNT)

disagree if you fill in MAXLEDS in the constructor that would be accepted, but your internal arrays go from 0..Maxleds-1  (read the chapters about arrays in C/C++) That would be one missing.


Quote
You have a double administration one for easy setting and one for sending.
You can win 60+bytes footprint by removing the ledMessage Array's. You could then also remove the complete update() function.
Yes and no smiley-wink
I had the just-in-time calculation in a very early stage of writing the code.  But I thougt that pre-calculating all values and just sending them out at the desired time would be better. I will think about this again later.
You can also precalculate the values in setLed so that the both arrays are allways ready to be send.


Quote
issing the function getLed(&r &g & b);
What do you mean exactly? To get the values of a specific led ( getLed(byte led) with an array return)? Or to search for one led with the matching color values (returning index of led)?
I would like to request the color back from the LED.  The array holds the value so why should I keep it.


Request for a new function
AddLed(r,g,b);
shifts all LED values in the internal array and places this RGB at index 0.  Easy to create a moving light show !






Logged

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Offline Offline
Newbie
*
Karma: 0
Posts: 10
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
The answer is easy. What sense would it make to control zero leds? I agree with you that MAXLEDS is a bit oversized, could change this to 1 or leave it like so.
I would make it explicit zero because this value is used for testing in other functions. setLEd() in particular
Agreed.


Quote
It is not possible to set the pins, now you are using 1 and 2  where pin 1 is allready part of the serial port....
This is the price of an early release ... I've tested the code only on one ATtiny85 right now. Last week I changed from software bit-banging to hardware output (USI). In a later (the next?) version of the library I will add support for real SPI and software mode.
OK, fair enough - however you can easily create a constructor with params and set these default to 1 and 2 (so UNO people can overrule)
Ok, is in work. I will change the default constructor to use the SPI/USI pins of the AVR (which should not be a problem afaik due to pin mapping lists and the preprocessor). A second constructor will be added to set the software mode:
Code:
DycoLed::DycoLed(byte _dataPin, byte _clockPin) {
  // set data pin to _dataPin
  // same to clock pin
}


Quote
[bug 2x]
void DycoLed::setLed(unsigned int led, byte value[3]) {
   if (led <= LEDCOUNT) {
if LEDCOUNT == 1 the only valid number is 0
Not right, 1 would be true, too. (smaller or equal to LEDCOUNT)

disagree if you fill in MAXLEDS in the constructor that would be accepted, but your internal arrays go from 0..Maxleds-1  (read the chapters about arrays in C/C++) That would be one missing.
Now I see what you mean, sorry for the late comprehension smiley
I would change "led <= ledCount" to "led < ledCount" because LED0 would be smaller than, let's say ledCount=1, LED1 wouldn't be allowed to be set/read. In the example sketch of the library the index for the first led is 0 and it works perfect with ledCount set to 1.


Quote
You have a double administration one for easy setting and one for sending.
You can win 60+bytes footprint by removing the ledMessage Array's. You could then also remove the complete update() function.
Yes and no smiley-wink
I had the just-in-time calculation in a very early stage of writing the code.  But I thougt that pre-calculating all values and just sending them out at the desired time would be better. I will think about this again later.
You can also precalculate the values in setLed so that the both arrays are allways ready to be send.
Would work fine for setting a color, but would make it more difficult to read a color from the two output arrays with the 2x 8 bit values. For each reading you would have to calculate it back to the tree-byte format like I write it into the two output arrays.
On the other hand it could save some time when shifting the color information back and forth (see bottom of this post).


Quote
issing the function getLed(&r &g & b);
What do you mean exactly? To get the values of a specific led ( getLed(byte led) with an array return)? Or to search for one led with the matching color values (returning index of led)?
I would like to request the color back from the LED.  The array holds the value so why should I keep it.
Will be added. In my opinion this will become important when you calculate more complex color effects for the leds. Was not important for the first release where I only set a color smiley-wink


Request for a new function
AddLed(r,g,b);
shifts all LED values in the internal array and places this RGB at index 0.  Easy to create a moving light show !
I admit this request and would go one level higher smiley What about a function like this:

Code:
void DycoLeds::shiftLeds(boolean direction, byte steps) {
  // shift color information x steps in direction y, for example
  // direction = 0, steps = 1 would make this with three connected leds:
  // led 2 looses it's color information and get's the color from led 1,
  // led 1 get's the color from led 0,
  // led 0 becomes black
}

// call setLed(0, newRed, newGreen, newBlue) to give the first led a new color
//...

//same for direction = 1 in shiftLeds(), but now the last led get's a new color.


I have more ideas for effects. For example:


LED0    LED1    LED2    LED3    LED4    LED5    LED6    LED7    LED8    LED9 ...
red                                             blue

A function like mergeLeds(byte firstLed, byte lastLed) could calculate the crossover colors for LED1 to LED6 if the first and last led are given to this function (LED0 and LED7). If you "move" LED6 to the left or right and let the function calculate the new leds in between, this could look nice, too smiley

Or something like getting a full set of colors from another controller or a computer and fade to this in a certain amount of time. Would need a second color array, but could be useful.


Any other ideas?


PS: while writing in the forum, I added a function called end() which does nothing more than call resetAll() and set isInit to false (for a new initialization later on).
« Last Edit: May 10, 2012, 07:48:09 pm by DeadDealer » Logged

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 220
Posts: 13846
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
A function like mergeLeds(byte firstLed, byte lastLed) could calculate the crossover colors for LED1 to LED6 if the first and last led are given to this function (LED0 and LED7). If you "move" LED6 to the left or right and let the function calculate the new leds in between, this could look nice, too smiley

yeah, morphing colors - can be done easily with linear interpolation with the map() function smiley

with respect to the double array's Think you should make one without the high low arrays and keep the RGB array intact.
I expect that just in time merging is fast enough in practice.

It would make the lib simpler too as you can drop the update() functions.

OTher ideas:
- knightRider sketch
- test() to test complete led array R 0..255 G 0..255 B0..255 ==> better make that a testsketch.pde
- binary counter sketch. 32 leds can just represent the 32 bits in a long...
- ...

Logged

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Offline Offline
Newbie
*
Karma: 0
Posts: 10
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Here comes version 1.2 of the lib (1.1 was not published due to heavy bug fixing and new functions)

At the moment 1.2 has ONLY SOFTWARE MODE. There is a bug in the hardware support (which worked with 1.0 and 1.1). For testing the leds it should be enough. Please feel free to test the new demo sketch which has the new functions.

In my opinion the lib now has everything it needs. There will be a little bit speed improvement and bug search, but I think there are enough functions for a "basic" library.

Here a test video with the demo sketch:


Have fun smiley

* DycoLed_1.2.zip (4.97 KB - downloaded 17 times.)
« Last Edit: May 14, 2012, 12:50:05 pm by DeadDealer » Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 10
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Fixed a little bug where color arrays like
Code:
byte RED[] = {31, 0, 0};
were read wrong. The normal color setter setLed(led, r, g, b) worked correctly.

Also the hardware mode for the ATtiny85 works again.

Updated the lib to 1.2.1.

* DycoLed_1.2.1.zip (5.12 KB - downloaded 33 times.)
Logged

Pages: [1]   Go Up
Jump to: