monitoring switch state

Using Arduino Programming Questions
1

so im using a float sensor to detect when the water level in a tank is full "FloatSwitch LOW" or empty "FloatSwitch HIGH". if the floatswitch state dont change within the 40 minutes i set as the delay then it will change the state of another variable and sound an alarm. the problem is i keep getting floatswitch high alarm as if "FloatSwitchMillis = millis();" is being reset every time the state of the switch changes. can someone look at this piece of code and tell me if you see anything wrong based on how im trying to use it?

if ((digitalRead(FloatSwitch) == HIGH) && TimersEnabled) { // waterlevel low
    if (millis() - FloatSwitchMillis >= timerParams.FloatSwitchHIGHduration) { // 40 minute delay
      if (AlarmEnabled) {
        timerParams.alarmhighcounter++;
        FloatSwitchAlarmStatus = true;
        AlarmBuzzer = true;
        FloatSwitchMillis = millis();
      }
    }
  }
  if ((digitalRead(FloatSwitch) == LOW) && TimersEnabled) { //water level high
    if (millis() - FloatSwitchLowMillis >= timerParams.FloatSwitchLOWduration) {
      if (AlarmEnabled) {
        timerParams.alarmlowcounter++;
        FloatSwitchAlarmStatus = true;
        AlarmBuzzer = true;
        FloatSwitchLowMillis = millis();
      }
    }
  }
  FloatSwitchState = digitalRead(FloatSwitch);
  if (FloatSwitchState != lastFloatSwitchState) {
    if (FloatSwitchState) {
      timerParams.FloatSwitchCounter++;
      FloatSwitchStateCounter++;
      FloatSwitchLowMillis = millis(); //to reset float switch comp millis every time water level 
      FloatSwitchMillis = millis();  //to reset float switch comp millis every time water level changes
      FloatSwitchResetMillis = millis();
      //Serial.println("switch high counter");
      //Serial.println(FloatSwitchStateCounter);

    } else {
      timerParams.FloatSwitchCounter++;
      FloatSwitchLowMillis = millis(); //to reset float switch every time water level changes
      FloatSwitchMillis = millis();  //to reset float switch every time water level changes
    }
  }
  lastFloatSwitchState = FloatSwitchState;
2

Read Snippets R Us! and pinned posts on how to post a question to the forum... (those posts titled How to use this forum - please read. and Read this before posting a programming question)

For example I would question the type and scope of FloatSwitchMillis but I don’t have time for guessing what you didn’t share

3

The problem may be in the code that you did not show us or in the schematic or the wiring that you did not show us.

4

im not sure but i think the problem may be with "FloatSwitchMillis" not being updated to the current millis() at the right time. heres a short version that compiles, i want "FloatSwitchMillis" to be updated every time the FloatSwitch State changes. im pretty sure its correct?

int FloatSwitch = 11;
bool TimersEnabled = 1;
bool AlarmEnabled = true;
bool AlarmBuzzer = false;
bool FloatSwitchAlarmStatus = false;
bool FloatSwitchState = false;
bool lastFloatSwitchState = false;
int alarmlowcounter = 0;
int alarmhighcounter = 0;
int FloatSwitchCounter = 0;
int FloatSwitchStateCounter = 0;
unsigned  long FloatSwitchHIGHduration = 40;
unsigned  long FloatSwitchLOWduration = 40;
unsigned long FloatSwitchMillis = 0;
unsigned long FloatSwitchLowMillis = 0;

void setup() {
  Serial.begin(9600);
  pinMode(FloatSwitch, INPUT_PULLUP);
}

void loop() {
  if ((digitalRead(FloatSwitch) == HIGH) && TimersEnabled) { 
    if (millis() - FloatSwitchMillis >= FloatSwitchHIGHduration * 60000) {
      if (AlarmEnabled) {
        alarmhighcounter++; // this is the statement that keeps becoming activated
        FloatSwitchAlarmStatus = true;
        AlarmBuzzer = true;
        FloatSwitchMillis = millis();
      }
    }
  }
  if ((digitalRead(FloatSwitch) == LOW) && TimersEnabled) { 
    if (millis() - FloatSwitchLowMillis >= FloatSwitchLOWduration * 60000) {
      if (AlarmEnabled) {
        alarmlowcounter++;
        FloatSwitchAlarmStatus = true;
        AlarmBuzzer = true;
        FloatSwitchLowMillis = millis();
      }
    }
  }
  FloatSwitchState = digitalRead(FloatSwitch);
  if (FloatSwitchState != lastFloatSwitchState) {
    if (FloatSwitchState) { //if floatswitch HIGH then process these
      FloatSwitchCounter++;
      FloatSwitchLowMillis = millis(); //every time float switch state changes update this
      FloatSwitchMillis = millis();  //every time float switch state changes update this

    } else {  //then when FloatSwitch become LOW after HIGH
      FloatSwitchCounter++; //not sure if these three are in the right spot. 
      FloatSwitchLowMillis = millis(); //every time float switch state changes update this
      FloatSwitchMillis = millis();  //every time float switch state changes update this
    }
  }
  lastFloatSwitchState = FloatSwitchState;

}
5

if i juat want to update FloatSwitchMillis cant u just do this without the else statement

  FloatSwitchState = digitalRead(FloatSwitch);
  if (FloatSwitchState != lastFloatSwitchState) {
    if (FloatSwitchState) { 
      FloatSwitchCounter++; // leave this here for other reasons
    }
    FloatSwitchLowMillis = millis(); //every time float switch state changes update this
    FloatSwitchMillis = millis();  //every time float switch state changes update this
  }
  lastFloatSwitchState = FloatSwitchState;
6

With INPUT_PULLUP you get a LOW on a trigger not sure this is what you test against in your logic?

You are also likely to get bouncing and as you read multiple times digitalRead(FloatSwitch) you might deal with different values in the same loop and tens of HIGH LOW transitions in a few ms. Is there a capacitor built into your sensor ?

7

Your program looks over complicated.

Why not

start of loop()
  read the state of the switch
  if it has changed
    save the start time
  else
    if the current time - start time >= period then sound the alarm
  end if
end of loop()
8

J-M-L:
With INPUT_PULLUP you get a LOW on a trigger not sure this is what you test against in your logic?

You are also likely to get bouncing and as you read multiple times digitalRead(FloatSwitch) you might deal with different values in the same loop and tens of HIGH LOW transitions in a few ms. Is there a capacitor built into your sensor ?

surprisingly enough i dont have any bounce issues. but this statement

 if ((digitalRead(FloatSwitch) == HIGH) && TimersEnabled) { 
    if (millis() - FloatSwitchMillis >= FloatSwitchHIGHduration * 60000) {
      if (AlarmEnabled) {
        alarmhighcounter++; // this is the statement that keeps becoming activated
        FloatSwitchAlarmStatus = true;
        AlarmBuzzer = true;
        FloatSwitchMillis = millis();
      }
    }
  }

keep becoming triggered even after "FloatSwitchMillis = millis();" is updated from the floatswitch state change. it dont happen until at least floatswitchHIGHduration is achieved but im still confused why it ever happens at all becasue "FloatSwitchMillis = millis();" is updated on every state change right? and the state changes every 15 minutes. as of right now after 40 minutes when the floatswitch becomes HIGH the above statement is activated. the main program is huge but at this point i may need someone to step in

9

basically in my main program to sum it all up, the timer code is the core of the program. theres 3 timers timer1/2/3 they control pumps 1-2 and 3. when timer 1 is running pump 1 turns on and after 40 minutes the water level is low enough to activate a state change on the float switch. the float switch is monitoring the water level for all 3 pumps and will alert me if the timers are enabled but the water level hasnt changed in 40 minutes. but for some reason i cant figure out why when the floatswitch becomes HIGH it activated the alarm as if FloatSwitchMillis; was never updated from the floatswitch state change.

10

in the code i added "timerParams.FloatSwitchCounter++;" to debug a little and it increments +1 for every state change so it should be updating FloatSwitchMillis = millis();? also to compile the code you may have to replace you ir remote library with the one i provided

11

You are playing with FloatSwitchMillis in many places and if the issue is with FloatSwitchAlarmStatus you should check where you set it and print debug info there. You would see if this is happening in the if statement you pointed.

On what type of Arduino are you? on UNO or MEGA

const int PhotoSensor_Delay = 60000;  //delay schedule change by 1 minute when light status changes//wait this long before updating state of day night mode

will not do what you want.
try this code for example

const int PhotoSensor_Delay = 60000;  
void setup() {
  Serial.begin(115200);
  Serial.print(PhotoSensor_Delay);
}

void loop() {}

You'll see the console displaying [color=purple]-5536[/color]

in your code I would recommend you force type on your constant just to avoid weird behaviors in computation --> using 60000[color=red]ul[/color] for example instead of just 60000. I've seen you use casting to uint32_t so should be on the safe side but better be on the safe side esp. in #define (you do it right in some but not to the full extent of casting the parameter )

I would make all the delays unsigned int or unsigned long to avoid comparison between signed and unsigned integer

const unsigned int shortPress = 2000; //enable temporary lcd
const unsigned int  ShowCounterPress = 3700; //SHOW Timer COUTNERS
const unsigned int  EnableTimerPress = 5400; //ENABLE TIMES SKIPSTARTUPDELAY=9
const unsigned int  CalibratePh1Press = 7100; //fast ph1 refresh rate
const unsigned int BlackoutOn = 8800; //turn off screen and leds
const unsigned int BlackoutOff = 10500; // turn on screen and leds
const unsigned int DisableTimers = 12200; //DISABLE TIMER BUTTON
const unsigned int DisableWarnIndicator = 13900; //TURN OFF PH INDICATOR BUTTON
const unsigned int Timer1Press = 15600; //Start Timer 1
const unsigned int Timer2Press = 17300; //Start Timer 2
const unsigned int Timer3Press = 19000; //Start Timer 3
const unsigned int CancelMenuPress = 20700; //Cancel Menu Selection
const unsigned int PhotoSensor_Delay = 60000;  //delay schedule change by 1 minute when light status changes//wait this long before updating state of day night mode


unsigned int TimerDisabledTooLongAlertInterval = 10000; //every 10 seconds sound the alarm if timer disabled too long
unsigned int AlarmBuzzerInterval = 3000; //how long to wait for resound float switch buzzer
unsigned int FloatSwitchResetDelay = 5000; //reset float switch counter delay
unsigned int AutoControlAlertDelay = 10000; //delay between alerts during autocontrol
unsigned int WaterWarnDelay = 10000; //after this duration process waterwarning indication
unsigned int watertempdelay = 1000;
unsigned int Ph1WarnDelay = 15000; //after this duration process ph low/high warning indicator
unsigned int Ph2WarnDelay = 15000; //after this duration process ph low/high warning indicator
unsigned int Ph3WarnDelay = 25000; //after this duration process ph low/high warning indicator
unsigned int calibratescreendelay = 500;
unsigned int tempdelay = 1500; //when to read the temp and ph sensor
unsigned int startcountdelay = 60; //STARTUPDELAY COUNTDOWN SCREEN REFRESH RATE

I would also fix all those magic numbers

 if ((results->value == 16738455) && (SkipStartupDelay == 0)) {

and define them with a name (and use 16738455ul)

when you do this

   ph1reading = pHvalue1 * 14.0 / 1024, 1;

you are actually setting ph1reading to 1... what's the ,1 thingy ?? did you copy that from a print statement (that was there probably to say I want to get only 1 decimal point)

Same issue with ph2reading, ph3reading or CalibratedPh1Value, CalibratedPh2Value, CalibratedPh3Value

You have plenty of lcd.init() - usually this is done once in the setup() (and double check if you can get a more recent LiquidCrystal_I2C which would have a lcd.begin() versus init. (seems old)

what this for loop achieving?

   for (auto& t : timer) {
      timer[0].stop();
      timer[1].stop();
      timer[0].stop();
    }

you have many of those.

in recvWithStartEndMarkers

 while ((Serial.available() > 0) && (!newData)) {
    long rc = Serial.read();

--> read returns an int, not a long (no big deal... but...) and I would make

 char startMarker = '<';
  char endMarker = '>';

those markers const

12

J-M-L:
You are playing with FloatSwitchMillis in many places and if the issue is with FloatSwitchAlarmStatus you should check where you set it and print debug info there. You would see if this is happening in the if statement you pointed.

good find i didnt even notice "const int PhotoSensor_Delay = 60000;" did you see anything wrong with this part?

if ((digitalRead(FloatSwitch) == HIGH) && TimersEnabled) { // waterlevel low
    if (millis() - FloatSwitchMillis >= timerParams.FloatSwitchHIGHduration) {
      if (AlarmEnabled) {
        timerParams.alarmhighcounter++;
        FloatSwitchAlarmStatus = true;
        AlarmBuzzer = true;
        FloatSwitchMillis = millis();
      }
    }
  }
  if ((digitalRead(FloatSwitch) == LOW) && TimersEnabled) { //water level high
    if (millis() - FloatSwitchLowMillis >= timerParams.FloatSwitchLOWduration) {
      if (AlarmEnabled) {
        timerParams.alarmlowcounter++;
        FloatSwitchAlarmStatus = true;
        AlarmBuzzer = true;
        FloatSwitchLowMillis = millis();
      }
    }
  }
  FloatSwitchState = digitalRead(FloatSwitch);
  if (FloatSwitchState != lastFloatSwitchState) {
    if (FloatSwitchState) {
     FloatSwitchStateCounter++;
    }
    timerParams.FloatSwitchCounter++;
    FloatSwitchLowMillis = millis(); //to reset float switch every time water level changes
    FloatSwitchMillis = millis();  //to reset float switch every time water level changes
    FloatSwitchResetMillis = millis();
  }
  lastFloatSwitchState = FloatSwitchState;

the problem is this statement keeps activating even if the floatswitch state changes and update FloatSwitchMillis before the 40 minute timer expires

  if ((digitalRead(FloatSwitch) == HIGH) && TimersEnabled) { // waterlevel low
    if (millis() - FloatSwitchMillis >= timerParams.FloatSwitchHIGHduration) {
      if (AlarmEnabled) {
        timerParams.alarmhighcounter++;
        FloatSwitchAlarmStatus = true;
        AlarmBuzzer = true;
        FloatSwitchMillis = millis();
      }
    }
  }

and when i call those loops it skips to the next timerpin / timer schedule. i had someone help me with that part so im not exactly sure how or why those loops work. also i know this is more a question for google but does usint ints instead of UL use less memory on the arduino?

13

im sorry, also im using a arduino mega. also u use the lcd.init() because that "screen" relay turns off the power to the screen. at the time i couldnt find another way. i play to redesign the whole project it works pretty good except for the alarm problem and what other bugs i havnt noticed yet but its been doing what ive wanted it to do for over a year now. the whole project is running on an isolation transformer and signal isolators. i will eventually use a different screen and use a logic level mosfet to control the backlight

14

the problem is this statement keeps activating even if the floatswitch state changes and update FloatSwitchMillis before the 40 minute timer expires

--> how do you know it keeps activating there... I don't see any print statement that would prove this is what happens...

on a MEGA an int or unsigned int uses 2 bytes, a long would use 4. but if you want to store a large value then 2 bytes is pretty limited...

15

did you see anything wrong with this part?

Yes.

 if ((digitalRead(FloatSwitch) == HIGH) && TimersEnabled) { // waterlevel low

For the life of me, I can not see ANY relationship between the state of a pin that a float switch is connected to and TimersEnabled.

Using compound if statements makes for shorter code, but shorter code can be harder-to-understand code, when the parts of a compound if statement don't appear to be related.

Using nested if statements in that case makes the code easier to understand.

and when i call those loops it skips to the next timerpin / timer schedule.

You call functions, not statements. So, everything in that sentence, after the word loops, makes no sense.

16

J-M-L:
--> how do you know it keeps activating there... I don't see any print statement that would prove this is what happens...

on a MEGA an int or unsigned int uses 2 bytes, a long would use 4. but if you want to store a large value then 2 bytes is pretty limited...

i know it keeps activating there because i have a program written in python that communicates through a tcp serial bridge and i can send it commands and read that status / variables. if i type into the serial it will print timerParams.alarmhighcounter and everytime the problem happens that counter has been counting up and only that counter. if i type it will set FloatSwitchAlarmStatus to false and then happen again 40 minutes later when that floatswitch becomes high

17

PaulS:
Yes.

 if ((digitalRead(FloatSwitch) == HIGH) && TimersEnabled) { // waterlevel low

For the life of me, I can not see ANY relationship between the state of a pin that a float switch is connected to and TimersEnabled.

Using compound if statements makes for shorter code, but shorter code can be harder-to-understand code, when the parts of a compound if statement don't appear to be related.

Using nested if statements in that case makes the code easier to understand.
You call functions, not statements. So, everything in that sentence, after the word loops, makes no sense.

check out the 9th comment it contains the code and modified ir remote library

18

notsolowki:
i know it keeps activating there because i have a program written in python that communicates through a tcp serial bridge and i can send it commands and read that status / variables. if i type into the serial it will print timerParams.alarmhighcounter and everytime the problem happens that counter has been counting up and only that counter. if i type it will set FloatSwitchAlarmStatus to false and then happen again 40 minutes later when that floatswitch becomes high

you can see what the value is but you don't know if it comes from that statement or a memory overflow somewhere else... printing from that very same if would bring certainty... (and I'm 99,999% sure it does not enter the if when the conditions are false)

19

J-M-L:
you can see what the value is but you don't know if it comes from that statement or a memory overflow somewhere else... printing from that very same if would bring certainty... (and I'm 99,999% sure it does not enter the if when the conditions are false)

im sorry im not sure what you mean about the false statement, i can add a print i suppose i dont understand how all of the code in the if statement including the counter is working but add a print to verify?

20

here is a printout of after having 1 false alarm

Alarm Monitor State '0' 

Alarm Low Counter '0' 

Alarm High Counter '1' 

Float Switch Total Counter '9' 

TimerDisabledTooLong '0' 

BuzzerEnabled '1' 

AlarmBuzzer '0' 

Alarm State '0' 

 Timer Enabled State '1' 

 Timer Current State '1' 

 Timer 1 State '1' 

 Timer 2 State '0' 

 Timer 3 State '0' 

 Last Timer 1 State '1' 

 Last Timer 2 State '0' 

 Last Timer 3 State '0' 

 Dht Sensor State '0' 

 Ph1 Warning Current State '0' 

 Ph2 Warning Current State '0' 

 Ph3 Warning Current State '0' 

 Ph Warning Enabled State '1' 

 Ph2 Warning Enabled State '1' 

 Ph3 Warning Enabled State '0' 

 Water Temp Warning Current State '0' 

 Water Temp Warning State '1' 

 Startup Delay State '2' 

 Screen Mode State '1' 

 Menu Mode State '0' 

 EEPROM Write Count '57' 

 TimerProcess '1' 

 Ph1 Timer Control '1' 

 Ph2 Timer Control '1' 

 Ph3 Timer Control '0' 

 Timer 1 Enabled '1' 

 Timer 2 Enabled '1' 

 Timer 3 Enabled '1' 

Ph1 Auto Control Mode '0

Ph2 Auto Control Mode '0

Ph3 Auto Control Mode '0

Ph1 Alert Low '5.72' 

Ph1 Alert High '6.10' 

Ph1 No Alert '5.72' 

Ph2 Alert Low '5.72' 

Ph2 Alert High '6.50' 

Ph2 No Alert '5.72' 

Ph3 Alert Low '5.72' 

Ph3 Alert High '6.10' 

Ph3 No Alert '5.72' 

Ph1 Timers Overridden '0' 

Ph2 Timers Overridden '0' 

Ph3 Timers Overridden '0' 

Orp Cal Factor Set To 0' 

$

Alarm Monitor State is 0 because i turned it off but maybe that output can help debug

the data in the EEPROM is valid. this is a printout of i may have changed some compared to what show in the sketch

PhAutoControl Wait To Activate 5000

orpMinMaxWait 25000

MinMaxUpdateDelay 1000

LcdPause 1000

Timer-Disabled-Repeat-AlertInterval 10000

AlarmBuzzerInterval 3000

FloatSwitchResetDelay 5000

AutoControlAlertDelay 10000

WaterWarnDelay 10000

Ph1WarnDelay 15000

Ph1WarnDelay 15000

AlarmREenable 180

FloatTempDelayDuration 10000

tempscreenOn 30000

calibrateontime 45000

t.FloatSwitchHIGHduration 40

t.FloatSwitchLOWduration 40

t.TimerDisabledlertInterval 90