Blink without delay, two intervals 240615

Hello Arduino forum,

Have done the Blink Without Delay in the Examples. Have
also done a sketch that blinks several LEDs by having each LED
have its own LEDx BWOD function and calling the different LEDx
functions from the loop() function. (Can post this sketch if
you are interested but basically it creates a BWOD function for
each LED and then calls each LED BWOD function from the loop() function.

Several forum posts on controlling several LEDs with different
intervals have also been studied, copied and implemented
successfully.

In these examples the BWOD toggles between high and low using the constuct

   if (ledStateX == LOW) {
      ledStateX = HIGH;
    } else {
      ledStateX = LOW;
    }

What is sought is to have the same LED blink longer
off than on. So it was conjectured that the toggle constuct
copied herewith above would be omitted and there should
be two BWOD functions, on for ON and another
for OFF.

Copied my attempt at this herewith below. Having trouble
getting it to compile.
Compilation error: expected unqualified-id before 'if'

Thanks.

Allen Pitts
Dallas TX

/* Blink without Delay
based on documentation found at
https://docs.arduino.cc/built-in-examples/digital/BlinkWithoutDelay/
and 
Blink_1_wo_Delay_Attiny85_240402,ino
Blink_1_wo_Delay_Attiny85_two_intervals_2400615.ino (this sketch)
uses two intervals so the On interval can be different from the Off interval.
*/

// constants won't change. Used here to set a pin number :
const int ledPin3 = 3;  // the number of the LED pin

// Variables will change :
int ledStateOn = LOW;
int ledStateOff = LOW;


unsigned long previousMillisOn = 0;
unsigned long previousMillisOff = 0;

// constants won't change :
const long intervalOn = 1000;
const long intervalOff = 500;

void setup() {
  // set the digital pin as output:
  pinMode(ledPin3, OUTPUT);
}

void loop() {
  runLED3();
}

void runLED3() {
  unsigned long currentMillisOn = millis();

  if (currentMillisOn - previousMillisOn >= intervalOn && ledStateOn = LOW)
  {
    // save the last time you blinked the LED
    previousMillisOn = currentMillisOn;

    // if the LED is off turn it on and vice-versa:
    //if (ledStateOn == LOW) {
    //  ledStateOn = HIGH;
    //} else {
    // ledStateOn = LOW;
    }

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin3, ledStateOn);
 
  }
 
  unsigned long currentMillisOff = millis();

  if (currentMillisOff - previousMillisff >= intervalOff && ledStateOff = HIGH)
  {
    // save the last time you blinked the LED
    previousMillisOff = currentMillisOff;

    // if the LED is off turn it on and vice-versa:
    //if (ledStateOn == LOW) {
    //  ledStateOn = HIGH;
    //} else {
    // ledStateOn = LOW;
    }

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin3, ledStateOff);
  ledStateOn = LOW;
  }

}

You have code outside of a function

Check where the closing brace of the runLED3() function is

void runLED3()
{
    unsigned long currentMillisOn = millis();

    if (currentMillisOn - previousMillisOn >= intervalOn&& ledStateOn = LOW)
    {
        // save the last time you blinked the LED
        previousMillisOn = currentMillisOn;

        // if the LED is off turn it on and vice-versa:
        //if (ledStateOn == LOW) {
        //  ledStateOn = HIGH;
        //} else {
        // ledStateOn = LOW;
    }

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin3, ledStateOn);
}    //end of the function

unsigned long currentMillisOff = millis();

if (currentMillisOff - previousMillisff >= intervalOff&& ledStateOff = HIGH)

This shows up well if you Auto Format the code in the IDE as I have in the code above

What's been said above, and

if (currentMillisOn - previousMillisOn >= intervalOn && ledStateOn = LOW)

should be

if (currentMillisOn - previousMillisOn >= intervalOn && ledStateOn == LOW)

and

if (currentMillisOff - previousMillisff >= intervalOff&& ledStateOff = HIGH)

should be

if (currentMillisOff - previousMillisOff >= intervalOff && ledStateOff == HIGH)

Sounds like some nice experiments.

Whenever you are making more of something by adding a digit to a variable name you should think about using an array.

Whenever you are making your code grow by copy/paste/editing existing code, you shoukd think about writing one generalized function. Arrays will make that easier, often.

And the N near identical lines which are you calls to the N near identical functions could be handled using a for loop that calls that general function N times.

So I recommend that if they are not, functions, arrays and for loops should be what you eat for breakfast.

It will make things easier in the long run… imagine for examlke finding a defect or changing you mind about how the functions you wrote are to work. With multiple near identical copies, you have to (carefully!) make multiple near identical edits.

I just hastily did that (lazy) in some test code for another thread and was distracted by two editing errors for longer than it would have taken me to just make a general function.

HTH

a7

Hello AllenPitts
Hello to Dallas

Consider this example derived from your sketch.

I've tidied it up a bit and added an array to control the LED with different ON/OFF times.

Check and test it.

/* Blink without Delay
  based on documentation found at
  https://docs.arduino.cc/built-in-examples/digital/BlinkWithoutDelay/
  and
  Blink_1_wo_Delay_Attiny85_240402,ino
  Blink_1_wo_Delay_Attiny85_two_intervals_2400615.ino (this sketch)
  uses two intervals so the On interval can be different from the Off interval.
*/

// constants won't change. Used here to set a pin number :
const int ledPin3 = 3;  // the number of the LED pin

// Variables will change :
int ledState = LOW;
int ledStateOff = LOW;


unsigned long previousMillisOn = 0;
unsigned long previousMillisOff = 0;

// constants won't change :
const long intervalOn = 1000;
const long intervalOff = 500;
const unsigned long intervals[] {intervalOff, intervalOn};

void setup() {
  // set the digital pin as output:
  pinMode(ledPin3, OUTPUT);
}

void loop() {
  runLED3();
}

void runLED3() {
  unsigned long currentMillisOn = millis();

  if (currentMillisOn - previousMillisOn >= intervals[ledState])
  {
    // save the last time you blinked the LED
    previousMillisOn = currentMillisOn;

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW) {
      ledState = HIGH;
    } else {
      ledState = LOW;
    }
    // set the LED with the ledState of the variable:
    digitalWrite(ledPin3, ledState);

  }
}
  /*
    unsigned long currentMillisOff = millis();

    if (currentMillisOff - previousMillisff >= intervalOff && ledStateOff = HIGH)
    {
    // save the last time you blinked the LED
    previousMillisOff = currentMillisOff;

    // if the LED is off turn it on and vice-versa:
    //if (ledState == LOW) {
    //  ledState = HIGH;
    //} else {
    // ledState = LOW;
    }

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin3, ledStateOff);
    ledState = LOW;
    }

    }
  */

If more than one LED is to be controlled, I recommend using structured/array, also known as a class.
I can look for an example of this in my sketchbox if needed.

Have a nice day and have fun programming in C++.

Have a look at Programming Electronics Academy. They have a nice series on Arduino multi-tasking and event timing...

Then use WOKWI.COM to simulate your idea...

duplicating code obviously increases the size of the code which makes it more difficult to debug

consider the following which uses arrays to support many more than just 1 timer. also not how arrays allow changes to both the # of timers and their periods without touching logic


const byte TmrPin [] = { 13, 12, 11, 10 };     // Capitalize Constants
const int Ntmr = sizeof(TmrPin);

const unsigned long TmrMsecOn  [Ntmr] = { 500, 250, 1000, 100 };
const unsigned long TmrMsecOff [Ntmr] = { 500, 750,  500, 200 };
      unsigned long tmrMsecPer [Ntmr];
      unsigned long tmrMsec    [Ntmr];
      int           tmrState   [Ntmr];

enum { Off = HIGH, On = LOW };

// -----------------------------------------------------------------------------
void
loop (void)
{
    unsigned long msec = millis ();

    for (unsigned n = 0; n < Ntmr; n++)  {
        if ((msec - tmrMsec [n]) >= tmrMsecPer [n])  {
            tmrMsec [n] += tmrMsecPer [n];

            if (Off == tmrState [n])  {
                tmrMsecPer [n] = TmrMsecOn [n];
                tmrState   [n] = On;
            }
            else {
                tmrMsecPer [n] = TmrMsecOff [n];
                tmrState   [n] = Off;
            }
            digitalWrite (TmrPin [n], tmrState [n]);
        }
    }
}

// -----------------------------------------------------------------------------
void
setup (void)
{
    Serial.begin (9600);

    for (unsigned n = 0; n < Ntmr; n++)  {
        tmrState   [n] = Off;
        tmrMsecPer [n] = TmrMsecOff [n];
        digitalWrite (TmrPin [n], tmrState [n]);
        pinMode      (TmrPin [n], OUTPUT);
    }
}

@paulpaulson

can you please explain what you exactly mean with this sentence?
Does this mean a structured array is a class?

I assume that @StefanL38 meant is that a struct is the same as a class except that all its member variables and functions are public

https://www.learncpp.com/

To me this says:

I will not explain what it is. Learn it yourself. Very helpful indeed.

Those who can read have a clear advantage.

just another variant of R***

You did not even try to type some letters that must be READ to explain it.

Arguments should be conducted in MSpaint drawings. No words allowed.

A 2 value array can shrinkify your code, and yeah you maybe don't know about arrays yet but let us know what this means to you?

// I'm doing this unpacked for clarity's sake:

const unsigned long interval[ 2 ]; // holds 2 values for off and on time
interval[ 0 ] = 500;
interval[ 1 ] = 1000;
// I can do this
byte index = 0;
// interval[ index ] is 500
// or this
index = 1;
// interval[ index ] is 1000

// and this
if ( millis() - startMs >= interval[ index ] )

// then index tells me if the led is LOW or HIGH so
{
index = 1 - index;
digitalWrite( ledpin, index );
startMs += interval[ index ];
}

Special note: ALWAYS use unsigned values for time!
The bug won't show up right away, but in time using long it will.
That time won't be for just under 25 days but when you time with 16 bit values using unsigned int times, an int will screw you in less than 33 seconds. Just make using unsigned for time a habit!

With many leds you can make the array 2 dimensions

const unsigned long interval[ leds ][ 2 ];

and have an array of indexes for which state by which led.

I'm running Raspbian Linux, not Windoze!

... in GIMP...

Hello @UKHeliBob, @Delta_G, @van_der_decken, @alto777, @paulpaulson, @xfpd, @gcjr, @stefanL38 and the Arduino forum,

Special shout out to paulpaulson for providing the tidied up example derived from the sketch copied into the original post, my feeble attempt.

Also thanks xfpd for the Millis() Arduino Function: 5+ Things To Consider
at Programming Electronics Academy web site. Gained some insights there.

I am not a programmer.
I am an architect so I get a little lost
trying to take descriptions of changes to the code such as the
excellent suggestion made by UkHeliBob and applying the suggestion to the
Blink_1_wo_Delay_Attiny85_two_intervals_2400615.ino.

The code provided by paupaulson, copied herewith below as
Blink_1_wo_Delay_Attiny85_two_intervals_2400617.ino
was compiled and loaded and works as designed.

A limited grasp of variables and their types as been
assimilated. Still working on arrays.

In the statement
const unsigned long intervals[] {intervalOff, intervalOn};
it seems that an array is created, intervals, with two
values, intervalOff, intervalOn.
Then intervals array is the compared to the difference
between currentMillisOn and previousMillisOn at line 38:
if (currentMillisOn - previousMillisOn >= intervals[ledState])

It is seen that ledState is used in this comparison
but ledStateOff is not found in any statement
beyond line 15 where it is first called.

Three questions please:

  1. If the the comparison
    if (currentMillisOn - previousMillisOn >= intervals[ledState])
    only calls ledState how is ledStateOff ever get called?

  2. It is recognized that using arrays is a superior method
    of coding as opposed to declaring separate variables with
    values contained in an array. But to better
    understand what the array is doing an attempt
    was made to desconstruct the array statement
    const unsigned long intervals[] {intervalOff, intervalOn};
    as two variables, say,
    const unsigned long intervalOpen = intervalOff
    const unsigned long intervalClosed = intervalOn
    But this only seems to duplicate
    const long intervalOn = 2000;
    const long intervalOff = 250;
    and so is redundant.
    And not sure how to substitute separate variables
    for ledState in the comparison:
    if (currentMillisOn - previousMillisOn >= intervals[ledState])
    So is there a way of writing the code without an array?

  3. It seems like the code is still using
    // if the LED is off turn it on and vice-versa:

  • if (ledState == LOW) {*
  •  ledState = HIGH;*
    
  • } else {*
  •  ledState = LOW;*
    
  • }*
    to toggle between on and off. If that is
    true, how are intervals different?

Ok, after studying papupalson's sketch
for another hour it maybe that because the
currentMillisOn - previousMillisOn comparison
is an IF statement that calls ledState
if currentMillisOn - previousMillisOn is true,
then somehow if the comparison is not true
it calls ledStateOff. Right? (Sorry, thats
a fourth question.)

Thanks

Allen in Dallas

/* Blink without Delay
  based on documentation found at
  https://docs.arduino.cc/built-in-examples/digital/BlinkWithoutDelay/
  and
  Blink_1_wo_Delay_Attiny85_240402.ino
  Blink_1_wo_Delay_Attiny85_two_intervals_2400617.ino (this sketch)
  uses two intervals so the On interval can be different from the Off interval.
*/

// constants won't change. Used here to set a pin number :
const int ledPin3 = 3;  // the number of the LED pin

// Variables will change :
int ledState = LOW;
int ledStateOff = LOW;


unsigned long previousMillisOn = 0;
unsigned long previousMillisOff = 0;

// constants won't change :
const long intervalOn = 2000;
const long intervalOff = 250;
const unsigned long intervals[] {intervalOff, intervalOn};

void setup() {
  // set the digital pin as output:
  pinMode(ledPin3, OUTPUT);
}

void loop() {
  runLED3();
}

void runLED3() {
  unsigned long currentMillisOn = millis();

  if (currentMillisOn - previousMillisOn >= intervals[ledState])
  {
    // save the last time you blinked the LED
    previousMillisOn = currentMillisOn;

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW) {
      ledState = HIGH;
    } else {
      ledState = LOW;
    }
    // set the LED with the ledState of the variable:
    digitalWrite(ledPin3, ledState);

  }
}
 

For inspiration, maybe take a look at the several different ways of doing Blink Without Delay with multiple LEDs in:

Oops. I meant:

The intervals are different because of this intervals[] array:

That sets intervals[LOW] = 250 and intervals[HIGH] = 2000, through taking advantage of some extra knowledge that LOW = 0 = false and HIGH = 1 = true.