SOLVED: Pump and light timer for Aeroponic system crashes randomly

The microcontroller of my aeroponic system died and I have created a replacement using Arduino Uno, a 2 relay relay board, 7 LEDs and three buttons. The system works fine for a few hours and then hangs. I have rewritten the code three times, added capacitors to all inputs to no avail. As this is my first Arduino project and also the first encounter I have with C++ I suspect the problem is with my code rather than the harware. To make the system work for more than a few hours I added a watchdog to the code which keeps the system running but forgets the user choices every few hours.

The user interface consists of three buttons
#1 for selecting mode: Vegetables, Herbs, Flowers, Others
#2 for selecting phase: Seeding, Growing, Ripening
#3 for toggling growing light on and off.
In addition there are 7 leds that indicate mode and phase.

This is the Arduino Uno board I am using

Link to the relay board I am using.

Any idea to the reason why the system hangs after a few hours would be greatly appreciated.

Timo

// Arduino replacement for Aerponics controller board
// Version 1.2
// Author timo121357 25.7.2019

// Watchdog - Version: Latest 
#include <Watchdog.h>

// Buttons of Aeroponics unit and corresponding Arduino IO pins
const byte leftButton = A0;
const byte middleButton = A1;
const byte lightButton = A2;

// LEDs of Aerponics unit and corresponding Arduino IO pins
const byte others = 3;
const byte flowers = 4;
const byte herbs = 5;
const byte vegetables = 2;
const byte ripening = 7;
const byte growing = 8;
const byte seeding = 6;

// Relay board and corresponding Arduino IO pins
const byte light = 9; 
const byte pump = 10;

// States
byte mode = 0; // 0=Vegetables, 1=Herbs, 2=Flowers, 3=Other
byte phase = 2; // 0,1=Seeding, 2,3=Growing, 4,5=Ripening
unsigned long pumpChanged=0; // time when pump state last was changed
unsigned long lightChanged=0; // time when light state last was changed

// light timers in hours: On, Off: Seeding, Growing, Ripening
unsigned long lightTimer[][6] = { 
{4,20,20,4,20,4}, // mode 0: Vegetables
{4,20,20,4,20,4}, // mode 1: Herbs
{4,20,20,4,20,4}, // mode 2: Flowers
{4,20,20,4,20,4}  // mode 3: Others
};

// pump timers in seconds: On, Off: Seeding, Growing, Ripening
unsigned long pumpTimer[][6] = {
{30,30,5,240,5,480},  // mode 0: Vegetables
{30,30,5,115,3,177},  // mode 1: Herbs
{30,30,5,60,3,300},   // mode 2: Flowers
{60,120,60,180,60,600}// mode 3: Others
};

Watchdog watchdog; // Watchdog declaration

void setup()
{
  Serial.begin(9600); // initialize serial communications at 9600 bps
  pinMode(leftButton, INPUT_PULLUP);
  pinMode(middleButton, INPUT_PULLUP);
  pinMode(lightButton, INPUT_PULLUP);
  pinMode(vegetables, OUTPUT);
  pinMode(herbs, OUTPUT);
  pinMode(flowers, OUTPUT);
  pinMode(others, OUTPUT);
  pinMode(seeding, OUTPUT);
  pinMode(growing, OUTPUT);
  pinMode(ripening, OUTPUT);
  pinMode(pump, OUTPUT);
  pinMode(light, OUTPUT);
  digitalWrite(light, HIGH);
  digitalWrite(pump, HIGH);
  watchdog.enable(Watchdog::TIMEOUT_2S);
}

void loop()
{
  watchdog.reset();// Watchdog gets a bone
  readButtons(); // read input from buttons
    
  // Set indicator LEDs:
  // Mode LED
  if (mode == 0) digitalWrite(vegetables,HIGH); else digitalWrite(vegetables,LOW);
  if (mode == 1) digitalWrite(herbs,HIGH); else digitalWrite(herbs,LOW);
  if (mode == 2) digitalWrite(flowers,HIGH); else digitalWrite(flowers,LOW);
  if (mode == 3) digitalWrite(others,HIGH); else digitalWrite(others,LOW);
  // Phase LED
  if (phase == 0 || phase == 1) digitalWrite(seeding,HIGH); else digitalWrite(seeding,LOW);
  if (phase == 2 || phase == 3) digitalWrite(growing,HIGH); else digitalWrite(growing,LOW);
  if (phase == 4 || phase == 5) digitalWrite(ripening,HIGH); else digitalWrite(ripening,LOW);

  // store current time before timer controls start
  unsigned long currentMillis = millis(); 
  
  // control the light time
  if (digitalRead(light) == HIGH){
    if ((unsigned long)(currentMillis - lightChanged >= 3600000 * lightTimer[mode][phase])){ 
      digitalWrite(light,LOW); //turn light off after timer has expired
      lightChanged = currentMillis;
    }
  }
  else if ((unsigned long)(currentMillis - lightChanged >= 3600000 * lightTimer[mode][phase+1])){  
    digitalWrite(light,HIGH); //turn light back on after timer has expired
    lightChanged = currentMillis;
  }
  
  // control pump time
  if (digitalRead(pump) == HIGH){
    if ((unsigned long)(currentMillis - pumpChanged >= 1000 * pumpTimer[mode][phase])){
      digitalWrite(pump,LOW); //turn pump off after timer has expired
      pumpChanged = currentMillis;
    }
  }
  else if ((unsigned long)(currentMillis - pumpChanged >= 1000*pumpTimer[mode][phase+1])){  
    digitalWrite(pump,HIGH); //turn pump back on after timer has expired
    pumpChanged = currentMillis;
  }
}

void readButtons() // read input from buttons
{
  delay(200); // to avoid multiple presses in one occasion
  if (digitalRead(leftButton) == LOW){
    mode += 1;
    if (mode > 3)  mode = 0;
  }
  else if (digitalRead(middleButton) == LOW){
    phase += 2; // phase has on and off times for each mode, hence increments in 2
    if (phase > 5) phase = 0;
  }
  else if (digitalRead(lightButton) == LOW){
    digitalWrite(light, !digitalRead(light)); //toggle light
  }
}
else if (digitalRead(middleButton) == LOW){
    phase += 2; // phase has on and off times for each mode, hence increments in 2
    if (phase > 5) phase = 0;

So phase can be as much as 5, which would make this:

else if ((unsigned long)(currentMillis - pumpChanged >= 1000*pumpTimer[mode][phase+1])){

go out of bounds.

Don't know if that's your issue, but it is the first thing that jumped out at me.

Also note that you've got the parenthesis misplaced here:

if ((unsigned long)(currentMillis - pumpChanged >= 1000 * pumpTimer[mode][phase])){

You're casting the boolean result of the comparison to unsigned long. Surely that isn't necessary. If there are pieces of that calculation that need to be cast, then this isn't doing it. You're casing the result of the entire expression which would normally either just be true or false.

Thank you for the quick reply, very helpful!
I made the changes based upon your observations as follows:

  // control the light time
  if (digitalRead(light) == HIGH){
    if ((unsigned long)(currentMillis - lightChanged) >= (unsigned long)(3600000 * lightTimer[mode][phase])){ 
      digitalWrite(light,LOW); //turn light off after timer has expired
      lightChanged = currentMillis;
    }
  }

and

  else if (digitalRead(middleButton) == LOW){
    phase += 2; // phase has on and off times for each mode, hence increments in 2
    if (phase > 4) phase = 0;
  }

I'll leave the system with this version over night and report results tomorrow.

Thanks for the help!

Unfortunately the system still hangs after these changes. A watchdog reset had happened last night.

What could I try next?

I'm not exactly sure how the compiler treats this literal;

    if ((unsigned long)(currentMillis - pumpChanged >= 1000 * pumpTimer[mode][phase])){

You might try changing the 1000 to 1000UL and add the UL to all those timing literals.

and what you have below isn't wrong but why not use a switch-case statement?

  if (mode == 0) digitalWrite(vegetables,HIGH); else digitalWrite(vegetables,LOW);
  if (mode == 1) digitalWrite(herbs,HIGH); else digitalWrite(herbs,LOW);
  if (mode == 2) digitalWrite(flowers,HIGH); else digitalWrite(flowers,LOW);
  if (mode == 3) digitalWrite(others,HIGH); else digitalWrite(others,LOW);

And lastly, readButtons() delays for 1/5th of a second as a cheap debounce and force the user to tap and release fast.
You could reduce the time if you keep track of the last read state for each button and only act when the button is LOW and the last state is HIGH, denoting a state change action rather than state driven action.

void readButtons() // read input from buttons
{
  delay(200); // to avoid multiple presses in one occasion
  if (digitalRead(leftButton) == LOW){
    mode += 1;
    if (mode > 3)  mode = 0;
  }
  else if (digitalRead(middleButton) == LOW){
    phase += 2; // phase has on and off times for each mode, hence increments in 2
    if (phase > 5) phase = 0;
  }
  else if (digitalRead(lightButton) == LOW){
    digitalWrite(light, !digitalRead(light)); //toggle light
  }
}

I would keep phase to being 0 to 2 and when accessing the array and when reading the phase button,

    if ( ++phase > 2 )   phase = 0;  // this uses the pre-increment ++ that increments the variable and THEN evaluates the expression
  // control the light time
  if (digitalRead(light) == HIGH){
    if ((unsigned long)(currentMillis - lightChanged >= 3600000 * lightTimer[mode][ phase * 2 ])){ 
      digitalWrite(light,LOW); //turn light off after timer has expired
      lightChanged = currentMillis;
    }
  }
  else if ((unsigned long)(currentMillis - lightChanged >= 3600000 * lightTimer[mode][ phase * 2 + 1 ])){  
    digitalWrite(light,HIGH); //turn light back on after timer has expired
    lightChanged = currentMillis;
  }
  
  // control pump time
  if (digitalRead(pump) == HIGH){
    if ((unsigned long)(currentMillis - pumpChanged >= 1000 * pumpTimer[mode][ phase * 2 ])){
      digitalWrite(pump,LOW); //turn pump off after timer has expired
      pumpChanged = currentMillis;
    }
  }
  else if ((unsigned long)(currentMillis - pumpChanged >= 1000*pumpTimer[mode][ phase * 2 + 1 ])){  
    digitalWrite(pump,HIGH); //turn pump back on after timer has expired
    pumpChanged = currentMillis;
  }
}

GoForSmoke:
I'm not exactly sure how the compiler treats this literal;

    if ((unsigned long)(currentMillis - pumpChanged >= 1000 * pumpTimer[mode][phase])){

Basically the literals will be handled as (signed) int by the compiler. on a ATMEGA that would be 16 bits, on a MKR or ESP would be 32 bits.

Adding UL is a good practice to show intent, but does not help much even on ATMEGA as pumpTimer or lightTimer (in 3600000 * lightTimer) are unsigned long, so in the simple math there the literal is promoted to unsigned long automatically.

Thanks a million for the support, here is what I have changed and not in the code.

if ((unsigned long)(currentMillis - pumpChanged >= 1000 * pumpTimer[mode][phase])){

is now

if ((unsigned long)(currentMillis - pumpChanged) >= (unsigned long)(1000 * pumpTimer[mode][phase*2])){

I changed phase to go from 0-2 and removed the delay in the beginning thanks to the advice i got. Now the button presses are recognized only once every time and there is no hurry in pressing the button.

void readButtons() // read input from buttons
{
  //left button
  leftButtonState=digitalRead(leftButton);
  if (leftButtonState !=lastLeftButtonState) {
    if (leftButtonState == LOW){
      if (++mode > 3)  mode = 0; // update mode and roll over when 3 is exceeded
    }  
  }
  lastLeftButtonState=leftButtonState;
  
  // middle button
  middleButtonState=digitalRead(middleButton);
  if (middleButtonState != lastMiddleButtonState){
    if (middleButtonState == LOW) {
      if (++phase > 2) phase = 0; // updaste phase and roll over when 2 is exceeded
    }
  }
  lastMiddleButtonState=middleButtonState;
  
  //ligth button
  lightButtonState=digitalRead(lightButton);
  if (lightButtonState != lastLightButtonState) {
    if (lightButtonState == LOW) {
      digitalWrite(light, !digitalRead(light)); //toggle light  
    }  
  }
  lastLightButtonState=lightButtonState;
}

i did not change the LED control section to Switch-Case, as to me that would have required me to define the state of each LED in each 7 cases. My solution seems simpler and easier, no?

The new code is now running in the system. I will report success or failure here. For reference I post the whole code version 1.3 here below.

// Arduino replacement for Aerponics controller board
// Version 1.3.0
// Author timo121357 28.7.2019 with help from Arduino Forum

// Watchdog - Version: Latest 
#include <Watchdog.h>

// Buttons of Aeroponics unit and corresponding Arduino IO pins
const byte leftButton = A0;
const byte middleButton = A1;
const byte lightButton = A2;

// LEDs of Aerponics unit and corresponding Arduino IO pins
const byte others = 3;
const byte flowers = 4;
const byte herbs = 5;
const byte vegetables = 2;
const byte ripening = 7;
const byte growing = 8;
const byte seeding = 6;

// Relay board and corresponding Arduino IO pins
const byte light = 9; 
const byte pump = 10;

// States
byte mode = 0; // 0=Vegetables, 1=Herbs, 2=Flowers, 3=Other
byte phase = 0; // 0=Seeding, 1=Growing, 2=Ripening
byte lastLeftButtonState = HIGH;
byte leftButtonState = HIGH;
byte lastMiddleButtonState = HIGH;
byte middleButtonState = HIGH;
byte lastLightButtonState=HIGH;
byte lightButtonState=HIGH;
unsigned long pumpChanged=0; // time when pump state last was changed
unsigned long lightChanged=0; // time when light state last was changed

// light timers in hours: On, Off: Seeding, Growing, Ripening
unsigned long lightTimer[][6] = { 
{4,20,20,4,20,4}, // mode 0: Vegetables
{4,20,20,4,20,4}, // mode 1: Herbs
{4,20,20,4,20,4}, // mode 2: Flowers
{4,20,20,4,20,4}  // mode 3: Others
};

// pump timers in seconds: On, Off: Seeding, Growing, Ripening
unsigned long pumpTimer[][6] = {
{30,30,5,240,5,480},  // mode 0: Vegetables
{30,30,5,115,3,177},  // mode 1: Herbs
{30,30,5,60,3,300},   // mode 2: Flowers
{60,120,60,180,60,600}// mode 3: Others
};

Watchdog watchdog; // Watchdog declaration

void setup()
{
  Serial.begin(9600); // initialize serial communications at 9600 bps
  pinMode(leftButton, INPUT_PULLUP);
  pinMode(middleButton, INPUT_PULLUP);
  pinMode(lightButton, INPUT_PULLUP);
  pinMode(vegetables, OUTPUT);
  pinMode(herbs, OUTPUT);
  pinMode(flowers, OUTPUT);
  pinMode(others, OUTPUT);
  pinMode(seeding, OUTPUT);
  pinMode(growing, OUTPUT);
  pinMode(ripening, OUTPUT);
  pinMode(pump, OUTPUT);
  pinMode(light, OUTPUT);
  digitalWrite(light, HIGH);
  digitalWrite(pump, HIGH);
  watchdog.enable(Watchdog::TIMEOUT_2S);
}

void loop()
{
  watchdog.reset();// Watchdog gets a bone
  readButtons(); // read input from buttons
    
  // Set indicator LEDs:
  // Mode LED
  if (mode == 0) digitalWrite(vegetables,HIGH); else digitalWrite(vegetables,LOW);
  if (mode == 1) digitalWrite(herbs,HIGH); else digitalWrite(herbs,LOW);
  if (mode == 2) digitalWrite(flowers,HIGH); else digitalWrite(flowers,LOW);
  if (mode == 3) digitalWrite(others,HIGH); else digitalWrite(others,LOW);
  
  // Phase LED
  if (phase == 0) digitalWrite(seeding,HIGH); else digitalWrite(seeding,LOW);
  if (phase == 1) digitalWrite(growing,HIGH); else digitalWrite(growing,LOW);
  if (phase == 2) digitalWrite(ripening,HIGH); else digitalWrite(ripening,LOW);

  // store current time before timer controls start
  unsigned long currentMillis = millis(); 

  // control light time
  if (digitalRead(light) == HIGH){
    if ((unsigned long)(currentMillis - lightChanged) >= (unsigned long)(3600000 * lightTimer[mode][phase*2])){ 
      digitalWrite(light,LOW); //turn light off after timer has expired
      lightChanged = currentMillis;
    }
  }
  else if ((unsigned long)(currentMillis - lightChanged) >= (unsigned long)(3600000 * lightTimer[mode][phase*2+1])){  
    digitalWrite(light,HIGH); //turn light back on after timer has expired
    lightChanged = currentMillis;
  }
  
  // control pump time
  if (digitalRead(pump) == HIGH){
    if ((unsigned long)(currentMillis - pumpChanged) >= (unsigned long)(1000 * pumpTimer[mode][phase*2])){
      digitalWrite(pump,LOW); //turn pump off after timer has expired
      pumpChanged = currentMillis;
    }
  }
  else if ((unsigned long)(currentMillis - pumpChanged) >= (unsigned long)(1000*pumpTimer[mode][phase*2+1])){  
    digitalWrite(pump,HIGH); //turn pump back on after timer has expired
    pumpChanged = currentMillis;
  }
}

void readButtons() // read input from buttons
{
  //left button
  leftButtonState=digitalRead(leftButton);
  if (leftButtonState !=lastLeftButtonState) {
    if (leftButtonState == LOW){
      if (++mode > 3)  mode = 0; // update mode and roll over when 3 is exceeded
    }  
  }
  lastLeftButtonState=leftButtonState;
  
  // middle button
  middleButtonState=digitalRead(middleButton);
  if (middleButtonState != lastMiddleButtonState){
    if (middleButtonState == LOW) {
      if (++phase > 2) phase = 0; // updaste phase and roll over when 2 is exceeded
    }
  }
  lastMiddleButtonState=middleButtonState;
  
  //ligth button
  lightButtonState=digitalRead(lightButton);
  if (lightButtonState != lastLightButtonState) {
    if (lightButtonState == LOW) {
      digitalWrite(light, !digitalRead(light)); //toggle light  
    }  
  }
  lastLightButtonState=lightButtonState;
}

J-M-L:
pumpTimer or lightTimer (in 3600000 * lightTimer) are unsigned long, so in the simple math there the literal is promoted to unsigned long automatically.

IIRC int gets promoted to long, +/- 2 billion and some is > 3.6 million. Using a long with unsigned longs in timers limits the longest interval of a bit over 24 days with millis() timing, and then it crashes where this code crashes sooner.

timo121357:

if ((unsigned long)(currentMillis - pumpChanged >= 1000 * pumpTimer[mode][phase])){

is now

if ((unsigned long)(currentMillis - pumpChanged) >= (unsigned long)(1000 * pumpTimer[mode][phase*2])){

Good news on the rest but (unsigned long)(1000 * pumpTimer[mode][phase*2]) is (1000 * pumpTimer[mode][phase*2]), int-UL mixed math results converted to unsigned long as opposed to literal 1000UL.

Literal 1000 is type int, the expression will result in an int, -32768 to 32767.

Think I found a problem with every interval > 30, them's multipliers! But hey, the RESULTS get converted to unsigned long.

unsigned long pumpTimer[][6] = {
{30,30,5,240,5,480},  // mode 0: Vegetables
{30,30,5,115,3,177},  // mode 1: Herbs
{30,30,5,60,3,300},   // mode 2: Flowers
{60,120,60,180,60,600}// mode 3: Others

GoForSmoke:
IIRC int gets promoted to long, +/- 2 billion and some is > 3.6 million. Using a long with unsigned longs in timers limits the longest interval of a bit over 24 days with millis() timing, and then it crashes where this code crashes sooner.

Actually the rules states that the literal will be handled as an int in the formula and then promoted to what makes sense for the operation. When you perform an operation between a signed and an unsigned integer then the whole thing is done as unsigned, with the signed literal first being interpreted as unsigned.

That leads to surprising results if the int was negative... for example, executing this simple code

uint16_t v = 12;

void setup()
{
  Serial.begin(115200);    
  Serial.println(v / -2);
}

void loop() {}

leads to seeing 0 in the Serial Console instead of the expected -6 and this is perfectly correct due to the unsigned promotion rule.

I added UL in front of the literals in the timer code. This will run over night. I will report success or failure tomorrow.

 // control light time
  if (digitalRead(light) == HIGH){
    if ((unsigned long)(currentMillis - lightChanged) >= 3600000UL * lightTimer[mode][phase*2]){ 
      digitalWrite(light,LOW); //turn light off after timer has expired
      lightChanged = currentMillis;
    }
  }
  else if ((unsigned long)(currentMillis - lightChanged) >= 3600000UL * lightTimer[mode][phase*2+1]){  
    digitalWrite(light,HIGH); //turn light back on after timer has expired
    lightChanged = currentMillis;
  }
  
  // control pump time
  if (digitalRead(pump) == HIGH){
    if ((unsigned long)(currentMillis - pumpChanged) >= 1000UL * pumpTimer[mode][phase*2]){
      digitalWrite(pump,LOW); //turn pump off after timer has expired
      pumpChanged = currentMillis;
    }
  }
  else if ((unsigned long)(currentMillis - pumpChanged) >= 1000UL * pumpTimer[mode][phase*2+1]){  
    digitalWrite(pump,HIGH); //turn pump back on after timer has expired
    pumpChanged = currentMillis;
  }

Some positive news: the system had not crashed during the night.
However, the light timer behaves now strangely, the light gets turned on and off not according to the schedule. Pump seems to work as should.

Could the UL added in front of the literals cause the timer to lose track of time?

The UL just means unsigned long.

Can you repost the full version as you used it?

Sure! Here is the version that is currently running:

// Arduino replacement for Aerponics controller board
// Version 1.3.1
// Author timo121357 28.7.2019 with help from Arduino Forum

// Watchdog - Version: Latest 
// #include <Watchdog.h>

// Buttons of Aeroponics unit and corresponding Arduino IO pins
const byte leftButton = A0;
const byte middleButton = A1;
const byte lightButton = A2;

// LEDs of Aerponics unit and corresponding Arduino IO pins
const byte others = 3;
const byte flowers = 4;
const byte herbs = 5;
const byte vegetables = 2;
const byte ripening = 7;
const byte growing = 8;
const byte seeding = 6;

// Relay board and corresponding Arduino IO pins
const byte light = 9; 
const byte pump = 10;

// States
byte mode = 0; // 0=Vegetables, 1=Herbs, 2=Flowers, 3=Other
byte phase = 0; // 0=Seeding, 1=Growing, 2=Ripening
byte lastLeftButtonState = HIGH;
byte leftButtonState = HIGH;
byte lastMiddleButtonState = HIGH;
byte middleButtonState = HIGH;
byte lastLightButtonState=HIGH;
byte lightButtonState=HIGH;
unsigned long pumpChanged=0; // time when pump state last was changed
unsigned long lightChanged=0; // time when light state last was changed

// light timers in hours: On, Off: Seeding, Growing, Ripening
unsigned long lightTimer[][6] = { 
{4,20,20,4,20,4}, // mode 0: Vegetables
{4,20,20,4,20,4}, // mode 1: Herbs
{4,20,20,4,20,4}, // mode 2: Flowers
{4,20,20,4,20,4}  // mode 3: Others
};

// pump timers in seconds: On, Off: Seeding, Growing, Ripening
unsigned long pumpTimer[][6] = {
{30,30,5,240,5,480},  // mode 0: Vegetables
{30,30,5,115,3,177},  // mode 1: Herbs
{30,30,5,60,3,300},   // mode 2: Flowers
{60,120,60,180,60,600}// mode 3: Others
};

//Watchdog watchdog; // Watchdog declaration

void setup()
{
  Serial.begin(9600); // initialize serial communications at 9600 bps
  pinMode(leftButton, INPUT_PULLUP);
  pinMode(middleButton, INPUT_PULLUP);
  pinMode(lightButton, INPUT_PULLUP);
  pinMode(vegetables, OUTPUT);
  pinMode(herbs, OUTPUT);
  pinMode(flowers, OUTPUT);
  pinMode(others, OUTPUT);
  pinMode(seeding, OUTPUT);
  pinMode(growing, OUTPUT);
  pinMode(ripening, OUTPUT);
  pinMode(pump, OUTPUT);
  pinMode(light, OUTPUT);
  digitalWrite(light, HIGH);
  digitalWrite(pump, HIGH);
//  watchdog.enable(Watchdog::TIMEOUT_2S);
}

void loop()
{
//  watchdog.reset();// Watchdog gets a bone
  readButtons(); // read input from buttons
    
  // Set indicator LEDs:
  // Mode LED
  if (mode == 0) digitalWrite(vegetables,HIGH); else digitalWrite(vegetables,LOW);
  if (mode == 1) digitalWrite(herbs,HIGH); else digitalWrite(herbs,LOW);
  if (mode == 2) digitalWrite(flowers,HIGH); else digitalWrite(flowers,LOW);
  if (mode == 3) digitalWrite(others,HIGH); else digitalWrite(others,LOW);
  
  // Phase LED
  if (phase == 0) digitalWrite(seeding,HIGH); else digitalWrite(seeding,LOW);
  if (phase == 1) digitalWrite(growing,HIGH); else digitalWrite(growing,LOW);
  if (phase == 2) digitalWrite(ripening,HIGH); else digitalWrite(ripening,LOW);

  // store current time before timer controls start
  unsigned long currentMillis = millis(); 

  // control light time
  if (digitalRead(light) == HIGH){
    if ((unsigned long)(currentMillis - lightChanged) >= 3600000UL * lightTimer[mode][phase*2]){ 
      digitalWrite(light,LOW); //turn light off after timer has expired
      lightChanged = currentMillis;
    }
  }
  else if ((unsigned long)(currentMillis - lightChanged) >= 3600000UL * lightTimer[mode][phase*2+1]){  
    digitalWrite(light,HIGH); //turn light back on after timer has expired
    lightChanged = currentMillis;
  }
  
  // control pump time
  if (digitalRead(pump) == HIGH){
    if ((unsigned long)(currentMillis - pumpChanged) >= 1000UL * pumpTimer[mode][phase*2]){
      digitalWrite(pump,LOW); //turn pump off after timer has expired
      pumpChanged = currentMillis;
    }
  }
  else if ((unsigned long)(currentMillis - pumpChanged) >= 1000UL * pumpTimer[mode][phase*2+1]){  
    digitalWrite(pump,HIGH); //turn pump back on after timer has expired
    pumpChanged = currentMillis;
  }
}

void readButtons() // read input from buttons
{
  //left button
  leftButtonState=digitalRead(leftButton);
  if (leftButtonState !=lastLeftButtonState) {
    if (leftButtonState == LOW){
      if (++mode > 3)  mode = 0; // update mode and roll over when 3 is exceeded
    }  
  }
  lastLeftButtonState=leftButtonState;
  
  // middle button
  middleButtonState=digitalRead(middleButton);
  if (middleButtonState != lastMiddleButtonState){
    if (middleButtonState == LOW) {
      if (++phase > 2) phase = 0; // updaste phase and roll over when 2 is exceeded
    }
  }
  lastMiddleButtonState=middleButtonState;
  
  //ligth button
  lightButtonState=digitalRead(lightButton);
  if (lightButtonState != lastLightButtonState) {
    if (lightButtonState == LOW) {
      digitalWrite(light, !digitalRead(light)); //toggle light  
    }  
  }
  lastLightButtonState=lightButtonState;
}

timo121357:
However, the light timer behaves now strangely, the light gets turned on and off not according to the schedule.

The lightChanged/pumpChanged timers are not reset when you hit the buttons, so if you change the state with the buttons then you could wind up with some odd numerical artifacts. Suppose the cycle is midway though one of those 20 hour periods, and you move the phase such that it passes through one of the 4-hour phases before going back to a 20-hour selection … when will the light change again? How does hittig the light button affect the timing?

--EDIT--

Perhaps you just need:

  //ligth button
  lightButtonState=digitalRead(lightButton);
  if (lightButtonState != lastLightButtonState) {
    if (lightButtonState == LOW) {
      digitalWrite(light, !digitalRead(light)); //toggle light 
      lightChanged = millis(); // RESET THE LIGHT TIMER
    } 
  }
  lastLightButtonState=lightButtonState;

timo121357:
Some positive news: the system had not crashed during the night.
However, the light timer behaves now strangely, the light gets turned on and off not according to the schedule. Pump seems to work as should.

Could the UL added in front of the literals cause the timer to lose track of time?

This would be great time to add debug prints and collect a log of what values are being run.

Sadder news, I am afraid. The system has crashed twice since the last time I wrote to the forum.

I am totally out ideas...

Post your latest sketch and I'll take another look at it.

Are you sure the relay switching is not causing a power glitch that crashes the Arduino? Sometimes software problems turn out to be hardware problems. :slight_smile:

johnwasser:
Post your latest sketch and I’ll take another look at it.

Are you sure the relay switching is not causing a power glitch that crashes the Arduino? Sometimes software problems turn out to be hardware problems. :slight_smile:

ElectroStatic Discharge ESD was my first suspicion, I added capacitors across all inputs and as a result the crashes became a lot more frequent. I probably need to disconnect the relay and see if the crashes disappear as a result.

Below my complete Sketch, thanks for helping!

// Arduino replacement for Aerponics controller board
// Version 1.3.2
// Author timo121357 29.7.2019 with help from Arduino Forum

// Watchdog - Version: Latest 
#include <Watchdog.h>

// Buttons of Aeroponics unit and corresponding Arduino IO pins
const byte leftButton = A0;
const byte middleButton = A1;
const byte lightButton = A2;

// LEDs of Aerponics unit and corresponding Arduino IO pins
const byte others = 3;
const byte flowers = 4;
const byte herbs = 5;
const byte vegetables = 2;
const byte ripening = 7;
const byte growing = 8;
const byte seeding = 6;

// Relay board and corresponding Arduino IO pins
const byte light = 9; 
const byte pump = 10;

// Default states
byte mode = 0; // 0=Vegetables, 1=Herbs, 2=Flowers, 3=Other
byte phase = 0; // 0=Seeding, 1=Growing, 2=Ripening
byte lastLeftButtonState = HIGH;
byte leftButtonState = HIGH;
byte lastMiddleButtonState = HIGH;
byte middleButtonState = HIGH;
byte lastLightButtonState=HIGH;
byte lightButtonState=HIGH;
unsigned long pumpChanged=0; // time when pump state last was changed
unsigned long lightChanged=0; // time when light state last was changed
unsigned long currentMillis=0; // current time

// light timers in hours: On, Off: Seeding, Growing, Ripening
unsigned long lightTimer[][6] = { 
{4,20,20,4,20,4}, // mode 0: Vegetables
{4,20,20,4,20,4}, // mode 1: Herbs
{4,20,20,4,20,4}, // mode 2: Flowers
{4,20,20,4,20,4}  // mode 3: Others
};

// pump timers in seconds: On, Off: Seeding, Growing, Ripening
unsigned long pumpTimer[][6] = {
{30,30,5,240,5,480},  // mode 0: Vegetables
{30,30,5,115,3,177},  // mode 1: Herbs
{30,30,5,60,3,300},   // mode 2: Flowers
{60,120,60,180,60,600}// mode 3: Others
};

void setup()
{
  Serial.begin(9600); // initialize serial communications at 9600 bps
  pinMode(leftButton, INPUT_PULLUP);
  pinMode(middleButton, INPUT_PULLUP);
  pinMode(lightButton, INPUT_PULLUP);
  pinMode(vegetables, OUTPUT);
  pinMode(herbs, OUTPUT);
  pinMode(flowers, OUTPUT);
  pinMode(others, OUTPUT);
  pinMode(seeding, OUTPUT);
  pinMode(growing, OUTPUT);
  pinMode(ripening, OUTPUT);
  pinMode(pump, OUTPUT);
  pinMode(light, OUTPUT);
  digitalWrite(light, HIGH);
  digitalWrite(pump, HIGH);
  Watchdog watchdog; // Watchdog declaration
  watchdog.enable(Watchdog::TIMEOUT_2S);
}

void loop()
{
  watchdog.reset();// Watchdog gets a bone

  // store current time before timer controls start
  currentMillis = millis(); 
  readButtons(); // read input from buttons
    
  // Set indicator LEDs:
  // Mode LED
  if (mode == 0) digitalWrite(vegetables,HIGH); else digitalWrite(vegetables,LOW);
  if (mode == 1) digitalWrite(herbs,HIGH); else digitalWrite(herbs,LOW);
  if (mode == 2) digitalWrite(flowers,HIGH); else digitalWrite(flowers,LOW);
  if (mode == 3) digitalWrite(others,HIGH); else digitalWrite(others,LOW);
  
  // Phase LED
  if (phase == 0) digitalWrite(seeding,HIGH); else digitalWrite(seeding,LOW);
  if (phase == 1) digitalWrite(growing,HIGH); else digitalWrite(growing,LOW);
  if (phase == 2) digitalWrite(ripening,HIGH); else digitalWrite(ripening,LOW);

  // control light time
  if (digitalRead(light) == HIGH){
    if ((unsigned long)(currentMillis - lightChanged) >= 3600000UL * lightTimer[mode][phase*2]){ 
      digitalWrite(light,LOW); //turn light off after timer has expired
      lightChanged = currentMillis;
    }
  }
  else if ((unsigned long)(currentMillis - lightChanged) >= 3600000UL * lightTimer[mode][phase*2+1]){  
    digitalWrite(light,HIGH); //turn light back on after timer has expired
    lightChanged = currentMillis;
  }
  
  // control pump time
  if (digitalRead(pump) == HIGH){
    if ((unsigned long)(currentMillis - pumpChanged) >= 1000UL * pumpTimer[mode][phase*2]){
      digitalWrite(pump,LOW); //turn pump off after timer has expired
      pumpChanged = currentMillis;
    }
  }
  else if ((unsigned long)(currentMillis - pumpChanged) >= 1000UL * pumpTimer[mode][phase*2+1]){  
    digitalWrite(pump,HIGH); //turn pump back on after timer has expired
    pumpChanged = currentMillis;
  }
}

void readButtons() // read input from buttons
{
  delay(2); // needed to stabilize buttonpres
  //left button
  leftButtonState=digitalRead(leftButton);
  if (leftButtonState !=lastLeftButtonState) {
    if (leftButtonState == LOW){
      if (++mode > 3)  mode = 0; // update mode and roll over when 3 is exceeded
    }  
  }
  lastLeftButtonState=leftButtonState;
  
  // middle button
  middleButtonState=digitalRead(middleButton);
  if (middleButtonState != lastMiddleButtonState){
    if (middleButtonState == LOW) {
      if (++phase > 2) phase = 0; // updaste phase and roll over when 2 is exceeded
    }
  }
  lastMiddleButtonState=middleButtonState;
  
  //ligth button
  lightButtonState=digitalRead(lightButton);
  if (lightButtonState != lastLightButtonState) {
    if (lightButtonState == LOW) {
      digitalWrite(light, !digitalRead(light)); //toggle light
      lightChanged = currentMillis;
    }  
  }
  lastLightButtonState=lightButtonState;
}

Can you explain in details how things are wired and powered?