Failing if statements using millis vs. delay

Hello All. It's been a long time since I've been here, but I'm involved in a project that has Arduino written all over it.
I have a stairway with battery powered illuminated globes controlled by RF(433mhz).
For years they have been solely controlled by a simple remote; manually switching them on at dark and then off sometime after daylight. It's a waste of battery that requires recharging about every three weeks.
I now have three PIRs that output an RF signal. I've captured those signals and will be using them to trigger the globes on for a few minutes. I still want to use the remote to turn them on for aesthetics.
I have two flags, REMOTE_ACTIVE and DARK, that enable/disable any action triggered by the PIRs.
There is an if statements for each PIR and one (TEST) that set their own flags. Those flags are used to control the globes and led indicators. These if statements are my problem. Prior to the if statements, the flags are properly set but I have not been able to have the flags go false after a delay 'flag_timeout'. I have print statements that show that the start time for the timeout is correct, yet when millis surpasses start time + flag_timeout the flag does not go false which would turn the globes off and extinguish a respective led. I know you've been here before reading the same problem. My internet searches seem to indicate I'm doing it right but evidently not. Please help me move on. - Scotty

#include <RCSwitch.h>
RCSwitch mySwitch = RCSwitch();




#define NANO_LED 13
#define DARK_LED 10
#define FIRST_FLOOR_LED 9
#define HIGH_POST_LED 8
#define LOW_POST_LED 7
#define REMOTE_ACTIVE_LED 6
#define TEST_SIGNAL_LED 5
boolean DARK = false;
boolean REMOTE_ACTIVE = false;
boolean FIRST_FLOOR = false;
boolean LOW_POST = false;
boolean HIGH_POST = false;
boolean TEST = false;
unsigned long remote_active_start_time;
unsigned long dark_start_time;
unsigned long first_floor_start_time;
unsigned long low_post_start_time;
unsigned long high_post_start_time;
unsigned long test_start_time;
unsigned long flag_timeout = 2000;
void FLAGS();
void LED_CONTROL();
void PRINT();
int led_pins[] = { DARK_LED, FIRST_FLOOR_LED, LOW_POST_LED, HIGH_POST_LED, REMOTE_ACTIVE_LED, TEST_SIGNAL_LED };
int num_led_pins = 6;


void setup() {
  Serial.begin(9600);
  mySwitch.enableReceive(0);  // Receiver on interrupt 0 => that is pin #2
  mySwitch.enableTransmit(18);
  for (int PM = 0; PM < num_led_pins; PM++) {
    pinMode(led_pins[PM], OUTPUT);
    digitalWrite(led_pins[PM], HIGH);
    delay(500);
    digitalWrite(led_pins[PM], LOW);
  }
}

void loop() {
  // put your main code here, to run repeatedly:
  FLAGS();
  LED_CONTROL();
  PRINT();
}
void FLAGS() {
  if (mySwitch.getReceivedValue() == 6727633) {  //remote 'on' button depressed
    REMOTE_ACTIVE = true;
    remote_active_start_time = millis();
    mySwitch.resetAvailable();
  }
  if (mySwitch.getReceivedValue() == 6727634) {  //remote 'off' button depressed
    REMOTE_ACTIVE = false;
    mySwitch.resetAvailable();
  }
  if (mySwitch.getReceivedValue() == 11111111) {  //sensor reporting DARK
    DARK = true;
    mySwitch.resetAvailable();
  }
  if (mySwitch.getReceivedValue() == 1111112) {  //sensor reporting LIGHT
    DARK = false;
    mySwitch.resetAvailable();
  }
  if (mySwitch.getReceivedValue() == 13880430) {
    FIRST_FLOOR = true;
    first_floor_start_time = millis();
    mySwitch.resetAvailable();
    if (millis() >= first_floor_start_time + flag_timeout) {
      FIRST_FLOOR = false;
    }
  }
  if (mySwitch.getReceivedValue() == 123913) {
    LOW_POST = true;
    low_post_start_time = millis();
    mySwitch.resetAvailable();
    if (millis() >= low_post_start_time + flag_timeout) {
      LOW_POST = false;
    }
  }
  if (mySwitch.getReceivedValue() == 687113) {
    HIGH_POST = true;
    high_post_start_time = millis();
    mySwitch.resetAvailable();
    if (millis() >= high_post_start_time + flag_timeout) {
      HIGH_POST = false;
    }
  }
  if (mySwitch.getReceivedValue() == 4157224) {  //sensor reporting DARK
    //DARK = true;
    TEST = true;
    test_start_time = millis();
    mySwitch.resetAvailable();

    if (millis() >= test_start_time + flag_timeout) {  // flag_timeout
      digitalWrite(LOW_POST_LED, HIGH);                //TEST = false;
      TEST = false;
    }
  }
}

void LED_CONTROL() {
  unsigned long leds_off_current_time = millis();
  //Serial.println("   IN LED CONTROL");

  digitalWrite(DARK_LED, DARK);
  digitalWrite(REMOTE_ACTIVE_LED, REMOTE_ACTIVE);
  digitalWrite(FIRST_FLOOR_LED, FIRST_FLOOR);
  digitalWrite(LOW_POST_LED, LOW_POST);
  digitalWrite(HIGH_POST_LED, HIGH_POST);
  digitalWrite(TEST_SIGNAL_LED, TEST);
}

void PRINT() {
  unsigned long print_previous_time = 0;
  unsigned long print_current_time = millis();
  int print_delay = 2000;
  if (print_current_time - print_previous_time >= print_delay) {
    Serial.print("MILLIS  ");
    Serial.println(millis());
    //Serial.print(" ");
    //Serial.println(DARK);
    //Serial.print("DARK ");
    //Serial.println(DARK);
    Serial.print("REMOTE ACTIVE ");
    Serial.println(REMOTE_ACTIVE);
    Serial.print("TEST SIGNAL ");
    Serial.println(TEST);
    Serial.print("TEST START_TIME ");
    Serial.println(test_start_time);
    //Serial.print("SIGNAL RECEIVED ");
    //Serial.println(SIGNAL_RECEIVED);
    //Serial.print("CURRENT TIME ");
    // Serial.println(current_time);
    //Serial.print("SIGNAL RECEIVED START TIME ");
    //Serial.println(signal_received_start_time);
    // Serial.print("GLOBES ON TIME ");
    //Serial.println(globes_on_time);
    //Serial.print("REMOTE DEPRESSED TIME ");
    //Serial.println(REMOTE_DEPRESSED_TIME);
    //Serial.print("leds off current time ");
    //Serial.println(leds_off_current_time);
    Serial.print("TEST START TIME ");
    Serial.println(test_start_time);
  }
  print_previous_time = millis();
}

Let's just look at one of these since they're all the same.

First, you set first_floor_start_time to the current value of millis().

Then you test millis() to see if it's greater than or equal to the value you just set it to, plus 2000 milliseconds.

Side note: unless you want weird trouble every 49 days or so, subtract rather than add millis values, e.g.: if (millis() - first_floor_start_time >= flag_timeout) {

Anyway, getting back on track. Unless your call to myswitch.resetAvailable() doesn't return for more than 2000 ms, your if test will always fail.

Thanks for replying. I changed flag_timeout to 30000. Same results. - Scotty

So now your if test will always fail unless your call to myswitch.resetAvailable() takes more than 30000 milliseconds, or 30 seconds.

Do you really think that's likely?

Put simply, your code is doing this:

   i = 1;
   if (i >= 2) {
      // do something if the impossible happens
   }

That is never, ever, going to work, is it?

it looks like all your timers depend on receiving an continually receiving an RF code. A more common approach isi to recognize and event that changes state and timers run within some state

i'm having a hard time figuring out what your code is doing. I would expect some signal to turn things on or off or that a timer turns things off after some period.

look this over

  • it uses buttons instead of RF codes as commands
  • it uses a switch statement instead of multiple if statements
  • it uses a a flag, active to enable a timer that runs outside of the receiving any command (i.e. button presss)

hopefully you can see from it how to solve your problem and possibly make your code simpler

const byte PinLeds [] = { 12, 11, 10 };
const int  Nleds      = sizeof(PinLeds);

const byte PinTestLed = 13;

enum { Off = HIGH, On = LOW };

const byte PinButs [] = { A1, A2, A3 };
const int  Nbuts      = sizeof(PinButs);
      byte butState [Nbuts];

const unsigned long MsecPeriod = 3000;
      unsigned long msec0;
      unsigned long msec;

bool active;

// -----------------------------------------------------------------------------
void allLeds (
    int  val)
{
    for (int n = 0; n < Nleds; n++)
        digitalWrite (PinLeds [n], val);
}

// -----------------------------------------------------------------------------
// check for button presses
const int NoBut = -1;

int
chkButs ()
{
    for (int n = 0; n < Nbuts; n++)  {
        byte but = digitalRead (PinButs [n]);
        if (butState [n] != but)  { 
            butState [n]  = but;
            delay (30);                 // ignore bounce

            if (LOW == but)
                return n;
        }
    }

    return NoBut;
}

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

    if (active && msec - msec0 >= MsecPeriod)  {
        active = false;
        allLeds (Off);
    }

    switch (chkButs ())  {
    case 0:                     // turn 1 LED on
        digitalWrite (PinLeds [0], On);
        active = true;
        msec0  = msec;
        break;

    case 1:                     // turn all LEDs On
        allLeds (On);
        active = true;
        msec0  = msec;
        break;

    case 2:                     // toggle the test-LED
        digitalWrite (PinTestLed, ! digitalRead (PinTestLed));
        break;
    }
}

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

    for (int n = 0; n < Nbuts; n++)  {
        pinMode (PinButs [n], INPUT_PULLUP);
        butState [n] = digitalRead (PinButs [n]);
    }

    for (int n = 0; n < Nleds; n++)
        pinMode      (PinLeds [n], OUTPUT);
    pinMode      (PinTestLed, OUTPUT);
    digitalWrite (PinTestLed, Off);

    allLeds (Off);
}