Problem getting one element of an LED sketch to run

Hi,

I feel like I'm going round in circles here and can't figure this out.

I am trying to rewrite this original code to work with the FastLED library (working fine) and without delay().

This is my code:

// candle for Adafruit NeoPixel
// 1 pixel version
// by Tim Bartlett, December 2013

// current settings for 5v Trinket

#include <FastLED.h>
#define LED_PIN 2
#define NUM_LEDS 1
#define BRIGHTNESS  100 // set at 25 to run on 5v (powersaving)

struct CRGB leds[NUM_LEDS];

byte onLedNumber = 0;
unsigned long currentTime;
unsigned long loopTime;
unsigned long onCurrentTime;
unsigned long onLoopTime;

// color variables: mix RGB (0-255) for desired yellow
int redPx = 255;
int grnHigh = 100; //110-120 for 5v, 135 for 3.3v
int bluePx = 10; //10 for 5v, 15 for 3.3v

// animation time variables, with recommendations
int burnDepth = 14; //10 for 5v, 14 for 3.3v -- how much green dips below grnHigh for normal burn -
int flutterDepth = 30; //25 for 5v, 30 for 3.3v -- maximum dip for flutter
int cycleTime = 120; //120 -- duration of one dip in microseconds

// pay no attention to that man behind the curtain
int fDelay;
int fRep;
int flickerDepth;
int burnDelay;
int burnLow;
int flickDelay;
int flickLow;
int flutDelay;
int flutLow;


void flutter(int f);
void flicker(int f);
void burn(int f);
void fullOn(int f);
void fire(int grnLow);

void setup() {
  FastLED.addLeds<WS2811, LED_PIN, RGB>(leds, NUM_LEDS);
  FastLED.setBrightness( BRIGHTNESS );
  flickerDepth = (burnDepth + flutterDepth) / 2.4;
  burnLow = grnHigh - burnDepth;
  burnDelay = (cycleTime / 2) / burnDepth;
  flickLow = grnHigh - flickerDepth;
  flickDelay = (cycleTime / 2) / flickerDepth;
  flutLow = grnHigh - flutterDepth;
  flutDelay = ((cycleTime / 2) / flutterDepth);
  Serial.begin(9600);
  onCurrentTime = millis();
  onLoopTime = onCurrentTime;
  currentTime = millis();
  loopTime = currentTime;
}

// In loop, call CANDLE STATES, with duration in seconds
// 1. fullOn() = solid yellow
// 2. burn() = candle is burning normally, flickering slightly
// 3. flicker() = candle flickers noticably
// 4. flutter() = the candle needs air!

void loop() {
  //  Serial.println("Burn");
  //  burn(10);
  //  Serial.println("Flicker");
  //  flicker(5);
  //  Serial.println("Burn");
  //  burn(8);
  Serial.println("Flutter");
  flutter(6);
  //  Serial.println("Burn");
  //  burn(3);
  Serial.println("On");
  fullOn(10);
  Serial.println("Burn");
  burn(10);
  //  Serial.println("Flicker");
  //  flicker(10);
}


// basic fire funciton - not called in main loop
void fire(int grnLow) {
  for (int grnPx = grnHigh; grnPx > grnLow; grnPx--) {
    currentTime = millis();
    if (currentTime >= (loopTime + fDelay)) {
      leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
      FastLED.show();
      loopTime = currentTime;  // Updates loopTime
    }
  }
  for (int grnPx = grnLow; grnPx < grnHigh; grnPx++) {
    currentTime = millis();
    if (currentTime >= (loopTime + fDelay)) {
      leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
      FastLED.show();
      loopTime = currentTime;  // Updates loopTime
    }
  }
}

// fire animation
void fullOn(int f) {
  fRep = f * 1000; 
  onCurrentTime = millis();
  if (onCurrentTime >= loopTime + fRep) {  
    // int grnPx = grnHigh - 5;
    // leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
    leds[onLedNumber] = CRGB::Red;
    FastLED.show();
    onLoopTime = onCurrentTime;  // Updates loopTime

  }
}

void burn(int f) {
  fRep = f * 8;
  fDelay = burnDelay;
  for (int var = 0; var < fRep; var++) {
    fire(burnLow);
  }
}

void flicker(int f) {
  fRep = f * 8;
  fDelay = burnDelay;
  fire(burnLow);
  fDelay = flickDelay;
  for (int var = 0; var < fRep; var++) {
    fire(flickLow);
  }
  fDelay = burnDelay;
  fire(burnLow);
  fire(burnLow);
  fire(burnLow);
}

void flutter(int f) {
  fRep = f * 8;
  fDelay = burnDelay;
  fire(burnLow);
  fDelay = flickDelay;
  fire(flickLow);
  fDelay = flutDelay;
  for (int var = 0; var < fRep; var++) {
    fire(flutLow);
  }
  fDelay = flickDelay;
  fire(flickLow);
  fire(flickLow);
  fDelay = burnDelay;
  fire(burnLow);
  fire(burnLow);
}

I have stuff commented out and Serial.prints to try and isolate the problem but it isn't helping.

The problem is that that this section seems to just get skipped and the LED does not light.

// fire animation
void fullOn(int f) {
  fRep = f * 1000; 
  onCurrentTime = millis();
  if (onCurrentTime >= loopTime + fRep) {  
    // int grnPx = grnHigh - 5;
    // leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
    leds[onLedNumber] = CRGB::Red;
    FastLED.show();
    onLoopTime = onCurrentTime;  // Updates loopTime

  }
}

Does anyone have any ideas? I'm going crazy!

Is this right:-

if (onCurrentTime >= loopTime + fRep)

or did you mean:-

if (onCurrentTime >= onLoopTime + fRep)

Your choice of variable names and lack of comments in the code make it hard to tell what's what.

OldSteve:
Is this right:-

if (onCurrentTime >= loopTime + fRep)

or did you mean:-

if (onCurrentTime >= onLoopTime + fRep)

Your choice of variable names and lack of comments in the code make it hard to tell what's what.

Agh Yes I spotted that, but even when I fix it, it doesn't fix the problem. Here's a simpler version of the problem:

Original code that works using delay:

// candle for Adafruit NeoPixel
// 1 pixel version
// by Tim Bartlett, December 2013

// current settings for 5v Trinket

#include <FastLED.h>
#define LED_PIN 2
#define NUM_LEDS 1
#define BRIGHTNESS  100 // set at 25 to run on 5v (powersaving)

struct CRGB leds[NUM_LEDS];

byte onLedNumber = 0;

// color variables: mix RGB (0-255) for desired yellow
int redPx = 255;
int grnHigh = 100; //110-120 for 5v, 135 for 3.3v
int bluePx = 10; //10 for 5v, 15 for 3.3v

// animation time variables, with recommendations
int burnDepth = 14; //10 for 5v, 14 for 3.3v -- how much green dips below grnHigh for normal burn -
int flutterDepth = 30; //25 for 5v, 30 for 3.3v -- maximum dip for flutter
int cycleTime = 120; //120 -- duration of one dip in milliseconds

// pay no attention to that man behind the curtain
int fDelay;
int fRep;
int flickerDepth;
int burnDelay;
int burnLow;
int flickDelay;
int flickLow;
int flutDelay;
int flutLow;


void flutter(int f);
void flicker(int f);
void burn(int f);
void on(int f);
void fire(int grnLow);

void setup() {
  FastLED.addLeds<WS2811, LED_PIN, RGB>(leds, NUM_LEDS);
  FastLED.setBrightness( BRIGHTNESS );
  flickerDepth = (burnDepth + flutterDepth) / 2.4;
  burnLow = grnHigh - burnDepth;
  burnDelay = (cycleTime / 2) / burnDepth;
  flickLow = grnHigh - flickerDepth;
  flickDelay = (cycleTime / 2) / flickerDepth;
  flutLow = grnHigh - flutterDepth;
  flutDelay = ((cycleTime / 2) / flutterDepth);

  Serial.begin(9600);
}

// In loop, call CANDLE STATES, with duration in seconds
// 1. on() = solid yellow
// 2. burn() = candle is burning normally, flickering slightly
// 3. flicker() = candle flickers noticably
// 4. flutter() = the candle needs air!

void loop() {
  Serial.println("Burn");
  burn(10);
  Serial.println("Flicker");
  flicker(5);
  Serial.println("Burn");
  burn(8);
  Serial.println("Flutter");
  flutter(6);
  Serial.println("Burn");
  burn(3);
  Serial.println("On");
  on(10);
  Serial.println("Burn");
  burn(10);
  Serial.println("Flicker");
  flicker(10);
}


// basic fire funciton - not called in main loop
void fire(int grnLow) {
  for (int grnPx = grnHigh; grnPx > grnLow; grnPx--) {
    leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
    FastLED.show();
    delay(fDelay);
  }
  for (int grnPx = grnLow; grnPx < grnHigh; grnPx++) {
    leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
    FastLED.show();
    delay(fDelay);
  }
}

// fire animation
void on(int f) {
  fRep = f * 1000;
  int grnPx = grnHigh - 5;
  // int grnPx = grnHigh - 5;
  // leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
  leds[onLedNumber] = CRGB::Red;
  FastLED.show();
  delay(fRep);
}

void burn(int f) {
  fRep = f * 8;
  fDelay = burnDelay;
  for (int var = 0; var < fRep; var++) {
    fire(burnLow);
  }
}

void flicker(int f) {
  fRep = f * 8;
  fDelay = burnDelay;
  fire(burnLow);
  fDelay = flickDelay;
  for (int var = 0; var < fRep; var++) {
    fire(flickLow);
  }
  fDelay = burnDelay;
  fire(burnLow);
  fire(burnLow);
  fire(burnLow);
}

void flutter(int f) {
  fRep = f * 8;
  fDelay = burnDelay;
  fire(burnLow);
  fDelay = flickDelay;
  fire(flickLow);
  fDelay = flutDelay;
  for (int var = 0; var < fRep; var++) {
    fire(flutLow);
  }
  fDelay = flickDelay;
  fire(flickLow);
  fire(flickLow);
  fDelay = burnDelay;
  fire(burnLow);
  fire(burnLow);
}

New code, just trying to replace the delay() in void on()

// candle for Adafruit NeoPixel
// 1 pixel version
// by Tim Bartlett, December 2013

// current settings for 5v Trinket

#include <FastLED.h>
#define LED_PIN 2
#define NUM_LEDS 1
#define BRIGHTNESS  100 // set at 25 to run on 5v (powersaving)

struct CRGB leds[NUM_LEDS];

byte onLedNumber = 0;

// delays

unsigned long onPreviousMillis = 0;        // will store last time LED was updated

// color variables: mix RGB (0-255) for desired yellow
int redPx = 255;
int grnHigh = 100; //110-120 for 5v, 135 for 3.3v
int bluePx = 10; //10 for 5v, 15 for 3.3v

// animation time variables, with recommendations
int burnDepth = 14; //10 for 5v, 14 for 3.3v -- how much green dips below grnHigh for normal burn -
int flutterDepth = 30; //25 for 5v, 30 for 3.3v -- maximum dip for flutter
int cycleTime = 120; //120 -- duration of one dip in milliseconds

// pay no attention to that man behind the curtain
int fDelay;
int fRep;
int flickerDepth;
int burnDelay;
int burnLow;
int flickDelay;
int flickLow;
int flutDelay;
int flutLow;


void flutter(int f);
void flicker(int f);
void burn(int f);
void on(int f);
void fire(int grnLow);

void setup() {
  FastLED.addLeds<WS2811, LED_PIN, RGB>(leds, NUM_LEDS);
  FastLED.setBrightness( BRIGHTNESS );
  flickerDepth = (burnDepth + flutterDepth) / 2.4;
  burnLow = grnHigh - burnDepth;
  burnDelay = (cycleTime / 2) / burnDepth;
  flickLow = grnHigh - flickerDepth;
  flickDelay = (cycleTime / 2) / flickerDepth;
  flutLow = grnHigh - flutterDepth;
  flutDelay = ((cycleTime / 2) / flutterDepth);

  Serial.begin(9600);
}

// In loop, call CANDLE STATES, with duration in seconds
// 1. on() = solid yellow
// 2. burn() = candle is burning normally, flickering slightly
// 3. flicker() = candle flickers noticably
// 4. flutter() = the candle needs air!

void loop() {
  Serial.println("Burn");
  burn(10);
  Serial.println("Flicker");
  flicker(5);
  Serial.println("Burn");
  burn(8);
  Serial.println("Flutter");
  flutter(6);
  Serial.println("Burn");
  burn(3);
  Serial.println("On");
  on(10);
  Serial.println("Burn");
  burn(10);
  Serial.println("Flicker");
  flicker(10);
}


// basic fire funciton - not called in main loop
void fire(int grnLow) {
  for (int grnPx = grnHigh; grnPx > grnLow; grnPx--) {



    leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
    FastLED.show();
    delay(fDelay);



  }
  for (int grnPx = grnLow; grnPx < grnHigh; grnPx++) {



    leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
    FastLED.show();
    delay(fDelay);



  }
}

// fire animation
void on(int f) {
  fRep = f * 1000;
  int grnPx = grnHigh - 5;

  unsigned long onCurrentMillis = millis();

  if (onCurrentMillis - onPreviousMillis > fRep) {
    
    onPreviousMillis = onCurrentMillis;   
    // int grnPx = grnHigh - 5;
    // leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
    leds[onLedNumber] = CRGB::Red;
    FastLED.show();
 

  }


}

void burn(int f) {
  fRep = f * 8;
  fDelay = burnDelay;
  for (int var = 0; var < fRep; var++) {
    fire(burnLow);
  }
}

void flicker(int f) {
  fRep = f * 8;
  fDelay = burnDelay;
  fire(burnLow);
  fDelay = flickDelay;
  for (int var = 0; var < fRep; var++) {
    fire(flickLow);
  }
  fDelay = burnDelay;
  fire(burnLow);
  fire(burnLow);
  fire(burnLow);
}

void flutter(int f) {
  fRep = f * 8;
  fDelay = burnDelay;
  fire(burnLow);
  fDelay = flickDelay;
  fire(flickLow);
  fDelay = flutDelay;
  for (int var = 0; var < fRep; var++) {
    fire(flutLow);
  }
  fDelay = flickDelay;
  fire(flickLow);
  fire(flickLow);
  fDelay = burnDelay;
  fire(burnLow);
  fire(burnLow);
}

This still just skips over that function.

I have narrowed down my code to the trouble spot, I hope it's easier to get help with this way.

This LED just stays on Purple and I would like it to run the code in the IF statement first. I must be doing something simple wrong!

#include <FastLED.h>
#define LED_PIN 2
#define NUM_LEDS 50
#define BRIGHTNESS  100 // set at 25 to run on 5v (powersaving)

struct CRGB leds[NUM_LEDS];

byte onLedNumber = 0;

// delays
unsigned long onPreviousMillis = 0;        // will store last time LED was updated

// declare functions because of arduino bug
// https://github.com/FastLED/FastLED/issues/234
void on(byte f);

void setup() {
  FastLED.addLeds<WS2811, LED_PIN, RGB>(leds, NUM_LEDS);
  FastLED.setBrightness( BRIGHTNESS );
}

void loop() {
  on(10);
}

// fire animation
void on(byte f) {
  unsigned int fRep = f * 1000;  // Convert to seconds
  unsigned long onCurrentMillis = millis();
  if (onCurrentMillis - onPreviousMillis > fRep) {
    onPreviousMillis = onCurrentMillis;
    leds[onLedNumber] = CRGB::Red;
    FastLED.show();
  } 
  else 
  {
    leds[onLedNumber] = CRGB::Purple;
    FastLED.show();
    delay(2000);
  }
}

dillonradio:
I have narrowed down my code to the trouble spot, I hope it's easier to get help with this way.

This LED just stays on Purple and I would like it to run the code in the IF statement first. I must be doing something simple wrong!

#include <FastLED.h>

#define LED_PIN 2
#define NUM_LEDS 50
#define BRIGHTNESS  100 // set at 25 to run on 5v (powersaving)

struct CRGB leds[NUM_LEDS];

byte onLedNumber = 0;

// delays
unsigned long onPreviousMillis = 0;        // will store last time LED was updated

// declare functions because of arduino bug
// Errors compiling Fastled library teensy 3.2 · Issue #234 · FastLED/FastLED · GitHub
void on(byte f);

void setup() {
  FastLED.addLeds<WS2811, LED_PIN, RGB>(leds, NUM_LEDS);
  FastLED.setBrightness( BRIGHTNESS );
}

void loop() {
  on(10);
}

// fire animation
void on(byte f) {
  unsigned int fRep = f * 1000;  // Convert to seconds
  unsigned long onCurrentMillis = millis();
  if (onCurrentMillis - onPreviousMillis > fRep) {
    onPreviousMillis = onCurrentMillis;
    leds[onLedNumber] = CRGB::Red;
    FastLED.show();
  }
  else
  {
    leds[onLedNumber] = CRGB::Purple;
    FastLED.show();
    delay(2000);
  }
}

The LED will light red for an instant after 10 seconds, (not visible to the naked eye), then in the next instant it will light purple and stay purple until another 10 seconds have passed.

To test this, try putting a very small delay in the 'if' statement, 100mS to 200mS for example, after lighting the LED red, and the LED will be red briefly, but for long enough to see, every 10 seconds.

OldSteve:
The LED will light red for an instant after 10 seconds, (not visible to the naked eye), then in the next instant it will light purple and stay purple until another 10 seconds have passed.

To test this, try putting a very small delay in the 'if' statement, 100mS to 200mS for example, after lighting the LED red, and the LED will be red briefly, but for long enough to see, every 10 seconds.

Riiight. OK. So What I would like is for it to stay on for 10 seconds. Should I be using a while statement then?

Maybe a small statemachine will do what you need. The below will switch to purple for two seconds followed by red for a variable number of seconds. I don't have the library so I assume your use of the library is OK.

void on(byte f)
{
  // number of state to execute
  // initial value 0 (purple on for two seconds)
  static byte statenumber = 0;

  switch (statenumber)
  {
    // state 0: purple on for 2 seconds
    case 0:
      // purple on
      leds[onLedNumber] = CRGB::Purple;
      FastLED.show();

      // delay
      if (doDelay(2000))
      {
        // go to next state
        statenumber = 1;
      }
      break;
    // state 1: red on for f seconds
    case 1:
      // red on
      leds[onLedNumber] = CRGB::Red;
      FastLED.show();

      // delay
      if (doDelay(f * 1000))
      {
        // go to next (first) state
        statenumber = 0;
      }
      break;
  }
}

The doDelay function

/*
  non-blocking delay for duration milliseconds
  returns false if delay in progress, else true
*/
bool doDelay(unsigned long duration)
{
  // variable to keep track of previous millis
  static unsigned long previousMillis = 0;

  // if delay not started
  if (previousMillis == 0)
  {
    // start delay
    previousMillis = millis();
  }
  // if delay has lapsed
  if (millis() - previousMillis >= duration)
  {
    // reset previous millis
    previousMillis = 0;
    // indicate that delay has lapsed
    return true;
  }
  // indicate delay in progress
  return false;
}

sterretje:
Maybe a small statemachine will do what you need. The below will switch to purple for two seconds followed by red for a variable number of seconds. I don't have the library so I assume your use of the library is OK.

void on(byte f)

{
 // number of state to execute
 // initial value 0 (purple on for two seconds)
 static byte statenumber = 0;

switch (statenumber)
 {
   // state 0: purple on for 2 seconds
   case 0:
     // purple on
     leds[onLedNumber] = CRGB::Purple;
     FastLED.show();

// delay
     if (doDelay(2000))
     {
       // go to next state
       statenumber = 1;
     }
     break;
   // state 1: red on for f seconds
   case 1:
     // red on
     leds[onLedNumber] = CRGB::Red;
     FastLED.show();

// delay
     if (doDelay(f * 1000))
     {
       // go to next (first) state
       statenumber = 0;
     }
     break;
 }
}




The doDelay function


/*
 non-blocking delay for duration milliseconds
 returns false if delay in progress, else true
*/
bool doDelay(unsigned long duration)
{
 // variable to keep track of previous millis
 static unsigned long previousMillis = 0;

// if delay not started
 if (previousMillis == 0)
 {
   // start delay
   previousMillis = millis();
 }
 // if delay has lapsed
 if (millis() - previousMillis >= duration)
 {
   // reset previous millis
   previousMillis = 0;
   // indicate that delay has lapsed
   return true;
 }
 // indicate delay in progress
 return false;
}

Thanks for the suggestion! I actually don't need it to go purple again afterwards, that was just used for debugging. I just need it to stay on red for the required seconds and then move on to the next part of the program. The doDelay should do the trick, I'll test it now. Thanks!

Your state machine code is also very useful so thanks for that!

sterretje:
Maybe a small statemachine will do what you need. The below will switch to purple for two seconds followed by red for a variable number of seconds. I don't have the library so I assume your use of the library is OK.

void on(byte f)

{
  // number of state to execute
  // initial value 0 (purple on for two seconds)
  static byte statenumber = 0;

switch (statenumber)
  {
    // state 0: purple on for 2 seconds
    case 0:
      // purple on
      leds[onLedNumber] = CRGB::Purple;
      FastLED.show();

// delay
      if (doDelay(2000))
      {
        // go to next state
        statenumber = 1;
      }
      break;
    // state 1: red on for f seconds
    case 1:
      // red on
      leds[onLedNumber] = CRGB::Red;
      FastLED.show();

// delay
      if (doDelay(f * 1000))
      {
        // go to next (first) state
        statenumber = 0;
      }
      break;
  }
}




The doDelay function


/*
  non-blocking delay for duration milliseconds
  returns false if delay in progress, else true
*/
bool doDelay(unsigned long duration)
{
  // variable to keep track of previous millis
  static unsigned long previousMillis = 0;

// if delay not started
  if (previousMillis == 0)
  {
    // start delay
    previousMillis = millis();
  }
  // if delay has lapsed
  if (millis() - previousMillis >= duration)
  {
    // reset previous millis
    previousMillis = 0;
    // indicate that delay has lapsed
    return true;
  }
  // indicate delay in progress
  return false;
}

I have succeeded in using the state machine to control the timing of the basic animation, but there is one more part of the program that uses delay and I wonder if I can use another smaller state machine to replace delay() here too?

void fire(int grnLow) {
  for (int grnPx = grnHigh; grnPx > grnLow; grnPx--) {
    leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
    FastLED.show();
    delay(fDelay);
  }
  for (int grnPx = grnLow; grnPx < grnHigh; grnPx++) {
    leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
    FastLED.show();
    delay(fDelay);
  }
}

As you can see, the length of each part of the animation is controlled by the variables in the FOR loop as well as delay() embedded within it.

Basically it fades the green pixel down in one FOR loop and then back up in another. It seems obvious that those would be the two states, but as the timing is contained within the loop, I'm not sure how to structure the switch statement.

Right I have it working but it might not be the most elegant solution. I have tried to comment it as fully as possible.

One issue is that I now have two similar functions:

doDelay() and animDelay() as animDelay is used as a function from within one of the states that doDelay() controls it was causing a conflict.

I'm not sure if there is a better way than just having two functions with different variable names.

Thanks for everyone's help here, it has made all the difference and I have learnt loads :smiley:

The code on PasteBin as it was too big to post in total - /* candle for Adafruit NeoPixel 1 pixel version by Tim Bartlett, Decembe - Pastebin.com

I was typing a complete essay how loop() was basically already the statemachine and how to change to use states and you already figured it out. Well done.

The problem with your new version is that it's actually still blocking. The whole purpose of replacing a delay based solution by a millis() based solution is that it will not block so you can e.g. react on a button press to stop/start/pause the sequence.

You can make your fire() return true or false similar as the delay functions. Below a non-blocking function version that simply goes through the motion of dimming and brightening and using millis() for the delay.

bool fire(int grnLow) {
  // remember if we're dimming (true) or brightening (false)
  static bool fDimming = true;
  // remember value for grnPx
  static int grnPx = grnHigh;
  // remember start time of delay
  unsigned long previousMillis = 0;

  // if in dimming mode
  if (fDimming == true)
  {
    // if timing not started yet
    if (previousMillis == 0)
    {
      leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
      FastLED.show();
      // set start time
      previousMillis = millis();
    }

    // if delay lapsed
    if (millis() - previousMillis >= fDelay)
    {
      // decrement grnPx
      grnPx--;
      // reset start time
      previousMillis = 0;
    }

    // if lower limit reached
    if (grnPx <= grnLow)
    {
      // switch to next state
      fDimming = false;
    }
  }
  // if in brightening mode
  else
  {
    // if timing not started yet
    if (previousMillis == 0)
    {
      grnPx = grnLow;
      leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
      FastLED.show();
      // set start time
      previousMillis = millis();
    }

    // if delay lapsed
    if (millis() - previousMillis >= fDelay)
    {
      // increment grnPx
      grnPx++;
      // reset start time
      previousMillis = 0;
    }

    // if upper limit reached
    if (grnPx >= grnHigh)
    {
      // switch to previous state
      fDimming = true;
      // indicate that we have done one cycle dim and brighten
      return true;
    }
  }
  // indicate function not completed yet
  return false;
}

The 'statemachine' is in this case simply implemented with a boolean flag indicating if we're dimming or brightening.

The function that calls this can check for the return value. The below is a modified version of burn(). It again returns true if it's completed and else it return false

bool burn(byte f) {
  Serial.println("Burn");
  fRep = f * 8;
  fDelay = burnDelay;

  // remember var count; initial value zero
  static byte var = 0;

  // call fire; if the result is true, a cycle has been completed
  if (fire(burnLow) == true)
  {
    // increment var
    var++;
  }

  // if not fRep cycles done
  if (var < fRep)
  {
    // indicate burn (still) in progress
    return false;
  }

  // reset var
  var = 0;
  // indicate burn completed
  return true;
}

And next you can modify e.g. case 0 to make use of the returned value. Actually case 0 consists of two states, a burn state and a delay state. That was not the situation in the original code that you were modifying. Not sure why it was added.

Following the original flow case 0 becomes

    // state 0: burn for seconds equal to onForSeconds
    case 0:
      // burn
      onForSeconds = 10;
      if(burn(onForSeconds) == true)
      {
        // go to next state
        statenumber = 1;
      }
      break;

I have not tried to compile and therefore also not tested it.

I hope this gives you the idea. You need to modify all functions that make use of fire and all states (cases in loop()).

sterretje:
The problem with your new version is that it's actually still blocking.

Thank you so much for your suggestions. I had just finished copying my 'candle' onto my larger sketch and lo and behold there was still blocking as you quite rightly pointed out from looking at the code.

I was reluctant to come back here for more help and have been trying to think of ways around it but you have given me a magnificent head start! It was along the lines I was thinking which I am glad about , it means i'm learning :slight_smile:

I have thought of some improvements to the code to, like randomising the burn length as well as the order of the animations and I'm pleased that it wasn't the state machine itself that was the problem, just my implementation of it as this means I can go ahead with my upgrades :slight_smile:

Thanks again, I think I've got another fun but busy day on it tomorrow now too :smiley:

Rob

sterretje:
You need to modify all functions that make use of fire and all states (cases in loop()).

One question, where it checks how many cycles I am doing in the flutter and flicker functions, sometimes it just calls fire() once. Would I still need a var counter but just change fRep to 1?

 // if not fRep cycles done
  if (var < 1)
  {
    // indicate burn (still) in progress
    return false;
  }

or is there a simpler way of call it just once?

Actually i've realised I'm thinking about flicker and flutter wrong.

flicker needs to return true when it's done

fire cycle once with burn delay timing
AND
fire cycle fRep times with flicker delay timing
AND
fire cycle once with burn delay timing three more times

flutter needs to return true when it's done

fire cycle once with burn delay timing
AND
fire cycle once with flicker delay timing
AND
fire cycle fRep times with flutter delay timing
AND
fire cycle twice with flicker delay timing
AND
fire cycle twice with burn delay timing

This is to add smoothing to the animation

I'm getting my head around how it should work, but not sure how to declare true using these multiple conditions.

No. You can directly return the result of the call to fire.

return fire(flickLow);

You are aware that flicker and flutter can / need to be implemented as their own statemachines?

sterretje:
You are aware that flicker and flutter can / need to be implemented as their own statemachines?

I have just realised this! While you were replying I wrote this:

Sorry for these multiple posts but I think writing it down like this is helping me think through it.

I think I need another switch statement to cycle through the different elements of the animation.

lots of embedded switches!

But your info was very useful to know how to use return for this purpose.

I have implemented all the state machines with the new code at:

I have added in some Serial.print markers to help debug and it is not getting to the second marker (line 287)

The millis() - previousMillis is just reading 0 in the serial monitor.

Any ideas what might not be working?

Thanks

This is the code that is causing the bug:

sterretje:

bool fire(int grnLow) {

// remember if we're dimming (true) or brightening (false)
  static bool fDimming = true;
  // remember value for grnPx
  static int grnPx = grnHigh;
  // remember start time of delay
  unsigned long previousMillis = 0;

// if in dimming mode
  if (fDimming == true)
  {
    // if timing not started yet
    if (previousMillis == 0)
    {
      leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
      FastLED.show();
      // set start time
      previousMillis = millis();
    }

// if delay lapsed
    if (millis() - previousMillis >= fDelay)
    {
      // decrement grnPx
      grnPx--;
      // reset start time
      previousMillis = 0;
    }

// if lower limit reached
    if (grnPx <= grnLow)
    {
      // switch to next state
      fDimming = false;
    }
  }
  // if in brightening mode
  else
  {
    // if timing not started yet
    if (previousMillis == 0)
    {
      grnPx = grnLow;
      leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
      FastLED.show();
      // set start time
      previousMillis = millis();
    }

// if delay lapsed
    if (millis() - previousMillis >= fDelay)
    {
      // increment grnPx
      grnPx++;
      // reset start time
      previousMillis = 0;
    }

// if upper limit reached
    if (grnPx >= grnHigh)
    {
      // switch to previous state
      fDimming = true;
      // indicate that we have done one cycle dim and brighten
      return true;
    }
  }
  // indicate function not completed yet
  return false;
}

I can't get it to return true. I think it's a problem with the lines that set previousMillis = 0 as when I comment them out, the program no longer freezes but still doesnt work correctly.

I'm trying a few different approaches, but if there is an obvious reason why that's not working then that would be ideal!

bool fire(int grnLow) {
  // remember if we're dimming (true) or brightening (false)
  static bool fDimming = true;
  // remember value for grnPx
  static int grnPx = grnHigh;
  // remember start time of delay
  static unsigned long previousMillis = 0;
  ...
  ...

previousMillis must be static, else it's lost everytime the fire function is finished. Modified in above code.

sterretje:
previousMillis must be static, else it's lost everytime the fire function is finished. Modified in above code.

Thanks. I fixed that and it helped and I also spotted another bug

bool fire(int grnLow) {
  // remember if we're dimming (true) or brightening (false)
  static bool fDimming = true;
  // remember value for grnPx
  static int grnPx = grnHigh;
  // remember start time of delay
  unsigned long previousMillis = 0;

  // if in dimming mode
  if (fDimming == true)
  {
    // if timing not started yet
    if (previousMillis == 0)
    {
      leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
      FastLED.show();
      // set start time
      previousMillis = millis();
    }

    // if delay lapsed
    if (millis() - previousMillis >= fDelay)
    {
      // decrement grnPx
      grnPx--;
      // reset start time
      previousMillis = 0;
    }

    // if lower limit reached
    if (grnPx <= grnLow)
    {
      // switch to next state
      fDimming = false;
    }
  }
  // if in brightening mode
  else
  {
    // if timing not started yet
    if (previousMillis == 0)
    {
      grnPx = grnLow;
      leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
      FastLED.show();
      // set start time
      previousMillis = millis();
    }

    // if delay lapsed
    if (millis() - previousMillis >= fDelay)
    {
      // increment grnPx
      grnPx++;
      // reset start time
      previousMillis = 0;
    }

    // if upper limit reached
    if (grnPx >= grnHigh)
    {
      // switch to previous state
      fDimming = true;
      // indicate that we have done one cycle dim and brighten
      return true;
    }
  }
  // indicate function not completed yet
  return false;
}

Should be

bool fire(int grnLow) {
  // remember if we're dimming (true) or brightening (false)
  static bool fDimming = true;
  // remember value for grnPx
  static int grnPx = grnHigh;
  // remember start time of delay
  unsigned long previousMillis = 0;

  // if in dimming mode
  if (fDimming == true)
  {
    // if timing not started yet
    if (previousMillis == 0)
    {
      leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
      FastLED.show();
      // set start time
      previousMillis = millis();
    }

    // if delay lapsed
    if (millis() - previousMillis >= fDelay)
    {
      // decrement grnPx
      grnPx--;
      // reset start time
      previousMillis = 0;
    }

    // if lower limit reached
    if (grnPx <= grnLow)
    {
      // switch to next state
      fDimming = false;
      grnPx = grnLow;
    }
  }
  // if in brightening mode
  else
  {
    // if timing not started yet
    if (previousMillis == 0)
    {
      leds[onLedNumber] = CRGB(redPx, grnPx, bluePx);
      FastLED.show();
      // set start time
      previousMillis = millis();
    }

    // if delay lapsed
    if (millis() - previousMillis >= fDelay)
    {
      // increment grnPx
      grnPx++;
      // reset start time
      previousMillis = 0;
    }

    // if upper limit reached
    if (grnPx >= grnHigh)
    {
      // switch to previous state
      fDimming = true;
      // indicate that we have done one cycle dim and brighten
      return true;
    }
  }
  // indicate function not completed yet
  return false;
}

Moving grnPx = grnLow; to the end of the dimming routine.

It's still not working as intended yet though, time for some more debugging!