1

I have a number of LED's connected to an Arduino which receives a JSON dict.

The JSON dict determines which action the LED's follow (either a fade, a pulse, on, or off), but I'm having trouble working out how to get one LED to flash whilst another fades etc.

The important parts of the code looks like this:

int led_choice;
int led_brightness;
String pattern_name;
int pattern_length;

// Timer Setup
unsigned long led1_current_time;
unsigned long led2_current_time;
unsigned long previousFadeMillis[2];
unsigned long previousPulseMillis[2];
unsigned long current_time;

// fade setup
int fadeInterval = 50;
int fadeIncrement = 5;
int fadeDirection = 1;
int fadeValue = 0;
int maxPWM = 255;
int minPWM = 0;
int translated_pin;

// Pulse setup
int pulse_state = 0;

void setup() {
  pinMode(BUILTIN_LED, OUTPUT);     // Initialize the BUILTIN_LED pin as an output
  Serial.begin(115200);
}

void loop() {

  current_time = millis();

  // CODE HERE TO READ SERIAL/MQTT, DECIPHER JSON AND EXTRACT VARIABLES

  if ( pattern_name == "fade"){
    fade(current_time, pattern_length, led_brightness, led_choice);
  } else if ( pattern_name == "pulse"){
    pulse(current_time, pattern_length, led_brightness, led_choice); 
  }
  else if (pattern_name == "on" )
  {
    solid_running(led_choice, led_brightness);
  }
  else {
    switch_off(led_choice);
  }
}


/********** LED CONTROL FUNCTIONS *********/
void fade(unsigned long thisMillis, int fadeInterval, int maxPWM, int led_pin) {
  if (thisMillis - previousFadeMillis[led_pin] >= fadeInterval) {
    // yup, it's time!
    if (fadeDirection == 1) {
      fadeValue = fadeValue + fadeIncrement;
      if (fadeValue >= maxPWM) {
        // At max, limit and change direction
        fadeValue = maxPWM;
        fadeDirection = 0;
      }
    } else {
      //if we aren't going up, we're going down
      fadeValue = fadeValue - fadeIncrement;
      if (fadeValue <= minPWM) {
        // At min, limit and change direction
        fadeValue = minPWM;
        fadeDirection = 1;
      }
    }
    // Only need to update when it changes
    analogWrite(led_pin, fadeValue);
    // reset millis for the next iteration (fade timer only)
    previousFadeMillis[led_pin] = thisMillis;
  }
}

void pulse(unsigned long thisMillis, int on_duration, int on_speed, int led_pin){
  if (thisMillis - previousPulseMillis[led_pin] >= on_duration) {
    // save the last time you blinked the LED
    previousPulseMillis[led_pin] = thisMillis;

    // if the led is off turn it on and vice-versa:
    if (pulse_state == 0) {
      pulse_state = 1;
      // set the led with the ledState of the variable:
      analogWrite(led_pin, 255);
    } else {
      analogWrite(led_pin, 0);
      }      
      pulse_state = 0;
    }
  }
}

void switch_off(int led_pin){
    analogWrite(led_pin, 0);
}

void solid_running(int led_pin, int led_brightness){
    analogWrite(led_pin, led_brightness);
}

If I send the json data telling LED 1 to pulse, as soon as I send a second string telling LED2 to fade, LED 1 stops pulsing.

I'm sure this is something obvious, but it's been driving me mad for days!

EDIT: Adding the second attempt at this

// Declare the number of LED's
const int led_count = 2;

// Setup the ENUM for the various Modes
enum EMode { Off, On, Pulse, Fade };
// Create the struct and initialise it
struct MMode {
  EMode pin_mode;
  int action_pin;
  int action_delay;
  int action_max;
  int action_min;
  int fade_direction;
  int fade_value;
  int fade_increment;
  unsigned long m_current_time;
  unsigned long previous_millis;
} leds[led_count] {
  {Fade, D4, 20, 255, 0, 1, 0, 1, 0, 0},
  {Fade, BUILTIN_LED, 20, 255, 0, 1, 0, 1, 0, 0}
};


void setup() {
  Serial.begin(9600);
  // Setup the Built-in LED
  pinMode(BUILTIN_LED, OUTPUT);
  // Setup the Red LED
  pinMode(D4, OUTPUT);
}

void loop() {
    for (int n = 0; n < led_count; n++) {
      leds[n].m_current_time = millis();
      switch(leds[n].pin_mode){
        case Fade:
          fade(leds[n]);
          break;
      }
    }
}

/********** LED CONTROL FUNCTIONS *********/
void fade(struct MMode led) {
  if (led.m_current_time - led.previous_millis >= led.action_delay) {
    // yup, it's time!
    if (led.fade_direction == 1) {
      led.fade_value = led.fade_value + led.fade_increment;
      Serial.print("Fade Up Value: ");
      Serial.print(led.fade_value);
      Serial.println("    ");
      if (led.fade_value >= led.action_max) {
        // At max, limit and change direction
        led.fade_value = led.action_max;
        led.fade_direction = 0;
      }
    } else {
      //if we aren't going up, we're going down
      led.fade_value = led.fade_value - led.fade_increment;
    //  Serial.print("Fade Down Value");
    //  Serial.print(led.fade_value);
    //  Serial.print("    ");
      if (led.fade_value <= led.action_min) {
        // At min, limit and change direction
        led.fade_value = led.action_min;
        led.fade_direction = 1;
      }
    }
    //Serial.print(led.fade_value);
    //Serial.print("  ");
    // Only need to update when it changes
    analogWrite(led.action_pin, led.fade_value);
    // reset millis for the next iteration (fade timer only)
    led.previous_millis = led.m_current_time;
    //Serial.print(led.previous_millis);
    //Serial.print("  ");
  }
}

This second iteration of the code appears to define the led's using the struct correctly, however when I upload it to the arduino, motor.fade_value gets set to 1, and never increments above this.

I get the feeling I'm missing something from the answers that I've had so far, but I'm not sure what that something is, and google isn't proving very helpful tonight...

  • your code has nothing to pulse or fade other led then `led_choice' – Juraj Aug 9 '18 at 11:51
1

you need to call the fade/pulse function every time until the led state's change,

Start by creating full separate state for each LED,

struct LED_state{
    STATE state;

    unsigned long led1_current_time;
    unsigned long previousMillis;
    int pin;
    int fadeValue;
    int fadeDirection;
    int pulse_state = 0;
}

LED_state led[2];

STATE is an enum for each animation you do. Change the functions to take a pointer to this state. Initialize them in init().

Then create an update function which looks like:

void updateLED(LED_state* state){
    switch(state->state){
    case FADE:  fade(state);break;
    case PULSE: pulse(state);break;
    case ON:    solid_running(state);break;
    case OFF:   switch_off(state);break;
    }
}

Every time through the loop you call this update function for each LED's state.

To change what the LED does you the only need to update the state member in the struct for the LED.

  if ( pattern_name == "fade"){
    led[chosen_led]->state = FADE;
  } else if ( pattern_name == "pulse"){
    led[chosen_led]->state = PULSE;
  } else if (pattern_name == "on" ){
    led[chosen_led]->state = ON;
  } else {
    led[chosen_led]->state = OFF;
  }
  • So is this the same approach as @michel-keijzers (it seems to be to my eyes!), or does it differ? If it differs, would you mind explaining what/why so I can make an informed decision? Thanks :) – ProfFalken Aug 9 '18 at 13:54
  • 1
    it's pretty much the same, except I added more state into the per led struct – ratchet freak Aug 9 '18 at 14:00
2

Without looking to the entire code, the best way is to use a 'mode' for each led. So for each led, keep track how it should behave (like Off, On, Blink, FadeOff, FadeOn etc) and some parameters (for the speed for fading for example). You can use a struct for this, like

enum EMode { Off, On, Pulse, FadeOff, FadeOn };
struct LedMode
{
   EMode mode;
   int fadeSpeed; // Only used when mode = fade off/on
};

LedMode ledModes[8]; // For e.g. 8 leds

When you get a JSon command, change the ledMode structure for that LED accordingly.

Than in the main loop use something like:

loop()
{
    for (int n = 0; n < 8; n++) // For each led
    {
       switch (ledModes[n].mode)
       {
       case Off: 
         TurnOff(ledModes[n]);
         break;

       case On:
          TurnOn(ledModes[n]);
          break;

       case Pulse:
          Pulse(ledModes[n]);
          break;

       case FadeOn:
          FadeOn(ledModes[n]);
          break;

       ...
    }
}

To prevent going into each case all the time, add in the structure a 'Dirty' flag which is set to true by JSON and set to false after performed the change (e.g. you only have to switch a LED On or Off one time, not in every loop). For FadeOn/Off until it has reached 255 or 0. You also can use a PreviousMode and check if the mode has been changed since last time.

And for the Pulse/FadeOn/Off you need to add in the structure the last set value to be able to increase/decrease it, and the time it has changed last time.

Btw, you also can pass n and get LedModes[n] directly since it is global.

  • 1
    So this would move all of the code that controls the LED's out of the dedicated functions and directly into the main loop as part of the switch statement? – ProfFalken Aug 9 '18 at 11:40
  • 1
    No, this was just the 'structure' .... PLEASE keep the separate functions, you can pass the structure for that led into the function (ledModes[n] where n is the LED that is currently handled in the loop). Use a separate function for each mode, that's very good programming practice. – Michel Keijzers Aug 9 '18 at 11:42
  • 1
    That was what threw me, keeping them makes a lot of sense. I'm used to coding in Python, but I struggle with translating things over to C! Thanks. – ProfFalken Aug 9 '18 at 11:50
  • 1
    You're doing a good job, it will make it easy if you use the struct with parameters, so every LED will have its own set of data, than in the mode functions (off, on, pulse etc) you can change the intensity LED depending on the parameters, independing of the other LEDs. – Michel Keijzers Aug 9 '18 at 11:52
  • 1
    Unfortunately I can only accept one answer, and it was the part about referencing and setting the variables in @rachettFreak's answer that finally got this working for me. Thanks for all the help, it really was appreciated, and if I could, I'd select your answer too! – ProfFalken Aug 16 '18 at 16:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.