Help with using which store time to achieve what i needed?

Projects Programming
1

Hi everyone,

i'm trying to make a project to turn off the LED for 1 min (5sec for testing) if detected a HIGH input, turn on the LED if detected LOW input and blink the LED if it is turn on for more than 3 mins(20sec for testing). I'm stuck at this coding for days but still can't figure out how to correct it. Any help would be great!

Below is the coding that i came up.

unsigned long offTime[4];                      //Time individual input are turn "ON/HIGH"
unsigned long blink_interval = 1000;
unsigned long LED_ontime [4];                  //Time individual input are left idle
unsigned long LED_blinktime [4];               //Time individual light last blinked
unsigned long idle_interval = 20000;          //20sec for testing
unsigned long interval = 5000;                //5sec for testing

int box[4];                               //Array for 4 box
int pinCount = 4;                             //Assign pinCount as 4
int outputPins[] = {5, 4, 3, 2};              //Array for output pins in sequence 
int inputPins[] = {A0, A1, A2, A3};           //Array for input pins in sequence


void setup() {

  for (int thisPin = 0; thisPin < pinCount; thisPin++)
      {
      pinMode(inputPins[thisPin], INPUT);
      }
  for (int thisPin = 0; thisPin < pinCount; thisPin++) 
      {
      pinMode(outputPins[thisPin], OUTPUT);      
      }
    Serial.begin(9600);
}

void loop() {

  for (int i = 0; i < pinCount; i++)
  {
  chamber[i] = digitalRead (inputPins[i]);      
  }
  
    unsigned long currentMillis = millis();     //start millis
    for (int i = 0; i < pinCount; i++)          //use a for loop to check all 4 box
    {
      if(chamber[i] == HIGH)                      //if input == HIGH
      {
      digitalWrite(outputPins[i], LOW);           //Set output==LOW to turn off the LED
      offTime[i] = currentMillis;                 //Set offTime to current time when it is turned OFF/LOW
      } 
      if( (chamber[i] == LOW) && (currentMillis - offTime[i] > interval) )    
      //if input==LOW and if time is more than interval
      {
      digitalWrite(outputPins[i], HIGH);          //Set output==HIGH to turn on the LED
      LED_ontime[i] = currentMillis - offTime[i]  ;             //Save the time the LED is last turn on
      }
      if (((currentMillis - LED_ontime[i]- offTime[i] > idle_interval) && (currentMillis - LED_blinktime[i] > blink_interval))|| ((LED_ontime[i] > idle_interval) && (offTime[i] == 0) && (currentMillis - LED_blinktime[i] > blink_interval)))
          {
          LED_blinktime[i] = currentMillis;
          if (digitalRead(outputPins[i]) == LOW)
          {
           digitalWrite(outputPins[i], HIGH);
          }
          else digitalWrite(outputPins[i], LOW);
          }
    }
}

Thanks for reading and any input given! :slight_smile:

2

The code does something. You forgot to tell us what it actually does, or how that differs from what you want.

      pinMode(inputPins[thisPin], INPUT);

How ARE the switches wired?

Your random indentation leaves a lot to be desired. Use Tools + Auto Format before posting any more code.

3

A pin being HIGH or LOW isn't a defined moment in time. A pin becoming HIGH or LOW is :wink:

So think what you mean is:

Fearlessly:
i'm trying to make a project to turn off the LED if the input turns HIGH

and turn on again after 1 min (5sec for testing).

Or turn on the LED immediately if the input comesLOW

and blink the LED if it is turn on for more than 3 mins(20sec for testing). (aka, the moment is become turned on is 3 minutes away)

This rephrasing already makes it more clear what you want to do and a makes it easier to make the code for it.

For starters, you want to use state change detection for the buttons. Aka, have a variable to hold the previous value of the input.

This also makes it clear we have 3 (or 4) time periods/intervals:

  1. 3 minutes => MaxOnInterval = 20000; //3 * 6000UL;
  2. 1 minuut => MaxOffInterval = 5000; //60000;
  3. 1 seconds => BlinkInterval = 1000;

What might be less obvious is we only need to remember a single "event" (time) per led (let's call that ledMillis). When it turned on or when it turned off. But we want to remember in what "state" the led should be in: on, off or blinking (let's call that ledState).

Pseudo code:

if(inputChanged){
  ledMillis = millis();
  if(input){
      digitalWrite(LedPin, LOW);
    ledState = LedOff
  else{
      digitalWrite(LedPin, HIGH);
    ledState = LedOn
}

if(ledState == LedOff && millis() - ledMillis > MaxOffInterval){
  ledMillis = millis();
  digitalWrite(LedPin, HIGH);
  ledState = LedOn;
}

if(ledState == LedOn && millis() - ledMillis > MaxOnInterval){
  ledMillis = millis();
  ledState = LedBlink;
}

if(ledState == LedBlink && millis() - ledMillis > BlinkInterval){
  ledMillis = millis();
  toggle(LedPin);
}

What was not answered, what do you wan to happen if the input was high for more than 1 minute (aka, led is turned on again) and you turn the input low. Above (pseudo) code makes the led turn to ON an reset the counting.

And yes, you need a pull up/down resistor on your inputs if they are switches. Easiest way is with the internal pull ups.

Other approach would be to store two time events:

  • When the led turned on or off
  • when the input turned on or off
4

Sorry, it does turn on at the start and off when a HIGH input is triggered, but the blinking function does not work. There is 4 inputs and 4 outputs correspondingly.

The switch are wired at the input pin of 5~2 and output at A0~A3 connecting to pull down resistor and to opto isolator chip then to LED.

I'm using for loop as i need to continuously check all 4 inputs. Thanks for the inputs, i'm still trying to digest it

5

Fearlessly:
Sorry, it does turn on at the start and off when a HIGH input is triggered, but the blinking function does not work.

What is "it"?

If it is the code from to start post, I scanned it and don't see state change detection. Might be in the complex way you do things but I think it's better to take a step back and rethink (with the knowledge of state change detection) about a solution.

Fearlessly:
There is 4 inputs and 4 outputs correspondingly.

That's just multiplying by four. Aka, not relevant for the question.

Fearlessly:
The switch are wired at the input pin of 5~2

So where are the pull resistors?

Fearlessly:
and output at A0~A3 connecting to pull down resistor and to opto isolator chip then to LED.

Pull down? Opto? Why?

Fearlessly:
I'm using for loop as i need to continuously check all 4 inputs. Thanks for the inputs, i'm still trying to digest it

Drop the whole "4 times/4switches/4 leds" thing for now. Focus on a single instance. If you have that without using delay(), it's a piece of cake to multiply that for four, five, ten, twenty.....

6

What is "it"?

Refers to LED

If it is the code from to start post, I scanned it and don't see state change detection. Might be in the complex way you do things but I think it's better to take a step back and rethink (with the knowledge of state change detection) about a solution.

Yup, i think i think in too complex manner, i shall do 1 at a time first. Thanks!

7

septillion:
Pseudo code:

if(inputChanged){

ledMillis = millis();
 if(input){
     digitalWrite(LedPin, LOW);
   ledState = LedOff
 else{
     digitalWrite(LedPin, HIGH);
   ledState = LedOn
}

if(ledState == LedOff && millis() - ledMillis > MaxOffInterval){
 ledMillis = millis();
 digitalWrite(LedPin, HIGH);
 ledState = LedOn;
}

if(ledState == LedOn && millis() - ledMillis > MaxOnInterval){
 ledMillis = millis();
 ledState = LedBlink;
}

if(ledState == LedBlink && millis() - ledMillis > BlinkInterval){
 ledMillis = millis();
 toggle(LedPin);
}

Hi so for LedState do i need to use state machine?

I'm currently trying this code but not sure what to declare for LedState, LedOn, LedOff & LedBlink.

Current coding i'm using

Void loop() {
   unsigned long LedMillis = millis();
   If(digitalRead (A0) ==HIGH)
   {
    digitalWrite(5, LOW)
    LedState = LedOff;
   }
   else
   {
     digitalWrite(5, HIGH);
     LedState = LedOn;
    }

   if(LedState == LedOff && millis() - LedMillis > MaxOffInterval)
  {
  LedMillis = millis();
  digitalWrite(5, HIGH);
  LedState = LedOn;
   }

   If (LedState == LedOn && millis() - LedMillis >  MaxOnInterval)
   {
     LedMillis = millis();
     LedState = LedBlink;
    }

    If ( LedState == LedBlink && millis() - LedMillis> BlinkInterval)
   {
    digitalWrite(5, !digitialRead(5));
    }

Currently i only declare as char for the LedState, LedOn, LedOff & LedBlink.

Above code allows the LED to turn on when my input is low and turn off the LED when input is high, but this code doesn't wait for the MaxOffInterval of 5secs and doesn't blink the led when over MaxOnInterval of 20secs. Which part did i do wrong for here? Please advice if possible! Thanks!

8

Yeah, with ledState you make a simple state machine. I just made pseudo code so it just indicates the task. You can use any variable type you like. But as for every variable, try to use the smallest possible. So a char or byte are logical choice. But you would need to define values for 'LedOn','LedOff' and 'LedBlink' as well.

An option would be to use a enum for it:

enum ledState_t{LedOn, LedOff, LedBlink};
ledState_t ledState = LedOn;

Couple of think to note about your code:

  • Please don't post snippets
  • Arduino is case sensitive, so If and Void are not the same as if and void.
  • LedMillis needs to be global because you want to know the time next loop as well.
If(digitalRead (A0) ==HIGH)
  • is still not using state change detection. You want to detect the moment the switch becomes HIGH / LOW. Try to think in defined moments that will only happen once if something happens. For example, if you drive to work, the moment you drive to work is not a single moment, it's a time span. But the moment you start drivinng to work is a defined moment that only happens once. Same for arriving at work.
  • Doing it for a single led is not an excuse to not use a variable for it :wink:
  • Watch the indentation of the code. Very hard to read this way!
  • Although you are free to write variables every way you like it's easier if you pick a fixed way. Common is to write cammelCaseForVariable and UpperCammelCaseForConst (or other objects you can't change). For example:
unsigned long ledMillis; //will change during runtime
const byte LedPin = 5; //can not change during runtime
ledState_t ledState; //changes during runtime
ledState = LedOn;  //LedOn can't change during runtime
9

Sorry i'm not good in programming for now so i will just stick to what i know can work, but those that you suggested are great!

Below is the whole code:

unsigned long BlinkInterval = 1000;
unsigned long MaxOnInterval = 20000;           //3*60000
unsigned long MaxOffInterval = 5000;           //60000

unsigned long LedMillis;
enum LedState_t{LedOn, LedOff, LedBlink};
LedState_t LedState = LedOn;

int pinCount = 4;                             //Assign pinCount as 4
int outputPins[] = {5, 4, 3, 2};              //Array for output pins in sequence 
int inputPins[] = {A0, A1, A2, A3};           //Array for input pins in sequence


void setup() {

  for (int thisPin = 0; thisPin < pinCount; thisPin++)
      {
      pinMode(inputPins[thisPin], INPUT);
      }
 
  for (int thisPin = 0; thisPin < pinCount; thisPin++) 
      {
      pinMode(outputPins[thisPin], OUTPUT);      
      }
    Serial.begin(9600);
}

void loop() {

   millis();
   if(digitalRead(A0) == HIGH)   // if input is high
   {
    digitalWrite(5, LOW);         //set led as low
    LedState = LedOff;            //Assigned the state to be LedOff
    LedMillis = millis();
   }
   if ( (digitalRead(A0) == LOW) && (millis() - LedMillis > MaxOffInterval))  //if input is low & time exceed 5sec
   {
    digitalWrite(5, HIGH);        //set led as high
    LedState = LedOn;             //Assigned the state to be LedOn
    LedMillis = millis();         
   }
   if (( LedState == LedOn) && (millis() - LedMillis > MaxOnInterval)) //if LedState = LedOn & time exceed 20sec
   {
    LedState = LedBlink;          //Assigned the staet to be LedBlink
    LedMillis = millis();
   }
   if ((LedState = LedBlink) && (millis() - LedMillis > BlinkInterval)) //if LedState = LedBlink & interval of 1sec
   {
    digitalWrite(5, !digitalRead(5));     //toggle the output
    LedMillis = millis();
   }

Here is what i know works if turning on the Led and off the Led after 5 sec

void loop() {

   millis();
   if(digitalRead(A0) == HIGH)   // if input is high
   {
    digitalWrite(5, LOW);         //set led as low
    LedState = LedOff;            //Assigned the state to be LedOff
    LedMillis = millis();
   }
   if ( (digitalRead(A0) == LOW) && (millis() - LedMillis > MaxOffInterval))  //if input is low & time exceed 5sec
   {
    digitalWrite(5, HIGH);        //set led as high
    LedState = LedOn;             //Assigned the state to be LedOn
    LedMillis = millis();         
   }

But this part where i need it to blink after it exceed 20secs doesn't work

if (( LedState == LedOn) && (millis() - LedMillis > MaxOnInterval)) //if LedState = LedOn & time exceed 20sec
   {
    LedState = LedBlink;          //Assigned the staet to be LedBlink
    LedMillis = millis();
   }
   if ((LedState = LedBlink) && (millis() - LedMillis > BlinkInterval)) //if LedState = LedBlink & interval of 1sec
   {
    digitalWrite(5, !digitalRead(5));     //toggle the output
    LedMillis = millis();
   }

Can help me understand why i'm unable to make this part of the coding works?

is still not using state change detection. You want to detect the moment the switch becomes HIGH / LOW. Try to think in defined moments that will only happen once if something happens. For example, if you drive to work, the moment you drive to work is not a single moment, it's a time span. But the moment you start driving to work is a defined moment that only happens once. Same for arriving at work.

Sorry but i don't really understand how to use state change detection

10

Let's go through this backwards :smiley:

Fearlessly:
Sorry but i don't really understand how to use state change detection

Did you open up the State Change Detection example in the IDE? :wink:

Fearlessly:
But this part where i need it to blink after it exceed 20secs doesn't work
[...]
Can help me understand why i'm unable to make this part of the coding works?

Because from when do you want to time that? From when the light is on (time span) or from the moment the light turned on?

Or in the car example, if you want to eat a cookie if you drove for 10 minutes, you time that from the moment you started driving. Not from the last time you checked you where still driving :wink:

Quick note on the code:

  • Watch indentation! You make it harder for yourself if you don't. Compiler does not care. Press Ctrl+T in the IDE to make it help you.
  • small edit of the first part. Try to spot the improvements
//time constants
const unsigned long BlinkInterval = 1000;
const unsigned long MaxOnInterval = 20000;           //3*60000
const unsigned long MaxOffInterval = 5000;           //60000

//pin constants
const byte OutputPins[] = {5, 4, 3, 2};      //Array for output pins in sequence
const byte PinCount = sizeof(OutputPins);
const byte InputPins[PinCount] = {A0, A1, A2, A3};    //Array for input pins in sequence

unsigned long ledMillis;
enum ledState_t {LedOn, LedOff, LedBlink};
ledState_t ledState = LedOn;

void setup() {
  for (byte thisPin = 0; thisPin < PinCount; thisPin++)
  {
    pinMode(InputPins[thisPin], INPUT);
    pinMode(OutputPins[thisPin], OUTPUT);
  }

  Serial.begin(9600);
}
11

Did you open up the State Change Detection example in the IDE? :wink:

Yup just did, i'm able to understand the simple example but when it comes to including millis function, i got lost. :frowning:

Because from when do you want to time that? From when the light is on (time span) or from the moment the light turned on?

Or in the car example, if you want to eat a cookie if you drove for 10 minutes, you time that from the moment you started driving. Not from the last time you checked you where still driving :wink:

Oh in this case, i want the time from the last time the switch is pressed, not when the time when the switch is first pressed :slight_smile: kinda means the time it last drove?

//time constants

const unsigned long BlinkInterval = 1000;
const unsigned long MaxOnInterval = 20000;          //3*60000
const unsigned long MaxOffInterval = 5000;          //60000

//pin constants
const byte OutputPins[] = {5, 4, 3, 2};      //Array for output pins in sequence
const byte PinCount = sizeof(OutputPins);
const byte InputPins[PinCount] = {A0, A1, A2, A3};    //Array for input pins in sequence

unsigned long ledMillis;
enum ledState_t {LedOn, LedOff, LedBlink};
ledState_t ledState = LedOn;

void setup() {
  for (byte thisPin = 0; thisPin < PinCount; thisPin++)
  {
    pinMode(InputPins[thisPin], INPUT);
    pinMode(OutputPins[thisPin], OUTPUT);
  }

Serial.begin(9600);
}

I just learned that declaring const will make it read-only value, nice, and using bytes instead of int will take up less data space.

12

Fearlessly:
Yup just did, i'm able to understand the simple example but when it comes to including millis function, i got lost. :frowning:

Simply said, every loop() you read the switch, compare it to the value of the last time you read it (previous time through loop()) and check if it's the different. Only if it's different you do stuff. After which you save the value for use in the next time through loop().

Fearlessly:
Oh in this case, i want the time from the last time the switch is pressed, not when the time when the switch is first pressed :slight_smile: kinda means the time it last drove?

No, it means the last time you started driving :wink:

Fearlessly:
I just learned that declaring const will make it read-only value, nice, and using bytes instead of int will take up less data space.

:sunglasses:

13

Simply said, every loop() you read the switch, compare it to the value of the last time you read it (previous time through loop()) and check if it's the different. Only if it's different you do stuff. After which you save the value for use in the next time through loop().

Yup yup, which i i apply this thinking to the coding in void loop() but doesn't seems to work but i know this below part works,

void loop() {

  LedMillis = millis();
  if (digitalRead(A0) == HIGH)  // if input is high
  {
    digitalWrite(5, LOW);         //set led as low
    LedState = LedOff;            //Assigned the state to be LedOff
    LedMillis = millis();
  }
  if ( (digitalRead(A0) == LOW) && (millis() - LedMillis > MaxOffInterval))  //if input is low & time exceed 5sec
  {
    digitalWrite(5, HIGH);        //set led as high
    LedState = LedOn;             //Assigned the state to be LedOn
    LedMillis = millis();
  }

But not this below part

  if (( LedState == LedOn) && (millis() - LedMillis > MaxOnInterval)) //if LedState = LedOn & time exceed 20sec
  {
    LedState = LedBlink;          //Assigned the staet to be LedBlink
    LedMillis = millis();
  }
  if ((LedState = LedBlink) && (millis() - LedMillis > BlinkInterval)) //if LedState = LedBlink & interval of 1sec
  {
    digitalWrite(5, !digitalRead(5));     //toggle the output
    LedMillis = millis();
  }

Which to my understanding of the state change detection should be similar to this code. I can only conclude that i saved the LedMillis at some wrong timing. :confused:

14

Fearlessly:
Yup yup, which i i apply this thinking to the coding in void loop() but doesn't seems to work but i know this below part works,

I really don't see it. You only read the current state of the button.

It would look something like (based on previous code )

//time constants
const unsigned long BlinkInterval = 1000;
const unsigned long MaxOnInterval = 20000;           //3*60000
const unsigned long MaxOffInterval = 5000;           //60000

//pin constants
const byte OutputPins[] = {5, 4, 3, 2};      //Array for output pins in sequence
const byte PinCount = sizeof(OutputPins);
const byte InputPins[PinCount] = {A0, A1, A2, A3};    //Array for input pins in sequence

unsigned long ledMillis;
enum ledState_t {LedOn, LedOff, LedBlink};
ledState_t ledState = LedOn;
bool previousSwitchState;

void setup() {
  for (byte thisPin = 0; thisPin < PinCount; thisPin++){
    pinMode(InputPins[thisPin], INPUT);
    pinMode(OutputPins[thisPin], OUTPUT);
    previousSwitchStateState = digitalRead(InputPins[0]);
  }

  Serial.begin(9600);
}

void loop() {
  const byte CurrentSwitchState = digitalRead(InputPins[0]);
  if(CurrentSwitchState != previousButtonState){
    Serial.println("It changed!");
    //now remember it for next loop!
    previousSwitchState = CurrentSwitchState;
  }
  delay(200); //small delay against bounce. ONLY use for test! In your program you don't really care about bounce.  
}

And yeah, you save 'ledMillis' on the wrong moment. Look at my pseudo code, only save it when you actually change to the state led. That's the moment you want to remember.

15

Thank you for your guidance so far! This is the code that i edited

//Time constants
const unsigned long BlinkInterval = 1000;
const unsigned long MaxOnInterval = 20000;           //idle time to be 20mins = 1200000, 20secs for testing
const unsigned long MaxOffInterval = 5000;           //60000

//Pin constants
const byte outputPins[] = {5, 4, 3, 2};              //Array for output pins in sequence
const byte pinCount = sizeof(outputPins);                             //Assign pinCount as 4
const byte inputPins[pinCount] = {A0, A1, A2, A3};           //Array for input pins in sequence

unsigned long LedMillis;
enum LedState_t {LedOn, LedOff, LedBlink};
LedState_t LedState = LedOn;
bool previousSwitchState;

void setup() {
  for (byte thisPin = 0; thisPin < pinCount; thisPin++)
  {
    pinMode(inputPins[thisPin], INPUT);
    pinMode(outputPins[thisPin], OUTPUT);
    previousSwitchState = digitalRead(inputPins[0]);
    digitalWrite(outputPins[0], HIGH);
  }
  Serial.begin(9600);
}

void loop() {

  const byte CurrentSwitchState = digitalRead(inputPins[0]);
  if (CurrentSwitchState != previousSwitchState)
  {
    previousSwitchState = CurrentSwitchState;
    LedMillis = millis();
    if (CurrentSwitchState == HIGH)
    {
      digitalWrite(outputPins[0], LOW);
      LedState = LedOff;
    }
    else
    {
      digitalWrite(outputPins[0], HIGH);
      LedState = LedOn;
    }
  }
  if (LedState == LedOff && millis() - LedMillis > MaxOffInterval)
  {
    digitalWrite(outputPins[0], HIGH);
    LedState = LedOn;
    LedMillis = millis();
  } 

  if (LedState == LedOn && millis() - LedMillis > MaxOnInterval)
  {
    LedMillis = millis();
    LedState = LedBlink;
  }
  if ((LedState == LedBlink) && (millis() - LedMillis > BlinkInterval))
  {
    digitalWrite(outputPins[0], !digitalRead(outputPins[0]));
    LedMillis = millis();
  }
}

So far it works almost exactly as i want except for this part

if (LedState == LedOff && millis() - LedMillis > MaxOffInterval)
  {
    digitalWrite(outputPins[0], HIGH);
    LedState = LedOn;
    LedMillis = millis();
  }

If i leave the switch/button at on state, it will still turn on the Led after MaxOffInterval (5secs)

No, it means the last time you started driving :wink:

Does it align with this? I think currently its taking the first time started driving thou :confused:

P.S. may look abit messy as i'm using my phone to type, but i did use the ctrl-T function on IDE :slight_smile:

16

Fearlessly:
If i leave the switch/button at on state, it will still turn on the Led after MaxOffInterval (5secs)

Fearlessly:
i'm trying to make a project to turn off the LED for 1 min (5sec for testing) if detected a HIGH input

That's what you wanted, didn't you?