'IF' condition does not work as expected with MILLIS()

shastri91 · June 29, 2020, 12:31pm

Hi,

I am making a UV disinfectant BOX using OSRAM 254nm wavelength light.

Intended operation,

When the start button is pressed and the reed switch ON.
2 relays should turn ON for 120sec I.e 2min.
If the start button is pressed again during the count down, another 60sec should be added to the down count. (if the start button is pressed twice 120sec should be added)
And if the reed switch goes OFF in between the down count, both the relay should be turned off.

I have made an effort to put up a basic code, I need the forums to help to complete the code part

as I have a good understanding of hardware.

This is initial code I have written. It works but i know it can be written well and it does not have No 3) function.

#include <stdio.h>
#include <Wire.h>               // Library for I2C
#include <LiquidCrystal_I2C.h> // Library for LCD

const long interval = 500;           // Interval at which buzzer beeps

// Input and output Pins
int ButtonState = 2;           // Interrupt input Pin D2 on Nano for Button switch
int ReedSwitch = 3;            // Interrupt input Pin D3 on Nano for magnetic reed switch
int FanRelay = 7;              // Output Pin D7 on Nano for fan relay
int LightRelay = 8;            // Output pin D8 on Nano for light relay
int BuzzerPin = 6;             // Output pin D6 on Nano for Buzzer

int Seconds;

int logic1 = 0;
int logic2 = 0;

int BuzzerState = LOW;

LiquidCrystal_I2C lcd(0x27, 16, 2);

void setup()
{
  pinMode(FanRelay, OUTPUT);
  pinMode(LightRelay, OUTPUT);
  pinMode(BuzzerPin, OUTPUT);
  pinMode(ButtonState, INPUT);
  pinMode(ReedSwitch, INPUT);
  digitalWrite(FanRelay, HIGH);
  digitalWrite(LightRelay, HIGH);
  digitalWrite(BuzzerPin, LOW);
  lcd.init();
  lcd.backlight();
  lcd.clear();
  lcd.setCursor(0, 0);
  lcd.print("UV SANITIZER BOX");
  delay(3000);
}


void splashscreen()
{
  lcd.clear();
  lcd.setCursor(0, 0);
  lcd.print("2 Min SANITATION");
  lcd.setCursor(0, 1);
  lcd.print("* Press Start *");
  delay(100);
}

void DisplayTime()
{
  lcd.clear();
  lcd.setCursor(0, 0);
  lcd.print(" Time Remaining");
  lcd.setCursor(0, 1);
  lcd.print("Seconds - ");
  lcd.setCursor(10, 1);
  lcd.print(Seconds);
}

void DisplayInterrupt()
{
  lcd.clear();
  lcd.setCursor(0, 0);
  lcd.print("UV SANITATION");
  lcd.setCursor(0, 1);
  lcd.print("Interrupted!!!!!");
  delay(3000);
}

void DisplayFinish()
{
  lcd.clear();
  lcd.setCursor(0, 0);
  lcd.print("UV SANITATION");
  lcd.setCursor(0, 1);
  lcd.print("****COMPLETE****");
  delay(5000);
}

void loop()
{
  splashscreen();
  logic1 = digitalRead(ButtonState);
  logic2 = digitalRead(ReedSwitch);

  if (logic1 == 1 && logic2 == 1 )
  {
    for (int Countdown = 240; Countdown >= 0; Countdown --)
    {
      digitalWrite(LightRelay, LOW);
      digitalWrite(FanRelay, LOW);
      tone(BuzzerPin, 500);
      delay(500);
      Seconds = (Countdown / 2);
      DisplayTime();

      logic2 = digitalRead(ReedSwitch);

      if (logic2 == 0 )
      {
        digitalWrite(LightRelay, HIGH);
        digitalWrite(FanRelay, HIGH);
        DisplayInterrupt();
        break;
      }
    }
    DisplayFinish();

  }
  else {

    digitalWrite(LightRelay, HIGH);
    digitalWrite(FanRelay, HIGH);
  }
}

I want to use MILLIS() for the delay.
In the below code “if ((logic1 == 1) && (logic2 == 1))” works fine only when Both logic1 & logic2 are HIGH. when logic2 is LOW and logic1 is HIGH the relay turns on for the duration logic1 is HIGH.

void loop()
{

logic1 = digitalRead(ButtonState);
logic2 = digitalRead(ReedSwitch);
    if ((logic1 == 1) && (logic2 == 1))
    {
        digitalWrite(FanRelay, HIGH);
        offat = millis()+5000;
        Serial.print(offat);
    }
    if (digitalRead(FanRelay) == HIGH)
    {
        logic2 = digitalRead(ReedSwitch);
        if((millis() >= offat) || (logic2 == LOW))
        {
          digitalWrite(FanRelay, LOW);  
        }
    }   
}

Thank you

Robin2 · June 29, 2020, 1:22pm

You should always use millis() in this style of code

if (millis() - timeStarted >= desiredInterval) {

because using subtraction ensures that the timing works correctly whenever millis() overflows back to 0

Have a look at the code in Several Things at a Time

...R

wildbill · June 29, 2020, 1:27pm

You may have floating inputs.do you have pull downs on your buttons? If not, consider changing pinmode to input_pullup.

shastri91 · June 29, 2020, 3:43pm

Hello,

I am trying with
if (millis() - timeStarted >= desiredInterval) from two days not but unable to understand it. It is not behaving as I expected. Im confused!!

What happens is, to turn on the relay I have to press and hold the button for the amount of “desiredInterval” and after that, it will be ON for “desiredInterval”.

I have an external pulldown resistor and when I press the button the input pin goes high.

Will keep trying.

Robin2 · June 29, 2020, 4:03pm

shastri91:
I am trying with
if (millis() - timeStarted >= desiredInterval) from two days not but unable to understand it. It is not behaving as I expected. Im confused!!

Please post the complete program in which you tried it. There may be some simple error.

...R

Perehama · June 29, 2020, 6:41pm

millis() can return a number from 0 to 4294967295, but unsigned. This is not the same thing as positive, or absolute value. unsigned means that 0 == 4294967295 +1. Now, let's say millis() is 5000 away from rolling over when we get to the line

offat = millis()+5000;

now offat = 0. So we come to the following line

if((millis() >= offat) || (logic2 == LOW))

which is true for all values when offat = 0.

If we change the second line to

if((millis() - offat >= 5000) || (logic2 == LOW))

and the first line to

offat = millis();

we have achieve the same expected result without the bug.

Also when you

logic1 = digitalRead(ButtonState);

instead of just using a local variable, or using digitalRead() inline, you have to remember to set it again and again, and if you forgot somewhere in your code, it will affect the conditionals based on it.

femmeverbeek · June 29, 2020, 7:14pm

Sorry to say but you started wrong. You are building a user interface, which means that the program must always be ready to react on an event.
The events are changes in state of the button, the reed switch and the timer. For each of them you need to have their last status.
e.g.
Boolean lastButtonState=false,
lastReedState=false,
lastTimerExpired=true;
Unsigned long desiredInterval = 0;
All that happens in the main loop of your program is constantly checking the status of the three events.

e.g.

Void loop()
{
   checkButton();
   checkReed();
   checkTimer();
}

The rest of the code is just happening inside the three event handlers. You must write it in such a way that they can be called over and over again, excecuting only once when the status changed. Never blocking the code. That means that you can never use the delay() as this will hold-up the program.

e.g. (untested)

Check button controls the timer

void checkButton()
{
if( lastButtonState != digitalRead(ButtonState)      )   // true when status of the button changed
{  
    lastButton = digitalRead(ButtonState);                  // update the status
    if ( lastButtonState && lastReedState   )
    {
           if (timerExipered)
          {
             timeStarted=millis();
             desiredInterval = 120000;
            }
           else
           desiredInterval += 60000;
     }
}

checkTimer controls the relays

void checktimer()
{
    boolean expired = (millis() - timeStarted > desiredInterval);          // not >= as this may inadvertently trigger at startup
    if (expired != lastTimerExpired)                                                  //change in timerExpired state
    { expired != lastTimerExpired
        if (expired) digitalWrite(FanRelay, LOW); 
        else           digitalWrite(FanRelay, HIGH);  
    }
}

For you to find out what the checkReed() should be doing.

system · May 6, 2021, 12:37am