A better way of a delay

I have a subroutine in a program which uses the delay function to allow time for a valve to move from on to off. I don't like using the delay function for oblivious reasons, and would love to sort this part of the code out to avoid using it. The problem is that I have three valves which use this part of the code and I can't figure out how to do this without mucking up the timing sequence on the other valves if they are using the timer function in this code at the same time. So I did it the lazy way and used delay.

I would also love to be able to display on screen which valve is moving but I can't work out how to pass this information to the screen.

void moveValve(char valveName){

digitalWrite(valveName, LOW);// Switches the relay on to send power to the valve motor.
   for (int i = 22; i >= 0; i--) {//valve movement takes 22 seconds, this displays the countdown on the screen
     u8g2.clearBuffer(); 
     u8g2.setFont(u8g2_font_profont10_tf ); 
     u8g2.setCursor(20,12);
     u8g2.print("   Valve movement ");
     u8g2.setCursor(25,25);
    // u8g2.print('valveName'); Not working
     u8g2.sendBuffer();
     u8g2.setFont(u8g2_font_courB24_tf );
     u8g2.setCursor (45,55);
     u8g2.print(i);
     u8g2.sendBuffer();
     delay(1000);
   }
   digitalWrite(valveName, HIGH);// Switches the relay off to stop the valve motor

Have you seen this >>>> SeveralThingsAtTheSameTime?

Needs to be written such that it is called frequently and with each call decides if it is time to turn the valve on or off.

You could call it every 1 seconds, for example, and see if 22 calls had come in so it is time to turn the valve off.

But a bigger issue is how do you want to publish what’s happening on three valves, potentially at once, on you screeb?

Or does the logic only ever run one valve at a time?

I think I am saying we need to see a bit more of your program, at least a small complete example of how the valve subroutine (function) is to be used.

a7

consider

byte pinsVal [] = { 11, 12, 13 };
#define N_VALS  sizeof(pinsVal)

byte          valState [N_VALS] = {}; 
unsigned long valMsec [N_VALS];
#define ValveOnMsec     1000

byte pinsBut [] = { A1, A2, A3 };
#define N_BUTS  sizeof(pinsBut)

byte butState [N_BUTS];

enum { OFF = HIGH, ON = LOW };

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

    for (unsigned n = 0; n < N_VALS; n++)  {
        digitalWrite (pinsVal [n], OFF);
        pinMode (pinsVal [n], OUTPUT);
    }

    for (unsigned n = 0; n < N_BUTS; n++)  {
        pinMode (pinsBut [n], INPUT_PULLUP);
        butState [n] = digitalRead (pinsBut [n]);
    }
}

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

    for (unsigned n = 0; n < N_VALS; n++)  {
        if (valState [n])  {
            if ( (msec - valMsec [n]) > ValveOnMsec)  {
                valState [n] = 0;
                digitalWrite (pinsVal [n], OFF);
            }

            char s [40];
            sprintf (s, "  valve %d open", n);
            Serial.println (s);
        }
    }

    for (unsigned n = 0; n < N_BUTS; n++)  {
        if (LOW == digitalRead (pinsBut [n]))  {
            valMsec  [n] = msec;
            valState [n] = 1;
            digitalWrite (pinsVal [n], ON);

        }
    }

    delay (100);    // avoid flood of prints
}

I would create a small class.
just "start" your timer/open the relay if you want to start your 22 seconds
call update() in loop

the relay object will deactive the relay if the time has passed.

you can have as many relays as you want - each one with it's own timer. Just create as many relays as you need.

Proof of concept


class Relay
{
  protected:
    const uint8_t pin;                 // a GPIO 
    const uint32_t interval = 22000;   // delay time
    uint32_t previousMillis = 0;
    bool isActive = false;
  public:
    Relay (uint8_t pin) : pin {pin}
    {}
    void begin()
    {
      pinMode(pin, OUTPUT);
      digitatlWrite(pin, HIGH);
    }
    void start()
    {
      previousMillis = millis();
      isActive = true;
      digitalWrite(pin, LOW);
    }
    void update()
    {
      if (isActive && millis() - previousMillis > interval)
      {
        isActive = false;
        digitalWrite(pin, HIGH);
      }
    }

};

Relay relay(2);  // create a relay object and assign pin

void setup() {
  relay.begin();     // call begin once in setup

  pinMode(A0, INPUT_PULLUP);  // a test button
}

void loop() {
  relay.update();    // call update in loop

  if (digitalRead(A0) == LOW) relay.start();  // an event to start the relay
}

Both the C version #4 and the C++ version #5 work directly unmodified. Although I did move any things off pin 13 and 12, the last pins I use as I am running out.

Neither begins the time-down period until the button is released. In the C++ version, this is easily handled by using the edge of the test button instead of the level; the C version integrates the buttons so it is a bit more tricky.

The delay(100); in the C version slows but does not stop the flood of output. It also delays the change due to the button press by some number of milliseconds, as many as 100, and makes the overall timing a bit less precise and the button inconsistent possibly sluggish. Again, this would be different if the code looked for and reacted to the edge, which can be recognized immediately. And would leave the loop running at full speed.

A possible exploitation a second button press during the time-down period could be to terminate early or extend by the delay period. Or like my toaster oven, provide a standard "A Bit More" toasting.

The OP didn't offer a full specification that would have let me write any code, even if I was gonna write the code for him.

In the C++ version, I suggest changing the begin routine, viz:

    void begin(uint32_t customDelay = 22000)
    {
        pinMode(pin, INPUT_PULLUP);

        interval = customDelay;
    }

And the update method could return the isActive status as bool to inform any printing or LED signaling &c.

C++ noob question: why are some things in the declaration and others in the begin method?

a7

Basically you need to stay away from hardware related code in the constructor. The hardware is not yet initialised at that point. You can pas the pin number to the begin() method. So after your constructor is called, the hardware is initialised and e.g. your pinMode might be reverted.

THX. So in this case, I could put the customDelay argument into the constructor.

I ask because I am trying to sort the syntax for an aggregate initilialiser to an array of Relays. With the determining things spread across the constructor and the begin(s) it was seemed impossible.

a7

shouldn't it be OUTPUT?
why not avoid the need for begin() and configure the pin in the constructor?
also initialize the output to off

1 Like

See post #7.

+1.

I think the code worked in the simulator because it has extra bright simulated LEDs that were running off the lull-up current…

They don’t burn out if you use them w/o a series resistor, either. :wink:

That might be something the wokwi folks would want to change, although I would never send something to the Moon on the basis of its passing a simulator test.

Oh, wait… :expressionless:

a7

Do the valves have built in limit switches to stop the motor at the end of travel? Post a link to the valve's datasheet or brand name and model number.

you are right.

no, you should not do hardware calls in the constructor.
Therefore the separate begin method to be called in setup().