ATTiny84 I2C with NeoPixels not working

Question

I fetch RGB values via I2C - which works (with a splitting function).

Now I have a problem:

• When I write values directly into strip.color() like strip.Color(255,0,0,0) the NeoPixels turn red when receiving the values.
• When I use the values from the I2C connection it won't work anymore. But the values are correct if I have a look in the Serial Monitor.

Maybe the data type is wrong or something?

#include <Wire.h>
#include <Adafruit_NeoPixel.h>
#define LED_PIN 4
#define LED_COUNT 4
static const int SLAVE_ADDRESS = 0x08;
uint32_t currentColor;
int currentBrightness = 50;
int flashState = 0;
String ledRed, ledGreen, ledBlue, ledWhite;
String temp;
String payload;
Adafruit_NeoPixel strip(LED_COUNT, LED_PIN, NEO_GRBW + NEO_KHZ800);
void setup() {
 Serial.begin(9600);
 strip.begin(); 
 strip.show(); // Turn off all pixels asap
 strip.setBrightness(50); // Set brightness
 
 pinMode(3, OUTPUT);
 digitalWrite(3, HIGH);
 Wire.begin(SLAVE_ADDRESS);
 Wire.onReceive(change);
}
// Set all pixels to color
void setPixelColors(uint32_t color) { 
 for(int p = 0; p < LED_COUNT; p++) {
 strip.setPixelColor(p, color);
 }
 strip.show();
 delay(1000);
 digitalWrite(3, HIGH);
}
// Split the I2C message
String split(String s, char parser, int index) {
 String rs="";
 int parserIndex = index;
 int parserCnt=0;
 int rFromIndex=0, rToIndex=-1;
 while (index >= parserCnt) {
 rFromIndex = rToIndex+1;
 rToIndex = s.indexOf(parser,rFromIndex);
 if (index == parserCnt) {
 if (rToIndex == 0 || rToIndex == -1) return "";
 return s.substring(rFromIndex,rToIndex);
 } else parserCnt++;
 }
 return rs;
}
void loop() {
 // ...
}
void change(int howMany) {
 digitalWrite(3, LOW);
 temp = "";
 if (Wire.available()) {
 // Fetch the string
 while(Wire.available()) {
 char c = Wire.read();
 temp.concat(c);
 }
 payload = String(temp);
 flashState = split(payload, ',', 0).toInt();
 ledRed = split(payload, ',', 1);
 ledGreen = split(payload, ',', 2);
 ledBlue = split(payload, ',', 3);
 ledWhite = split(payload, ',', 4);
 byte bufRed[3];
 ledRed.toCharArray(bufRed, 3);
 
 byte bufGreen[3];
 ledGreen.toCharArray(bufGreen, 3);
 byte bufBlue[3];
 ledBlue.toCharArray(bufBlue, 3);
 
 byte bufWhite[3];
 ledWhite.toCharArray(bufWhite, 3);
 
 //setPixelColors(strip.Color(255, 0, 255, 0)); // WORKING
 
 currentColor = strip.Color(bufRed, bufGreen, bufBlue, bufWhite);
 setPixelColors(currentColor); // NOT WORKING
 }
 
 // Proof the I2C reading is finished
 delay(10);
}

I tried different data types, attached a status LED so I know it the I2C connection is working. When I test it with my UNO I get for R=255 G=0 B=255 W=0; like intended.

"Fun fact": On my UNO it works like a charm. It's just the ATTiny85 that won't accept that values(?)

---

New Code:

#include <Wire.h>
#include <Adafruit_NeoPixel.h>
#define LED_PIN 4
#define LED_COUNT 20
static const int SLAVE_ADDRESS = 0x08;
int currentBrightness = 50;
int flashState = 0;
int ledRed, ledGreen, ledBlue, ledWhite;
volatile bool recvFlag = false;
volatile uint32_t currentColor = 0;
String temp;
String payload;
Adafruit_NeoPixel strip(LED_COUNT, LED_PIN, NEO_GRBW + NEO_KHZ800);
void setup() {
 Serial.begin(9600);
 strip.begin();
 strip.show(); // Turn OFF all pixels ASAP
 strip.setBrightness(50); // Set BRIGHTNESS to about 1/5 (max = 255)
 Wire.begin(SLAVE_ADDRESS);
 Wire.onReceive(change);
}
void setPixelColors(uint32_t color) {
 for (int p = 0; p < LED_COUNT; p++) {
 strip.setPixelColor(p, color);
 strip.show();
 }
}
String split(String s, char parser, int index) {
 String rs = "";
 int parserIndex = index;
 int parserCnt = 0;
 int rFromIndex = 0, rToIndex = -1;
 while (index >= parserCnt) {
 rFromIndex = rToIndex + 1;
 rToIndex = s.indexOf(parser, rFromIndex);
 if (index == parserCnt) {
 if (rToIndex == 0 || rToIndex == -1) return "";
 return s.substring(rFromIndex, rToIndex);
 } else parserCnt++;
 }
 return rs;
}
void loop() {
 if (recvFlag) { 
 payload = String(temp);
 flashState = split(payload, ',', 0).toInt();
 ledRed = split(payload, ',', 1).toInt();
 ledGreen = split(payload, ',', 2).toInt();
 ledBlue = split(payload, ',', 3).toInt();
 ledWhite = split(payload, ',', 4).toInt();
 currentColor = strip.Color(ledRed, ledGreen, ledBlue, ledWhite);
 setPixelColors(currentColor);
 
 recvFlag = false;
 }
}
void change(int howMany) {
 temp = "";
 if (Wire.available()) {
 // Fetch the string
 while (Wire.available()) {
 char c = Wire.read();
 temp.concat(c);
 }
 // Proof the I2C reading is finished
 recvFlag = true;
 }
}

Answer 1

Honestly I don't know, why exactly this works on your Uno. You need to know, that the onReceive callback function is called from an ISR (Interrupt Service Routine). And inside an ISR you cannot do things, that rely on interrupts to work (like delay()), since only one interrupt can run at a time. I'm not sure, how the Neopixel library works in its core, but its probably not a good idea to run the show() method inside an ISR. Also generally you don't want to do long things inside an ISR, since it will block all the other interrupt powered functions of your microcontroller (for example I2C, Serial, time keeping via delay() or millis()...).

Instead the best practice is to only set a flag and then do these things in the main code (aka in loop()). So something like this:

volatile bool recv_flag = false;
volatile uint32_t current_color = 0;
void setup(){
 // Initiating everything like you already did in your code
 ...
}
void loop(){
 if(recv_flag){
 setPixelColors(currentColor);
 recv_flag = false;
 }
}
void change(int howMany) {
 digitalWrite(3, LOW);
 temp = "";
 if (Wire.available()) {
 // Doing all the data parsing like you already did
 ...
 // save current color in global variable
 currentColor = strip.Color(bufRed, bufGreen, bufBlue, bufWhite);
 // set flag
 recv_flag = true;
 }
}

Note that every variable, that will be written to inside an ISR should be declared as volatile. This prevents the compiler from optimizing it out, since it cannot know when the interrupts will happen. So it might think that this variable will never change and just replace it with a literal. The volatile keyword prevents that.

---

As mentioned above I'm even wondering, why your code works on the Uno. Though the main difference between the Uno and the Attiny84 is, that the Uno as a full fledged I2C hardware interface, while the Attiny84 only has an USI (Universal Serial interface). So the Uno has most of the I2C communication implemented in hardware, while the Attiny84 needs to run (probably interrupt based) code to do the communication.

• dessi: I got it to work very shortly with 4 LEDs. Thought that was it. But then I tried with 20 LEDs - nothing happend again. I tried the example with "colorWipe" from the library. This works - so there is enough power and every LED is working.... And my API+Splitting has be working to because it worked for 2-3 tests before adding more LEDs. Any other ideas why THIS is happening now? :/
• chrisl: @dessi You can add the new code to your question. Then I will see, if I can help you with that. But please leave the current question text intact, so that others can learn from it
• dessi: just updated the question with the new code
• chrisl: @dessi All in all your code looks good. You should declare temp as volatile, since you are writing to it inside the ISR. But I don't think this is the issue. Can you be sure, that the I2C communication works like intended? For example by lighting and LED, of the correct data was received? And can you check, if the code blocks at some point? If yes, where exactly does it block? Or does the code run, but it has no effect? That would be important information to debug further.
• chrisl: Maybe you get an issue with all the String use. It used dynamic memory allocation, which can make swiss cheese out of your memory (for reference Majenkos blog entry about this. Though that should only be a problem after some more I2C transactions.