millis time delay help.

Hi all, after posting my first question on this forum and have my problem solved so quickly I thought I would give it ago again.

I using sinric to control some stepper motors this is working fine but I need to have my ‘relayPin1’ automatically switch off after a few seconds after been turned on.

I can do this with delay but then the motors don’t run so I need to use millis().

I will post my code below and would be very grateful if someone could point out where I’m going wrong.

#include <Arduino.h>
#include <ESP8266WiFi.h>
#include <ESP8266WiFiMulti.h>
#include <WebSocketsClient.h> //  https://github.com/kakopappa/sinric/wiki/How-to-add-dependency-libraries
#include <ArduinoJson.h> // https://github.com/kakopappa/sinric/wiki/How-to-add-dependency-libraries
#include <StreamString.h>


ESP8266WiFiMulti WiFiMulti;
WebSocketsClient webSocket;
WiFiClient client;

#define MyApiKey "840e2b02-a079-4f9e-b17b-0f6586e8a95e" // TODO: Change to your sinric API Key. Your API Key is displayed on sinric.com dashboard
#define MySSID "*******" // TODO: Change to your Wifi network SSID
#define MyWifiPassword "*******" // TODO: Change to your Wifi network password
#define INTERVAL_MESSAGE1 5000
#define HEARTBEAT_INTERVAL 300000 // 5 Minutes 

const int relayPin1 = 16; // TODO: Change according to your board
const int relayPin2 = 9; // TODO: Change according to your board
int myCounter = 0;
int mySwitchPin =9; 
int mySwitchPin1 =16;
int motorPin1 = 14;
int motorPin2 = 12;
int motorPin3 = 13;
int motorPin4 = 15;
int delayTime = 2;
float c ;

unsigned long startMillis;
unsigned long currentMillis;
const unsigned long period = 3000; 


uint64_t heartbeatTimestamp = 0;
bool isConnected = false;
 
#define DEVICE1 "5c4"  //TODO: Device ID of first device
#define DEVICE2 "5c4"  //TODO: Device ID of second device



void setPowerStateOnServer(String deviceId, String value);

// deviceId is the ID assgined to your smart-home-device in sinric.com dashboard. Copy it from dashboard and paste it here

void turnOn(String deviceId) {
  if (deviceId == DEVICE1)
  {  
    Serial.print("Turn on device id: ");
    Serial.println(deviceId);

     digitalWrite(relayPin1, LOW); // turn on relay with voltage LOW
       if ((relayPin1 == LOW ) && (currentMillis - startMillis >= period)) {
     digitalWrite(relayPin1, HIGH);  
     startMillis = currentMillis;  
   }
  else  if (deviceId == DEVICE2)
  {  
    Serial.print("Turn on device id: ");
    Serial.println(deviceId);
    
     digitalWrite(relayPin2, LOW); // turn on relay with voltage LOW
     
  }  
  else {
    Serial.print("Turn on for unknown device id: ");
    Serial.println(deviceId);
  }    
  }
}
void turnOff(String deviceId) {
   if (deviceId == DEVICE1)
   {  
     Serial.print("Turn off Device ID: ");
     Serial.println(deviceId);
     
     digitalWrite(relayPin1, HIGH);  // turn off relay with voltage HIGH
   }
   else if (deviceId == DEVICE2)
   {  
     Serial.print("Turn off Device ID: ");
     Serial.println(relayPin2);
     
     digitalWrite(relayPin2, HIGH);  // turn off relay with voltage HIGH
   }
   else {
     Serial.print("Turn off for unknown device id: ");
     Serial.println(deviceId);    
  }
}

void webSocketEvent(WStype_t type, uint8_t * payload, size_t length) {
  switch(type) {
    case WStype_DISCONNECTED:
      isConnected = false;    
      Serial.printf("[WSc] Webservice disconnected from sinric.com!\n");
      break;
    case WStype_CONNECTED: {
      isConnected = true;
      Serial.printf("[WSc] Service connected to sinric.com at url: %s\n", payload);
      Serial.printf("Waiting for commands from sinric.com ...\n");        
      }
      break;
    case WStype_TEXT: {
        Serial.printf("[WSc] get text: %s\n", payload);
                 
        DynamicJsonBuffer jsonBuffer;
        JsonObject& json = jsonBuffer.parseObject((char*)payload); 
        String deviceId = json ["deviceId"];     
        String action = json ["action"];
        
        if(action == "setPowerState") { // Switch or Light
            String value = json ["value"];
            if(value == "ON") {
                turnOn(deviceId);
            } else {
                turnOff(deviceId);
            }
        }
        else if (action == "test") {
            Serial.println("[WSc] received test command from sinric.com");
        }
      }
      break;
    case WStype_BIN:
      Serial.printf("[WSc] get binary length: %u\n", length);
      break;
  }
}

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

  startMillis = millis();  //initial start time

  pinMode(mySwitchPin, INPUT_PULLUP);
  pinMode(mySwitchPin1, INPUT_PULLUP);
  pinMode(motorPin1, OUTPUT);
  pinMode(motorPin2, OUTPUT);
  pinMode(motorPin3, OUTPUT);
  pinMode(motorPin4, OUTPUT);
  
    
  pinMode(relayPin1, OUTPUT);
  pinMode(relayPin2, OUTPUT);

  digitalWrite(relayPin1, HIGH);
  digitalWrite(relayPin2, HIGH);
  
  WiFiMulti.addAP(MySSID, MyWifiPassword);
  Serial.println();
  Serial.print("Connecting to Wifi: ");
  Serial.println(MySSID);  

  // Waiting for Wifi connect
  while(WiFiMulti.run() != WL_CONNECTED) {
    delay(500);
    Serial.print(".");
  }
  if(WiFiMulti.run() == WL_CONNECTED) {
    Serial.println("");
    Serial.print("WiFi connected. ");
    Serial.print("IP address: ");
    Serial.println(WiFi.localIP());
  }

  // server address, port and URL
  webSocket.begin("iot.sinric.com", 80, "/");

  // event handler
  webSocket.onEvent(webSocketEvent);
  webSocket.setAuthorization("apikey", MyApiKey);
  
  // try again every 5000ms if connection has failed
  webSocket.setReconnectInterval(5000);   // If you see 'class WebSocketsClient' has no member named 'setReconnectInterval' error update arduinoWebSockets
}

void loop() {
  currentMillis = millis();
  
  webSocket.loop();
  
  if(isConnected) {
      uint64_t now = millis();
      
      // Send heartbeat in order to avoid disconnections during ISP resetting IPs over night. Thanks @MacSass
      if((now - heartbeatTimestamp) > HEARTBEAT_INTERVAL) {
          heartbeatTimestamp = now;
          webSocket.sendTXT("H");          
  }
 }

 if (digitalRead(mySwitchPin) == LOW)
    clockwise();
    
  if (digitalRead(mySwitchPin1) == LOW)
    anticlockwise();
}

void clockwise()
{
  for (int i = 0; i < 510; i++)
  {
    digitalWrite(motorPin1, HIGH);
    digitalWrite(motorPin2, LOW);
    digitalWrite(motorPin3, LOW);
    digitalWrite(motorPin4, LOW);
    delay(delayTime);
    digitalWrite(motorPin1, LOW);
    digitalWrite(motorPin2, HIGH);
    digitalWrite(motorPin3, LOW);
    digitalWrite(motorPin4, LOW);
    delay(delayTime);
    digitalWrite(motorPin1, LOW);
    digitalWrite(motorPin2, LOW);
    digitalWrite(motorPin3, HIGH);
    digitalWrite(motorPin4, LOW);
    delay(delayTime);
    digitalWrite(motorPin1, LOW);
    digitalWrite(motorPin2, LOW);
    digitalWrite(motorPin3, LOW);
    digitalWrite(motorPin4, HIGH);
    delay(delayTime);
  }
}

void anticlockwise()
{
  for (int i = 0; i < 510; i++)
  {
    digitalWrite(motorPin1, LOW);
    digitalWrite(motorPin2, LOW);
    digitalWrite(motorPin3, LOW);
    digitalWrite(motorPin4, HIGH);
    delay(delayTime);
    digitalWrite(motorPin1, LOW);
    digitalWrite(motorPin2, LOW);
    digitalWrite(motorPin3, HIGH);
    digitalWrite(motorPin4, LOW);
    delay(delayTime);
    digitalWrite(motorPin1, LOW);
    digitalWrite(motorPin2, HIGH);
    digitalWrite(motorPin3, LOW);
    digitalWrite(motorPin4, LOW);
    delay(delayTime);
    digitalWrite(motorPin1, HIGH);
    digitalWrite(motorPin2, LOW);
    digitalWrite(motorPin3, LOW);
    digitalWrite(motorPin4, LOW);
    delay(delayTime);
  }
}



void setPowerStateOnServer(String deviceId, String value) {
  DynamicJsonBuffer jsonBuffer;
  JsonObject& root = jsonBuffer.createObject();
  root["deviceId"] = deviceId;
  root["action"] = "setPowerState";
  root["value"] = value;
  StreamString databuf;
  root.printTo(databuf);
  
  webSocket.sendTXT(databuf);
}

Thanks in advance

unsigned long startMillis;
unsigned long currentMillis;

There is nothing magic about Millis in the name of the variables. start? Start of what? Use a name that makes sense. Why would you want to type currentMillis, when it is so much easier to type now?

     digitalWrite(relayPin1, LOW); // turn on relay with voltage LOW
       if ((relayPin1 == LOW ) && (currentMillis - startMillis >= period)) {

Interesting.

const int relayPin1 = 16; // TODO: Change according to your board

I’m nearly certain that 16 is NEVER equal to 0 (the value of LOW).

You just wrote the pin LOW. Why do you think it is necessary to test (incorrectly) the state of the pin?

Did you look at that Blink Without Delay example? Do you see how that example sets the time that an event occurred? Where are YOU setting startMillis (the variable that I guess is supposed to be the time that the relay was set low)?

Put EVERY { on a line BY ITSELF. Put EVERY } on a line BY ITSELF. Use Tools + Auto Format, so you at least look like you know what you are doing.

Not a bad job, so far, though. Just lots of room for improvement.

When you get the instruction to turn on, record the time.

While you are waiting for new instructions, check the time and turn it off when required.

This results is the Arduino checking the time thousands of times per second. Fortunately it never gets bored asking "Are we there yet?

I think I understand how it should work but I can't get it to work so it would seem I am missing something.

Paul, the reason it looks like I don't know what I'm doing is because I don't. I started this for the first time a few weeks ago.

So as a complete novice I am looking for some help.

Thanks

C++ cannot read your mind :(

PaulS is giving you hints.

You have: const int relayPin1 = 16; that is, you are assigning 16 to relayPin1

You then ask a question: if ((relayPin1 == LOW ). . . well, relayPin1 can never be LOW because you gave it the value of 16.

I appreciate any help but as I say I am very new to this so cryptic clues really aren't of any use to me.

I'm not asking to for any one to do it for me, that is no way to learn.

Straight forward explanations is what I need.

I understand what you mean with regard to the pin can't be low with a value of 16. 16 was the GPIO pin I assigned.

Are you saying that I can't use GPIO16 for this purpose

How about if I change it to

int relayPin1 = 4;
int relayPin2 = 5;

Would that make a difference

The point being is: relayPin1 is 16 (the 16th digital pin)

You have: if ((relayPin1 == LOW ) && . . . You want: if(digitalRead(relayPin1) == LOW && . . .

However, just before the if(. . .) you have: digitalWrite(relayPin1, LOW); // turn on relay with voltage LOW Therefore there is no real need to check if the state of relayPin1 is LOW since you just made it LOW.

Ok, I get what you are saying with regard to digitalRead.

The reason I put that in there is I thought that even though I told the pin to go low I would then have to tell the program to be looking for this happening to then start the counter.

Should it look more like this

digitalWrite(relayPin1, LOW); // turn on relay with voltage LOW
       if (currentMillis - startMillis >= period) {
     digitalWrite(relayPin1, HIGH);  
     startMillis = currentMillis;  
   }

I really do appreciate the help.

The demo Several Things at a Time illustrates the use of millis() to manage timing without blocking. It may help with understanding the technique.

Have a look at Using millis() for timing. A beginners guide if you need more explanation.

...R

I have looked at several tutorials and examples but when trying to apply it to my sketch I am struggling

I thought I understood it but clearly I am missing something as I cant get it working. It is getting quite frustrating as I'm sure this is easy.

All I want is to replicate this

void turnOn(String deviceId) {
  if (deviceId == DEVICE1)
  {  
    Serial.print("Turn on device id: ");
    Serial.println(deviceId);

     digitalWrite(relayPin1, LOW); 
     delay 2000;
     digitalWrite(relayPin1, HIGH);    }
  else  if (deviceId == DEVICE2)

but using millis() instead of delay as I need the program to keep running during the delay.

Turning on can be done inside a specific function that's called from the web event. When you do that, record the time in a global variable.

When there are no events to process, loop() will repeat thousands of times per second. Inside that function, check the time. Fortunately the Arduino never gets bored asking "Are we there yet?" thousands of times per second.

I can get the delay working in a basic sketch using millis() in the loop() but I can't in the program I want to use it for.

Are you saying put the startMillis in the WebSocketEvent function?

It must be a global variable so that both functions can read and write to it.

The event which switches the thing on should set startMillis.

How would YOU turn a light off 2 hours after it was turned on?

You COULD sit and stare at a timer, and turn the light off as soon as the timer counted down to 0.

Or you could write down when the light was turned on. Periodically, yo'd see if the light was on, and, if so, whether the light had been on long enough.

The Arduino doesn't have a clock. It has millis() to tell you how long it has been running. You can record the value of millis() when some event o interest occurs, like when the LED is turned on.

The MIllis in the variable name is USELESS information. Where the value came from is not important. That it is the time some event occurred IS. So, use a name like lightTurnedOn, not startMillis. startMillis doesn't tell you anything useful. lightTurnedOndoes.

You can then determine how long the light has been on, and if it has been on long enough, and is actually on, you can turn it off.

if(millis() - lightTurnedOn > lightOnTime)
{
   // The light has been on long enough, if it is indeed on
   if(lightOnTime > 0)
   {
      // The light IS on. Turn it off and clear the on time

     lightOnTime = 0;
}

I understand and agree it easier if it relevant to the actual program you are using.

In my case 'motorStartOn' what i'm struggling with is where to put it.

I press a button on an app and via the sinric cloud its send a on signal which in turn set pin4 low.

So if I get this (which I may well not)

When the app sends the on signal I, want to start the millis() then periodically check to see a predetermined time (3 seconds) has passed if hasn't do nothing, if it has set pin 4 high,

D4vew5577: When the app sends the on signal I, want to start the millis() then periodically check to see a predetermined time (3 seconds) has passed if hasn't do nothing, if it has set pin 4 high,

Nearly. When the app sends the on signal I, want to store the value of millis() in an unsigned long variable then periodically check to see a predetermined time (3 seconds) has passed if hasn't do nothing, if it has set pin 4 high

Ok, so millis() starts as soon as the program starts, When the on signal is sent I need to record the time then check it to see if 3 seconds has passed from the time recorded?

D4vew5577: Ok, so millis() starts as soon as the program starts, When the on signal is sent I need to record the time then check it to see if 3 seconds has passed from the time recorded?

From the arduino documentation:

https://www.arduino.cc/reference/en/language/functions/time/millis/: Returns the number of milliseconds since the Arduino board began running the current program. This number will overflow (go back to zero), after approximately 50 days.

So, yes. For checking, if 3 seconds have elapsed, you need to subtract the current time from the time, you have received the on-signal and compare that to 3 seconds.

Here’s something that may work.

Start by declaring a couple of global variables. One, an unsigned long, will be used to store the system millis count when relay 1 is turned on. The other will be a boolean that tells another part of the code that the relay is on or off. The define is the number of millis we want to wait before turning off the relay.

.
.
.
#define RELAY1_SHUTOFF_TIME     3000ul
unsigned long
    timeRelayOneOn;
bool
    bRelay1State;
.
.
.

I also added a line to setup() to initialize that flag to false:

.
.
.
    digitalWrite(relayPin1, HIGH);
    digitalWrite(relayPin2, HIGH);
    bRelay1State = false;
.
.
.

In your turnOn() logic, change your relay 1 code to:

.
.
.
    if (deviceId == DEVICE1)
    {  
        Serial.print("Turn on device id: ");
        Serial.println(deviceId);

        digitalWrite(relayPin1, LOW); // turn on relay with voltage LOW
        timeRelayOneOn = millis();
        bRelay1State = true;
    }//if        
.
.
.

This sets the relay “on”, grabs the time and sets the flag to true, indicating the relay is on.

I would also modify your turnOff() code because it can turn relay 1 off too. There, all you need to do is set the flag false if relay 1 is turned off:

.
.
.
    if (deviceId == DEVICE1)
    {  
        Serial.print("Turn off Device ID: ");
        Serial.println(deviceId);
     
        digitalWrite(relayPin1, HIGH);  // turn off relay with voltage HIGH
        bRelay1State = false;
     
    }//if
.
.
.

In your loop, add the following. It checks to see if the relay is on (the flag is true) and if so, if the desired number of ticks has elapsed. If so, the relay is turned off and the flag is cleared:

.
.
.
    if( bRelay1State )
    {
        if( millis() - timeRelayOneOn >= RELAY1_SHUTOFF_TIME )
        {
            bRelay1State = false;
            digitalWrite(relayPin1, HIGH);
        }//if
        
    }//if
.
.
.

Blackfin: bool bRelay1State;

I suggest "bRelay1On" for better readability.

You might want to think about extracting the relay functionality into its own class. From your code I suspect you have two relays?

class TimedRelay
{
  private:
    uint8_t pinNumber;
    bool isOn;
    bool checkTimeToTurnOff;
    unsigned long turnOnTime;
    unsigned long turnOnDuration;
  public:
    void begin(uint8_t pin)
    {
      pinNumber = pin;
      turnOnTime = 0;
      checkTimeToTurnOff = false;
      
      pinMode(pin, OUTPUT);
      turnOff();
    }

    void turnOn()
    {
      isOn = true;
      digitalWrite(pinNumber, LOW);
    }

    void turnOff()
    {
      digitalWrite(pinNumber, HIGH);
      isOn = false;
      checkTimeToTurnOff = false;
    }

    void turnOnForMs(unsigned long ms)
    {
      turnOnDuration = ms;
      checkTimeToTurnOff = true;
      turnOn();
      turnOnTime = millis();
    }

    void spin()
    {
      if(!isOn)
        return;

      if(!checkTimeToTurnOff)
        return;
      
      if(millis() - turnOnTime >= turnOnDuration)
      {
        turnOff();
      }
    }

    bool isRelayOn()
    {
      return isOn;
    }
};

Now, for every relay you have to create a new global instance of TimedRelay:

TimedRelay gRelay1;
TimedRelay gRelay2;

In your setup:

void setup() {
  // ...
  gRelay1.begin(16);
  gRelay2.begin(9);
}

And in your command processing for turning on:

if (deviceId == DEVICE1)
    {  
        //...
        // Turns the relay on for 3000ms = 3s
        gRelay1.turnOnForMs(3000);
    }

And for turning off:

if (deviceId == DEVICE1)
    {  
        //...
        // Turns the relay off immediately
        gRelay1.turnOff();
     
    }

And in your loop you add:

void loop()
{
    //...
    // Spin needs to be called on every loop-iteration to check, if the
    // relay has to be turned off.
    gRelay1.spin();
}

By doing this your code will be a lot cleaner to read and to understand. It'll also be much easier to make changes to the relay code, e.g. if in the future you have to put it to HIGH to turn on. Also adding more relays won't be a lot of work, just add a new TimedRelay instance, call begin(pin) in your setup, and you are good to go.