relay control with a timer. Repeatedly turning on and off only once

Greetings;

The code below works:

I turn the recirc pump relay on between the hours of ON and OFF.

While on, I toggle a couple of relays in order to switch polarities on some copper electrodes.

//------------------------Relay manipulation routines------------------

#define RECIRC_PUMP 30  //motor relay
#define ECO_RELAY_1 31  //relay for polarity switching
#define ECO_RELAY_2 32  //relay for polarity switching

#define RecircOnHour 16
//#define RecircOnMinute 01
#define RecircOffHour 17
//#define RecircOffMinute 01

void relaySetup() {                 // set all 8 ports to OUTPUT and HIGH
  for (byte i = 30; i < 38; i++) {
    digitalWrite(i, HIGH);  //-----------------------------------------------> I want to make sure the relay doesn't give me an ON glitch. is this OK?
    pinMode(i, OUTPUT);
    digitalWrite(i, HIGH);
  }
} //eof relaySetup()

void relay() {
  if (hour() >= RecircOnHour && hour() < RecircOffHour) {   // if time is within ON and OFF hours
    digitalWrite(RECIRC_PUMP, LOW);                         // turn on PUMP
    if (minute() % 2) plus24V();
    else if (!(minute() % 2)) minus24V();
  } //eof if time check
  else {                                                    // time is outside of ON or OFF hours, pump and ECo are off
    digitalWrite(RECIRC_PUMP, HIGH);                        // turn off pump
    off24V();
  }
} //eof relay()

void plus24V() {
  digitalWrite(ECO_RELAY_1, LOW);   // turn on relay 1
  digitalWrite(ECO_RELAY_2, HIGH);  // relay 2 is opposite for power
}
void minus24V() {                 
  digitalWrite(ECO_RELAY_1, HIGH);  // turn off relay 1
  digitalWrite(ECO_RELAY_2, LOW);   // relay 2 is opposite for power
}

void off24V() {                     // turn off both when both are LOW or both are HIGH, the electrodes are OFF!
  digitalWrite(ECO_RELAY_1, HIGH);                        
  digitalWrite(ECO_RELAY_2, HIGH);
}

the relay() about once per second.

I have this nagging feeling that addressing the control pins every second is going to create a problem at one point.

It seems that it would be better to test for ON, turn on the pump relay and not touch it until OFF time comes.

Same goes for reversing polarities: on even minutes flip relays once, and don't touch again until the minute turns odd...

I have a tentative solution, which sets and resets a Pump flag and a 24V flag, but this obviously adds two additional global variables. Not ideal.

Any hints to tweak the code would be appreciated.

Btw, there's also the comment in the setupRelay() on presetting the output state prior to changing pinMode...

Cheers.

Writing the same value repeatedly to a control pin will have no effect on your program or your relay. It is very common in code to do it this way. There is no need to only detect the transition from ON/OFF or OFF/ON and write it once.

consider

#if 0
#define RECIRC_PUMP 30  //motor relay
#define ECO_RELAY_1 31  //relay for polarity switching
#define ECO_RELAY_2 32  //relay for polarity switching

#else
#define RECIRC_PUMP 10
#define ECO_RELAY_1 11
#define ECO_RELAY_2 12
#endif

#define ON  LOW
#define OFF HIGH

#define RecircOnHour 16
#define RecircOffHour 17

enum { STOP, POS, NEG };
int state = STOP;

char s [20];

// -----------------------------------------------------------------------------
long time = (57L + (15L * 60)) * 60;

int seconds (void) { return  time % 60; };
int minutes (void) { return (time / 60) % 60; };
int hour (void)    { return (time / 3600 % 24); }

void timeUpdate (void)
{
    time += 15;

    sprintf (s, "%2d:%02d:%02d", hour (), minutes (), seconds ());
    Serial.println (s);

    delay (100);
}

// -----------------------------------------------------------------------------
void relay (
    byte  pin,
    byte  state )
{
    digitalWrite (pin, state);

    sprintf (s, "  relay:  %d, %s", pin, ON == state ? "ON" : "OFF");
    Serial.println (s);
}

// ---------------------------------------------------------
void plus24V () {
    relay (ECO_RELAY_1, ON);   // turn on relay 1
    relay (ECO_RELAY_2, OFF);  // relay 2 is opposite for power
}

void minus24V () {
    relay (ECO_RELAY_1, OFF);  // turn off relay 1
    relay (ECO_RELAY_2, ON);   // relay 2 is opposite for power
}

void off24V () {  // turn off both when both are ON or both are OFF, the electrodes are OFF!
    relay (ECO_RELAY_1, OFF);
    relay (ECO_RELAY_2, OFF);
    relay (RECIRC_PUMP, OFF);
}

// -----------------------------------------------------------------------------
void loop (void)
{
    timeUpdate ();

    switch (state)  {
    case POS:
        // Serial.println ("    POS");
        if (RecircOffHour <= hour ())  {
            off24V ();
            state = STOP;
        }
        else if (minutes () % 2)  {
            minus24V ();
            state = NEG;
        }
        break;

    case NEG:
        // Serial.println ("    NEG");
        if (RecircOffHour <= hour ())  {
            off24V ();
            state = STOP;
        }
        else if (! (minutes () % 2))  {
            plus24V ();
            state = POS;
        }
        break;

    default:
    case STOP:
        // Serial.println ("    STOP");
        if (RecircOnHour <= hour () && hour () < RecircOffHour) {
            relay (RECIRC_PUMP, ON);
            plus24V ();
            state = POS;
        }
        break;
    }
}

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


    digitalWrite (RECIRC_PUMP, OFF);
    digitalWrite (ECO_RELAY_1, OFF);
    digitalWrite (ECO_RELAY_2, OFF);

    pinMode (RECIRC_PUMP, OUTPUT);
    pinMode (ECO_RELAY_1, OUTPUT);
    pinMode (ECO_RELAY_2, OUTPUT);
}

results

15:57:15
15:57:30
15:57:45
15:58:00
15:58:15
15:58:30
15:58:45
15:59:00
15:59:15
15:59:30
15:59:45
16:00:00
  relay:  10, ON
  relay:  11, ON
  relay:  12, OFF
16:00:15
16:00:30
16:00:45
16:01:00
  relay:  11, OFF
  relay:  12, ON
16:01:15
16:01:30
16:01:45
16:02:00
  relay:  11, ON
  relay:  12, OFF
16:02:15
16:02:30
16:02:45
16:03:00
  relay:  11, OFF
  relay:  12, ON
16:03:15
16:03:30
16:03:45
16:04:00
  relay:  11, ON
  relay:  12, OFF
16:04:15
16:04:30
16:04:45
16:05:00
  relay:  11, OFF
  relay:  12, ON
16:05:15
16:05:30
16:05:45
16:06:00
  relay:  11, ON
  relay:  12, OFF
16:06:15
16:06:30
16:06:45
16:07:00
  relay:  11, OFF
  relay:  12, ON
16:07:15
16:07:30
16:07:45
16:08:00
  relay:  11, ON
  relay:  12, OFF
16:08:15
16:08:30
16:08:45
16:09:00
  relay:  11, OFF
  relay:  12, ON
16:09:15
16:09:30
16:09:45
16:10:00
  relay:  11, ON
  relay:  12, OFF
16:10:15
16:10:30
16:10:45
16:11:00
  relay:  11, OFF
  relay:  12, ON
16:11:15
16:11:30
16:11:45
16:12:00
  relay:  11, ON
  relay:  12, OFF
16:12:15
16:12:30
16:12:45
16:13:00
  relay:  11, OFF
  relay:  12, ON
16:13:15
16:13:30
16:13:45
16:14:00
  relay:  11, ON
  relay:  12, OFF
16:14:15
16:14:30
16:14:45
16:15:00
  relay:  11, OFF
  relay:  12, ON
16:15:15
16:15:30
16:15:45
16:16:00
  relay:  11, ON
  relay:  12, OFF
16:16:15
16:16:30
16:16:45
16:17:00
  relay:  11, OFF
  relay:  12, ON
16:17:15
16:17:30
16:17:45
16:18:00
  relay:  11, ON
  relay:  12, OFF
16:18:15
16:18:30
16:18:45
16:19:00
  relay:  11, OFF
  relay:  12, ON
16:19:15
16:19:30
16:19:45
16:20:00
  relay:  11, ON
  relay:  12, OFF
16:20:15
16:20:30
16:20:45
16:21:00
  relay:  11, OFF
  relay:  12, ON
16:21:15
16:21:30
16:21:45
16:22:00
  relay:  11, ON
  relay:  12, OFF
16:22:15
16:22:30
16:22:45
16:23:00
  relay:  11, OFF
  relay:  12, ON
16:23:15
16:23:30
16:23:45
16:24:00
  relay:  11, ON
  relay:  12, OFF
16:24:15
16:24:30
16:24:45
16:25:00
  relay:  11, OFF
  relay:  12, ON
16:25:15
16:25:30
16:25:45
16:26:00
  relay:  11, ON
  relay:  12, OFF
16:26:15
16:26:30
16:26:45
16:27:00
  relay:  11, OFF
  relay:  12, ON
16:27:15
16:27:30
16:27:45
16:28:00
  relay:  11, ON
  relay:  12, OFF
16:28:15
16:28:30
16:28:45
16:29:00
  relay:  11, OFF
  relay:  12, ON
16:29:15
16:29:30
16:29:45
16:30:00
  relay:  11, ON
  relay:  12, OFF
16:30:15
16:30:30
16:30:45
16:31:00
  relay:  11, OFF
  relay:  12, ON
16:31:15
16:31:30
16:31:45
16:32:00
  relay:  11, ON
  relay:  12, OFF
16:32:15
16:32:30
16:32:45
16:33:00
  relay:  11, OFF
  relay:  12, ON
16:33:15
16:33:30
16:33:45
16:34:00
  relay:  11, ON
  relay:  12, OFF
16:34:15
16:34:30
16:34:45
16:35:00
  relay:  11, OFF
  relay:  12, ON
16:35:15
16:35:30
16:35:45
16:36:00
  relay:  11, ON
  relay:  12, OFF
16:36:15
16:36:30
16:36:45
16:37:00
  relay:  11, OFF
  relay:  12, ON
16:37:15
16:37:30
16:37:45
16:38:00
  relay:  11, ON
  relay:  12, OFF
16:38:15
16:38:30
16:38:45
16:39:00
  relay:  11, OFF
  relay:  12, ON
16:39:15
16:39:30
16:39:45
16:40:00
  relay:  11, ON
  relay:  12, OFF
16:40:15
16:40:30
16:40:45
16:41:00
  relay:  11, OFF
  relay:  12, ON
16:41:15
16:41:30
16:41:45
16:42:00
  relay:  11, ON
  relay:  12, OFF
16:42:15
16:42:30
16:42:45
16:43:00
  relay:  11, OFF
  relay:  12, ON
16:43:15
16:43:30
16:43:45
16:44:00
  relay:  11, ON
  relay:  12, OFF
16:44:15
16:44:30
16:44:45
16:45:00
  relay:  11, OFF
  relay:  12, ON
16:45:15
16:45:30
16:45:45
16:46:00
  relay:  11, ON
  relay:  12, OFF
16:46:15
16:46:30
16:46:45
16:47:00
  relay:  11, OFF
  relay:  12, ON
16:47:15
16:47:30
16:47:45
16:48:00
  relay:  11, ON
  relay:  12, OFF
16:48:15
16:48:30
16:48:45
16:49:00
  relay:  11, OFF
  relay:  12, ON
16:49:15
16:49:30
16:49:45
16:50:00
  relay:  11, ON
  relay:  12, OFF
16:50:15
16:50:30
16:50:45
16:51:00
  relay:  11, OFF
  relay:  12, ON
16:51:15
16:51:30
16:51:45
16:52:00
  relay:  11, ON
  relay:  12, OFF
16:52:15
16:52:30
16:52:45
16:53:00
  relay:  11, OFF
  relay:  12, ON
16:53:15
16:53:30
16:53:45
16:54:00
  relay:  11, ON
  relay:  12, OFF
16:54:15
16:54:30
16:54:45
16:55:00
  relay:  11, OFF
  relay:  12, ON
16:55:15
16:55:30
16:55:45
16:56:00
  relay:  11, ON
  relay:  12, OFF
16:56:15
16:56:30
16:56:45
16:57:00
  relay:  11, OFF
  relay:  12, ON
16:57:15
16:57:30
16:57:45
16:58:00
  relay:  11, ON
  relay:  12, OFF
16:58:15
16:58:30
16:58:45
16:59:00
  relay:  11, OFF
  relay:  12, ON
16:59:15
16:59:30
16:59:45
17:00:00
  relay:  11, OFF
  relay:  12, OFF
  relay:  10, OFF
17:00:15
17:00:30
17:00:45
17:01:00
17:01:15
17:01:30
17:01:45

Thanks All;

Here's what I came up with, which doesn't use global variables.

//------------------------Relay manipulation routines------------------

#define RECIRC_PUMP 30  //motor relay
#define ECO_RELAY_1 31  //relay for polarity switching
#define ECO_RELAY_2 32  //relay for polarity switching

#define RecircOnHour 17
//#define RecircOnMinute 01
#define RecircOffHour 21
//#define RecircOffMinute 01

void relaySetup() {                 // set all 8 ports to OUTPUT and HIGH
  for (byte i = 30; i < 38; i++) {
    digitalWrite(i, HIGH);  //-----------------------------------------------> I want to make sure the relay doesn't give me an ON glitch. is this OK?
    pinMode(i, OUTPUT);
    digitalWrite(i, HIGH);
  }
} //eof relaySetup()

void relay() {
  if ( digitalRead(RECIRC_PUMP) && (hour() >= RecircOnHour) && (hour() < RecircOffHour)) recircPumpON();  // if pump was OFF if time is right, turn pump ON
  if (!digitalRead(RECIRC_PUMP) && (hour() >= RecircOffHour)) recircPumpOFF(); // if pump is ON and time has passed, turn pump OFF
  if (!digitalRead(RECIRC_PUMP) &&  (minute() % 2) && digitalRead(ECO_RELAY_1)) plus24V();  // if pump is ON toggle electrodes
  if (!digitalRead(RECIRC_PUMP) && !(minute() % 2) && digitalRead(ECO_RELAY_2)) minus24V(); // if pump is ON toggle electrodes
} //eof relay()

void recircPumpON() {
  digitalWrite(RECIRC_PUMP, LOW);                    // turn on PUMP
  //digitalWrite(ECO_RELAY_1, LOW);
  Serial.println("Pump ON");
}
void recircPumpOFF() {
  digitalWrite(RECIRC_PUMP, HIGH);                    // turn on PUMP
  off24V();
  Serial.println("Pump OFF");
}

void plus24V() {
  digitalWrite(ECO_RELAY_1, LOW);   // turn on relay 1
  digitalWrite(ECO_RELAY_2, HIGH);  // relay 2 is opposite for power
  //Serial.println("plus24V");
} //eof plus24V
void minus24V() {
  digitalWrite(ECO_RELAY_1, HIGH);  // turn off relay 1
  digitalWrite(ECO_RELAY_2, LOW);   // relay 2 is opposite for power
  //Serial.println("minus24V");
} //eof minus24V
void off24V() {                     // turn off both when both are LOW or both are HIGH, the electrodes are OFF!
  digitalWrite(ECO_RELAY_1, HIGH);
  digitalWrite(ECO_RELAY_2, HIGH);
  Serial.println("off24V");
} //eof off24V

I just test for the state of the output pins, which are effectively flags.

An IF to test for being within the ON period to turn ON the pump

Another IF to test if we're over the ON period to turn OFF the pump. Since time is uni-directional (as long as you believe in conventional science), testing for before the ON period doesn't make sense.

And two IFs to toggle the electrode direction.

This way, we never rewrite the outputs. It seems the code is cleaner too.

I notice that the last 3 IFs have a !digitalRead(RECIRC_PUMP) leading the test. I guess this could be put reformulated as a leading IF with that test only and concatenated IFs for the remainder...

It took me a while to detect a typo:

  if (!digitalRead(RECIRC_PUMP) &&  (minute() % 2) && digitalRead(ECO_RELAY_1)) plus24V();  // if pump is ON toggle electrodes
  if (!digitalRead(RECIRC_PUMP) && !(minute() % 2) && digitalRead(ECO_RELAY_2)) minus24V(); // if pump is ON toggle electrodes

the plus24V(); was typed as plus24V; - no brackets. the IDE didn't catch it, and I went through a good hour of testing to figure out, why plus24V() never executed in that IF statement. Any idea, why this wouldn't get flagged by the compiler?

Unless there are ideas to reduce this code further, I'm going to mark it closed, when I wake up in the AM.

Cheers;

It seems like the code is less clean since those additional tests inside each if() are not really doing anything useful, but it does work. If I were to look at that code, my initial thought may be that reading the state of the pump somehow changes the logic rather than a simple time on/off test.

But hey, it's your code and it works!

trilife:
Unless there are ideas to reduce this code further, I'm going to mark it closed, when I wake up in the AM.

did you reduce the code?

i often look at code size, # lines, as a measure of improvement. it's often a subjective assessment of improvement/enhancement vs added lines. did adding lines make the code easier to understand. testing the code in simulation can quickly reveal unexpected behaviors

writing code in conventional ways helps find bugs quicker. writing code to be more readable is for your benefit.

if you're going to rely on the pin state as state variables, there's certainly no need to repeatedly read the pin. why not simply read it once in a local variable within the function?

bool circPump = digitalRead (RECIRC_PUMP);
bool eco1       = digitalRead (ECO_RELAY_1);

and if several conditions depend on the same thing why not use a nested if

    if (! circPump)  {
      if (hour() >= RecircOffHour)
            ... 
      else if (minute() %2 && eco1)
            ... 
    }

putting the body of an if statement on the same line as the conditional test is bad practice.
should be

    // time to turn pump ON?
    if ( digitalRead(RECIRC_PUMP) && (hour() >= RecircOnHour) && (hour() < RecircOffHour))
        recircPumpON();

the plus24V(); was typed as plus24V; - no brackets. the IDE didn't catch it

You didn't choose the "All warnings" or "extended Messages when compiling" options :

void foo() {Serial.println("foo");}
void setup() {
  Serial.begin(9600); 
  foo;
}
void loop() {}

Gives warning: statement has no effect [-Wunused-value] for line 4