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

Use Control-T to auto-format your code and errors like this will be easy to spot.

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

1 Like

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);
    }
}

1 Like

@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/

1 Like

Was there something there that mis-defines a class to be a set of structured arrays?

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.

1 Like

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.

Add:

enum {
   ON_TIME = 0,
   OFF_TIME = 1
};

And you can:
interval[ledNumber][ON_TIME];
interval[ledNumber][OFF_TIME];

1 Like

I'm running Raspbian Linux, not Windoze!