Relay out of time but no errors, compiling two sketches

Hi,

I'm setting up an indoor farming box using sensors and relays for pumps. I have both sensor code and relay code working correctly individually, however when i compile them together the relay looses time. Upon turning on it seems to stay on for longer, and off for shorter, but after that it seems to be holding time okay.

I can get the sensor data printed to the serial monitor without an issue however.

class Switcher 
{ 
// class member variables 
  int relayPin; // number of pin for relay
  long OnTime;  
  long OffTime;

  int relayState; // set relay state (active low)
  unsigned long previousMillis; // set time since last update


public: //constructor
Switcher(int pin, long on, long off) 
{ 
  relayPin = pin;
  pinMode(relayPin, OUTPUT); 

  OnTime = on; 
  OffTime = off;

  relayState = HIGH; 
  previousMillis = 0; 
  
}

void Update() 
{
 // check the time to see if relays need to be turned on
 // or off 
  unsigned long currentMillis = millis();

  if ((relayState == LOW) && (currentMillis - previousMillis>= OnTime))
  { 
   relayState = HIGH; // Turn it off 
   previousMillis = currentMillis; // Remember the time 
   digitalWrite(relayPin, relayState); //update the relay 
   }
   else if ((relayState == HIGH) && (currentMillis - previousMillis >= OffTime)) 
   {
   relayState = LOW; // turn it on 
   previousMillis = currentMillis; 
   digitalWrite(relayPin, relayState); 
   }
 }
 }; 

 Switcher relay1(8, 1000, 10000); //set the pin, time on time off for relay 1 class member 

 
void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600); //for sensor values 
 
  delay (150); // to not send too much data in one go
}

void loop() {
  // put your main code here, to run repeatedly
    relay1.Update(); //check to see if its time to switch relay
  
  // sensor read 
  sensorValue = analogRead(sensorPin);
  percent = convertToPercent(sensorValue);
  printValuesToSerial();
  delay(1000);
}

int convertToPercent(int value) //make the sensor values a percentage 
{
  int percentValue = 0;
  percentValue = map(value, 1023, 400, 0, 100);
  return percentValue;
}

void printValuesToSerial() // print sensor values to monitor 
{
  Serial.print("\n\nAnalog Value: ");
  Serial.print(sensorValue);
  Serial.print("\nPercent: ");
  Serial.print(percent);
  Serial.print("%");


}

Any help would be appreciated and sorry if i've just made a dumb error!!

B

Your Switcher class uses millis() and loop() contains a delay()

Is that a good idea I wonder ?

This is not all your code.

Sorry, this is all of the code

const byte sensorPin = A0;  //set moisture sensor pin
int sensorValue = 0;
int percent = 0; // sensor read

// class

class Switcher
{
    // class member variables
    int relayPin; // number of pin for relay
    long OnTime;
    long OffTime;

    int relayState; // set relay state (active low)
    unsigned long previousMillis; // set time since last update


  public: //constructor
    Switcher(int pin, long on, long off)
    {
      relayPin = pin;
      pinMode(relayPin, OUTPUT);

      OnTime = on;
      OffTime = off;

      relayState = HIGH;
      previousMillis = 0;

    }

    void Update()
    {
      // check the time to see if relays need to be turned on
      // or off
      unsigned long currentMillis = millis();

      if ((relayState == LOW) && (currentMillis - previousMillis >= OnTime))
      {
        relayState = HIGH; // Turn it off
        previousMillis = currentMillis; // Remember the time
        digitalWrite(relayPin, relayState); //update the relay
      }
      else if ((relayState == HIGH) && (currentMillis - previousMillis >= OffTime))
      {
        relayState = LOW; // turn it on
        previousMillis = currentMillis;
        digitalWrite(relayPin, relayState);
      }
    }
};

Switcher relay1(8, 10000, 30000); //set the pin, time on time off for relay 1 class member


void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600); //for sensor values

  delay (150); // to not send too much data in one go
}

void loop() {
  // put your main code here, to run repeatedly
  relay1.Update(); //check to see if its time to switch relay

  // sensor read
  sensorValue = analogRead(sensorPin);
  percent = convertToPercent(sensorValue);
  printValuesToSerial();
  delay(1000);
}

int convertToPercent(int value) //make the sensor values a percentage
{
  int percentValue = 0;
  percentValue = map(value, 1023, 400, 0, 100);
  return percentValue;
}

void printValuesToSerial() // print sensor values to monitor
{
  Serial.print("\n\nAnalog Value: ");
  Serial.print(sensorValue);
  Serial.print("\nPercent: ");
  Serial.print(percent);
  Serial.print("%");


}

UKHeliBob:
Your Switcher class uses millis() and loop() contains a delay()

Is that a good idea I wonder ?

I'm guessing its not a good idea. Although im new to this so not sure the alternative. I can take the delay under setup out but the delay connected to the sensor readings is to give a reading per second.

If you can manage to use millis() to time your relays, you should also be able to use the exact same method to time your sensor readings :slight_smile:

EDIT: As said in another thread, you are wasting memory on "too large" data types. Pin number and pin state in int's instead of byte's. On/off time in long's instead of unsigned int's..

but the delay connected to the sensor readings is to give a reading per second.

There are better ways, obviously. Look at the blink without delay example. You can read a sensor or write to a file, or the serial port, periodically (once a second) just as easily as you can toggle the state of a pin.

I'm guessing its not a good idea.

As implied in my question I am not sure whether it will cause a problem but having gone to the trouble of writing a class that uses millis() it seems a shame to use delay() in loop()

so not sure the alternative

using millis() for timing would be the obvious way to do it.

And another side note: You are setting the pin's mode in the Switcher class' constructor. That constructor is invoked before "setup()" is called and that means that you are configuring hardware which has not yet been initialized. Usually you would have a "Switcher::begin()" method which can be called in "setup()" in order to do hardware config's.

I assumed as much and made the corrections. Anyhow, I compiled and it's been running fine for the last hour. What's the problem?

You changed this as well...
Switcher relay1(8, 10000, 30000);