Problem: Forced shut ON/OFF of a relay during it's millis function

Hi everyone,

Hope you are well.

I have a little tricky trouble with automating a relay using millis function. The point is: I have created, using Arduino Uno, Nokia LCD display and buttons, a menu than allow me to do different things. Currently I am using millis to track the time (I’m waiting for an RTC module) and depends on my settings it turns ON or OFF a relay for a certain amount of time. Till here everything goes well but when I manually shut ON or OFF a relay some problems come.

I will explain it by an example: I have a small seeding house for my vegetable garden and I have to keep the light for 18 hours ON and 6 hours OFF but the light it’s annoying during my daily maintenance so I have to turn it OFF, after that the timing doesn’t work correctly.

I really cannot understand how get out of it.
Thanks for any suggestion.

sketch_apr18a.ino (27.3 KB)

it's going to be hard to find the problem in a 1000+ line program.

you could use a structure to hold state, time and pin values for each timed operation that can be passed to a common sub-function instead of duplicating code

the display code can be captured in a single sub-function that is passed pointers to one or more strings to display. Use sprintf to for a complete line instead of individual print()

seems that many of the menu cases are just setting the color to black or white. looks like it controls whether the display text is made visible or not.

looks like your menu code is a collection of if statements rather than a few if/else if or switch statements. Makes it hard to know which cases are exercised. And it 400 lines

the top of the file has a lot of unnecessary comments; variable names are often self explanatory. But there're very few comments in the code.

i'm sure you're reluctant to make changes after getting it to work, but such a large amount of code written this way is difficult to maintain and debug, as you're finding out

What do you mean "timing doesn't work properly"?

Form you code, the menu system appears to be how you are manually overriding the relay. Am I correct?

That code is toggling the relay state. Now you timing using millis() means that when the if statement detects that an interval has expired, it uses the current state of the relay to decide what to do.
Unfortunately that state is now "inverted" from what it is supposed to be according to the timers, and the wrong next interval is chosen as a result.

Is that what is happening?

I would suggest:

  1. Remove these from the start of loop():
 digitalWrite(6, relayone);
  digitalWrite(8, relaytwo);
  digitalWrite(9, relaythree);
  digitalWrite(10, relayfour);
  1. In your menu system where you override the state of the relays:
     if (digitalRead(6)) 
      {
        turnRelayOff();
      }
      else 
      {
        turnRelayOn();
      }

Now these functions will invert the current relay state without effecting the timing.
3) You'll also need to update the display routine:

   if (digitalRead(6)) 
    {
      display.print("OFF");
    }
      else 
    {
      display.print("ON");
    }

If you want the menu to reflect the current state of the relay (as opposed to the state of the timer). Although your logic looks wrong here? Surely if the relay is ON the light is ON?

digitalWrite(6, relayone);
  digitalWrite(8, relaytwo);
  digitalWrite(9, relaythree);
  digitalWrite(10, relayfour);

Wouldn’t that be simpler to deal with if it was:

for (int i=0; i<4; i++){
   digitalWrite(relayPins[i], relayStates[i]);
}

I bet it would make a lot of other things in your code a lot easier too.

Anytime you catch yourself adding numbers to names, what you really want is an array. Make those numbers something that your code can use.

pcbbc:
What do you mean "timing doesn't work properly"?

Form you code, the menu system appears to be how you are manually overriding the relay. Am I correct?

That code is toggling the relay state. Now you timing using millis() means that when the if statement detects that an interval has expired, it uses the current state of the relay to decide what to do.
Unfortunately that state is now "inverted" from what it is supposed to be according to the timers, and the wrong next interval is chosen as a result.

Is that what is happening?

I would suggest:

  1. Remove these from the start of loop():
 digitalWrite(6, relayone);

digitalWrite(8, relaytwo);
 digitalWrite(9, relaythree);
 digitalWrite(10, relayfour);



2) In your menu system where you override the state of the relays:


if (digitalRead(6))
     {
       turnRelayOff();
     }
     else
     {
       turnRelayOn();
     }



Now these functions will invert the current relay state without effecting the timing.
3) You'll also need to update the display routine:


if (digitalRead(6))
   {
     display.print("OFF");
   }
     else
   {
     display.print("ON");
   }



If you want the menu to reflect the current state of the relay (as opposed to the state of the timer). Although your logic looks wrong here? Surely if the relay is ON the light is ON?

I have tried your suggestion but it does not work. With my setup I have the millis function that switch the relay at specific time and the manual switching also works but with the problem exposed before.
With your suggestions the millis function works but I can not manually switch the relay.

The point of the problem is that I would like to manually switch a relay without changing the relay state in order to have a proper timing with millis (think it should go this way).

If you have any other suggestions I will appreciate it.
Thanks.