New Library: DycoLed for Arduino

Projects Libraries
2

Thanks for sharing,

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

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.

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]

                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.

void DycoLed::send() 
{
  if (millis()- previousMillis > interval) 
  {
     previousMillis = millis();
     sendNow();
   }
}

	// 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

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]