Blink not working

Question

I have written this code to write and receive signals over i2c between two Uno's. I have a master and a slave code, this is working fine. Now I want the led to blink, for this I added the blink without delay example. Unfortunately I can't get it working at all...

// Wire Slave Receiver

#include <Wire.h> //include Library
// define Led Pins
#define LED1 3 //Pin 3
#define LED2 4 //Pin 4
#define LED3 5 //Pin 5

int ledState = LOW;             // ledState used to set the LED
unsigned long previousMillis = 0;        // will store last time LED was updated
const long interval = 500;           // interval at which to blink (milliseconds)

void setup()
{
  Wire.begin(4);                // join i2c bus with address #4
  Wire.onReceive(receiveEvent); // register event
  Serial.begin(9600);           // start serial for output

  //set Pin modes
  pinMode(LED1, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(LED3, OUTPUT);
  //intern pulldown
  digitalWrite(LED1, LOW);
  digitalWrite(LED2, LOW);
  digitalWrite(LED3, LOW);

}

void loop()
{
  unsigned long currentMillis = millis();
  delay(100);
}

// function that executes whenever data is received from master
// this function is registered as an event, see setup()
void receiveEvent(int howMany)
{
  while (1 < Wire.available()) // loop through all but the last
  {

    char c = Wire.read(); // receive byte as a character
    Serial.print(c, BIN);     // print the character as Binary


  }
  int x = Wire.read();    // receive byte as an integer
  Serial.println(x, BIN); // print the integer as Binary


  if (x == 1) { //turn LED1 on
    digitalWrite(LED2, LOW);
    digitalWrite(LED3, LOW);
    digitalWrite(LED1, HIGH);
    Serial.println("LED1 on");
  }
  if (x == 2) { //blink LED1
    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval) {
      previousMillis = currentMillis;
      if (ledState == LOW) {
        ledState = HIGH;
      } else {
        ledState = LOW;
      }
      digitalWrite(LED1, ledState);
    }
  }

  if (x == 3) { //turn LED2 on
    digitalWrite(LED1, LOW);
    digitalWrite(LED2, HIGH);
    delay(100);
  }
  if (x == 4) { //blink LED2
    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval) {
      previousMillis = currentMillis;
      if (ledState == LOW) {
        ledState = HIGH;
      } else {
        ledState = LOW;
      }
      digitalWrite(LED2, ledState);
    }
  }
  if (x == 5) { //turn LED3 on
    digitalWrite(LED2, LOW);
    digitalWrite(LED3, HIGH);
  }
  if (x == 6) { //blink LED3
    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval) {
      previousMillis = currentMillis;
      if (ledState == LOW) {
        ledState = HIGH;
      } else {
        ledState = LOW;
      }
      digitalWrite(LED3, ledState);
    }
  }
  if (x == 7) { //turn LED1 nad 2 on
    digitalWrite(LED3, LOW);
    digitalWrite(LED1, HIGH);
    digitalWrite(LED2, HIGH);
  }
  if (x == 8) { //Blink LED1 and 2
    digitalWrite(LED1, LOW);
    digitalWrite(LED2, LOW);
  }
  if (x == 9) { //turn LED2 and 3 on
    digitalWrite(LED1, LOW);
    digitalWrite(LED2, LOW);

    digitalWrite(LED2, HIGH);
    digitalWrite(LED3, HIGH);
  }

} 

---

Thanks for your response. I tried to put all the of statements into the loop, but it's still not working. :P I have no idea how to put it into the loop so it works.

// Wire Slave Receiver

#include <Wire.h> //include Library
// define Led Pins
#define LED1 3 //Pin 3
#define LED2 4 //Pin 4
#define LED3 5 //Pin 5

int ledState = LOW;             // ledState used to set the LED
unsigned long previousMillis = 0;        // will store last time LED was updated
const long interval = 500;           // interval at which to blink (milliseconds)

void setup()
{
  Wire.begin(4);                // join i2c bus with address #4
  Wire.onReceive(receiveEvent); // register event
  Serial.begin(9600);           // start serial for output

  //set Pin modes
  pinMode(LED1, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(LED3, OUTPUT);
  //intern pulldown
  digitalWrite(LED1, LOW);
  digitalWrite(LED2, LOW);
  digitalWrite(LED3, LOW);

}

void loop()
{
    if (x == 1) { //turn LED1 on
    digitalWrite(LED2, LOW);
    digitalWrite(LED3, LOW);
    digitalWrite(LED1, HIGH);
    Serial.println("LED1 on");
  }
  if (x == 2) { //blink LED1
    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval) {
      previousMillis = currentMillis;
      if (ledState == LOW) {
        ledState = HIGH;
      } else {
        ledState = LOW;
      }
      digitalWrite(LED1, ledState);
    }
  }

  if (x == 3) { //turn LED2 on
    digitalWrite(LED1, LOW);
    digitalWrite(LED2, HIGH);
    delay(100);
  }
  if (x == 4) { //blink LED2
    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval) {
      previousMillis = currentMillis;
      if (ledState == LOW) {
        ledState = HIGH;
      } else {
        ledState = LOW;
      }
      digitalWrite(LED2, ledState);
    }
  }
  if (x == 5) { //turn LED3 on
    digitalWrite(LED2, LOW);
    digitalWrite(LED3, HIGH);
  }
  if (x == 6) { //blink LED3
    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval) {
      previousMillis = currentMillis;
      if (ledState == LOW) {
        ledState = HIGH;
      } else {
        ledState = LOW;
      }
      digitalWrite(LED3, ledState);
    }
  }
  if (x == 7) { //turn LED1 nad 2 on
    digitalWrite(LED3, LOW);
    digitalWrite(LED1, HIGH);
    digitalWrite(LED2, HIGH);
  }
  if (x == 8) { //Blink LED1 and 2
    digitalWrite(LED1, LOW);
    digitalWrite(LED2, LOW);
  }
  if (x == 9) { //turn LED2 and 3 on
    digitalWrite(LED1, LOW);
    digitalWrite(LED2, LOW);

    digitalWrite(LED2, HIGH);
    digitalWrite(LED3, HIGH);
  }
  unsigned long currentMillis = millis();
  delay(100);
}

// function that executes whenever data is received from master
// this function is registered as an event, see setup()
void receiveEvent(int howMany)
{
  while (1 < Wire.available()) // loop through all but the last
  {

    char c = Wire.read(); // receive byte as a character
    Serial.print(c, BIN);     // print the character as Binary


  }
  int x = Wire.read();    // receive byte as an integer
  Serial.println(x, BIN); // print the integer as Binary




}

Answer 1

You are misinterpreting how the onReceive callback works. The millis() code like in the BlinkWithoutDelay example relies on the fact, that it is run repeatedly very fast, so that effectively the time is checked regularily very fast.

The onReceive callback is only called once every time, that the Wire library received an I2C message, from an interrupt service routine. It is not called repeatedly. So the millis() code in there, will only be executed once. Since no time has passed (and millis() wouldn't increment inside an ISR either way) nothing happens there and the callback exits. After that the code is not executed further times.

You need to put the millis() code into the loop() function, so that it is executed repeatedly. In fact you can put all the if statements with x into the loop() function.

---

EDIT:

Your second version of the code should result in compilation errors (which you really should have included into your question). They will say, that x is not declared in this scope. You read the I2C data from the buffer in the onReceive callback and save it into a newly created variable x. Since x is declared inside the function, it will cease to exist right after the end of this function. The loop() function does not know about x. You need to define x globally outside of any function at the start of your sketch.

This results in this code:

// Wire Slave Receiver

#include <Wire.h> //include Library
// define Led Pins
#define LED1 3 //Pin 3
#define LED2 4 //Pin 4
#define LED3 5 //Pin 5

uint8_t x=0;                    // define x at global scope, so that it is available everywhere
int ledState = LOW;             // ledState used to set the LED
unsigned long previousMillis = 0;        // will store last time LED was updated
const long interval = 500;           // interval at which to blink (milliseconds)

void setup()
{
  Wire.begin(4);                // join i2c bus with address #4
  Wire.onReceive(receiveEvent); // register event
  Serial.begin(9600);           // start serial for output

  //set Pin modes
  pinMode(LED1, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(LED3, OUTPUT);
  //intern pulldown
  digitalWrite(LED1, LOW);
  digitalWrite(LED2, LOW);
  digitalWrite(LED3, LOW);

}

void loop()
{
    if (x == 1) { //turn LED1 on
    digitalWrite(LED2, LOW);
    digitalWrite(LED3, LOW);
    digitalWrite(LED1, HIGH);
    Serial.println("LED1 on");
  }
  if (x == 2) { //blink LED1
    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval) {
      previousMillis = currentMillis;
      if (ledState == LOW) {
        ledState = HIGH;
      } else {
        ledState = LOW;
      }
      digitalWrite(LED1, ledState);
    }
  }

  if (x == 3) { //turn LED2 on
    digitalWrite(LED1, LOW);
    digitalWrite(LED2, HIGH);
    delay(100);
  }
  if (x == 4) { //blink LED2
    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval) {
      previousMillis = currentMillis;
      if (ledState == LOW) {
        ledState = HIGH;
      } else {
        ledState = LOW;
      }
      digitalWrite(LED2, ledState);
    }
  }
  if (x == 5) { //turn LED3 on
    digitalWrite(LED2, LOW);
    digitalWrite(LED3, HIGH);
  }
  if (x == 6) { //blink LED3
    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval) {
      previousMillis = currentMillis;
      if (ledState == LOW) {
        ledState = HIGH;
      } else {
        ledState = LOW;
      }
      digitalWrite(LED3, ledState);
    }
  }
  if (x == 7) { //turn LED1 nad 2 on
    digitalWrite(LED3, LOW);
    digitalWrite(LED1, HIGH);
    digitalWrite(LED2, HIGH);
  }
  if (x == 8) { //Blink LED1 and 2
    digitalWrite(LED1, LOW);
    digitalWrite(LED2, LOW);
  }
  if (x == 9) { //turn LED2 and 3 on
    digitalWrite(LED1, LOW);
    digitalWrite(LED2, LOW);

    digitalWrite(LED2, HIGH);
    digitalWrite(LED3, HIGH);
  }
  unsigned long currentMillis = millis();
  delay(100);
}

// function that executes whenever data is received from master
// this function is registered as an event, see setup()
void receiveEvent(int howMany)
{
  while (1 < Wire.available()) // loop through all but the last
  {
    char c = Wire.read(); // receive byte as a character
    Serial.print(c, BIN);     // print the character as Binary
  }
  x = Wire.read();    // receive byte as an integer
  Serial.println(x, BIN); // print the integer as Binary
}

Note, that I changed the type of x to uint8_t, an unsigned int of 8 bits. Restricting x to 8 bits prevents problems with writing a variable inside an ISR. The Uno has an 8-bit microcontroller, so it handles 8 bits at a time. int has 16 bits and is thus handled in two steps. An interrupt can occur at any time, so the ISR might write x in the middle of a calculation, which results in a totally garbled value of x. Handling a 1 byte/8 bits variable cannot be interrupted (also called "atomic") and prevents this issue.