help with my if statement in a class

I am building a relay control class that turns a relay on for XX seconds and turns it off based on millis(). I also want to use it for multitasking as well.However the if statement does not work and the relay never gets turned on. please help

auger_Control.ino (1 Bytes)

Post your sketch here, using code tags.

The file you attached is empty apart from a single blank line.

I apologise that I chose the wrong file.Here is the new one.
I can not find the code tags btw.

relaytest.ino (1.98 KB)

OP's code

//this class is made to control relay on amd off as timing based(non blocking delay methods)
// created on Jan14 2018 for use on my greenhouse control

class TimeRelay

//*************************************************time relay**************************************************************


{
  private:

    int _pinRelay;// which pin is used
    bool _onState;// 0 for low level triggered, 1 for high level triggered
    int _mode;// choose 1 for now for off delay(turn relay on for XX second and turn it off)
    int _onTime;// duration in second when powered on(receive 0 for low triggered and vise versa)
    int _offTime;// not use for now
    int _repeat;// 0 for 1 cycle,
    unsigned long _curM;
    int _relayPickedup = 0;


  public:

    TimeRelay(int pinRelay, bool onState, int mode, int onTime, int offTime, int repeat)
    {
      _pinRelay = pinRelay;
      _onState = onState;
      _mode = mode;
      _onTime = onTime;
      _offTime = offTime;
      _repeat = repeat;

      pinMode(_pinRelay, OUTPUT);
    }


    int offDelay ()// return the count number for  LCD display

    {


      // if relay has not been picked up then turn it on, until timeout(onTime)
      if (_relayPickedup = 0)
        
        {

       //   Serial.println(_onState );
          _curM = millis();
          digitalWrite(_pinRelay, _onState); //relay powers up once;
          _relayPickedup = 1;
        }


      if (_curM + _onTime < millis() )
      {
        if ( ((_curM + _onTime) - millis()) % 1000 == 0)
        {
          return (_onTime - ((_curM + _onTime) - millis()) / 1000);
        }
      }

      else // within time limit.

      {

        digitalWrite(_pinRelay, !_onState); //relay  powers off


      }


    }



}
;//end if class





TimeRelay timerelay(6, 0, 1, 10, 10, 0);




void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);    // serial / USB port

}

void loop() {
  // put your main code here, to run repeatedly:

  timerelay.offDelay();
  //delay(1000);

}

zhangyang1999:
.
I can not find the code tags btw.

Then you clearly didn't read this.

zhangyang1999:
the if statement does not work and the relay never gets turned on.

That's nowhere near enough information to be able to help you. What have you tried to debug or narrow down the problem?

Pieter

the if statement below does not get triggered even is true.

if (_relayPickedup = 0)
        
        {

       //   Serial.println(_onState );
          _curM = millis();
          digitalWrite(_pinRelay, _onState); //relay powers up once;
          _relayPickedup = 1;
        }
if (_relayPickedup = 0)

The 'if' statement is working perfectly, your usage is incorrect. You are giving it a condition that always evaluates to 'false'. Try:

if (_relayPickedup == 0)
      pinMode(_pinRelay, OUTPUT);

belongs inside a begin() function to be called by setup, in the constructor it may not work.

     if (_curM + _onTime < millis() )

don't use addition on timer values, use

     if (millis() - _curM >= _onTime)
    int offDelay ()

is missing a return statement at the bottom.

Does this suit your needs?

class TimeRelay {
  private:
    uint8_t relayPin;      // which pin is used
    unsigned int onTime;   // duration in milliseconds when powered on
    unsigned int offTime;  // duration in milliseconds when powered off
                           // (if repeating)
    bool activeLow;        // false for high level triggered, true for low level
    unsigned int repeat;   // 0 if repeating indefinitely

    enum : uint8_t { INACTIVE, OFF, ON } state = INACTIVE;
    unsigned long prevMillis;

    TimeRelay *next;         // Keep a linked list of all instances
    static TimeRelay *first;

  public:
    TimeRelay(uint8_t relayPin, bool activeLow = false)
      : relayPin(relayPin), activeLow(activeLow) {
      this->next = first;    // Insert this new TimeRelay at front of list
      first      = this;
    }

    ~TimeRelay() {
      if (this == first)
        first = nullptr;
      else
        for (TimeRelay *t = first; t != nullptr; t = t->next)
          if (t->next == this) {
            t->next = this->next;
            break;
          }
    }

    void begin() {
      pinMode(relayPin, OUTPUT);
    }

    void set(unsigned int onTime, unsigned int offTime = 0,
             unsigned int repeat = 1) {
      this->onTime  = onTime;
      this->offTime = offTime;
      this->repeat  = repeat;
      turnOn();
      state      = ON;
      prevMillis = millis();
    }

  private:
    void turnOn() {
      digitalWrite(relayPin, !activeLow);
    }
    void turnOff() {
      digitalWrite(relayPin, activeLow);
    }

  public:
    void update() {
      if (state == INACTIVE)
        return;

      unsigned long elapsed = millis() - prevMillis;
      if (state == OFF && elapsed >= offTime) {
        turnOn();
        state      = ON;
        prevMillis = millis();
        if (repeat)
          --repeat;
      } else if (state == ON && elapsed >= onTime) {
        turnOff();
        if (repeat == 1) {
          state = INACTIVE;
        } else {
          state      = OFF;
          prevMillis = millis();
        }
      }
    }

    static void loop() {  // Update all TimeRelay instances
      for (TimeRelay *t = first; t != nullptr; t = t->next)
        t->update();
    }
};

TimeRelay *TimeRelay::first = nullptr;

// -----------------------------------------------------------------------------

TimeRelay timerelay(6, false);

void setup() {
  timerelay.begin();            // initialize (pinMode)
  timerelay.set(1000, 2000, 3); // 1 second on, 2 seconds off, repeat 3 times
}

void loop() {
  TimeRelay::loop(); // Update all TimeRelay instances
}

Thank you for everybody's reply.

PieterP is great for making a such modified time relay code. thank you very much

I managed to make mine worked somehow because I made my second if statement backward so it is false all the time and loop through to "else" making relay inactive all the time. but yours is more efficient and much better thank you.

is it possible to make class possible to return a countdown number every second so I can feed it to the lcd display or is better to make countdown time independently. Much appreciated

zhangyang1999:
is it possible to make class possible to return a countdown number every second so I can feed it to the lcd display or is better to make countdown time independently. Much appreciated

    unsigned int update() {
      // [...]
      else if (state == ON) {
        if (elapsed < onTime)
          return onTime - elapsed;
        turnOff();
        if (repeat == 1) {
          state = INACTIVE;
        } else {
          state      = OFF;
          prevMillis = millis();
        }
      }
      return 0;
    }

I would split the function of display away from the function of update. As your system gets more complex you may not always be displaying the countdown on the screen and then it will not update because you didn't call update() anywhere except in the display code.

Make a new function called secondsLeft() or something descriptive.

I modified my code a bit. I initiated four objects from the begining these are k1,k2,k3,k4. I only called k1.offdelay and k2.offdelay inside the loop. However, k3 and k4 also get energised and stayed on forever.

It seems like once I initiated the object and associated relay picked up with out method offDelay called.

//this class is made to control relay on and off as timing based(non blocking delay methods)
// created on Jan14 2018 for use on my greenhouse control

class TimeRelay

//*************************************************time relay**************************************************************


{
  private:

    int _pinRelay;// which pin is used
    bool _onState;// 0 for low level triggered, 1 for high level triggered
    int _mode;// choose 1 for now for off delay(turn relay on for XX second and turn it off)
    int _onTime;// duration in millisecond when powered on(receive 0 for low triggered and vise versa)
    int _offTime;// not use for now
    int _repeat;// 0 for 1 cycle,
    unsigned long _currentMillis;
    bool _relayPickedup ;




  public:

    TimeRelay(int pinRelay, bool onState, int mode, int onTime, int offTime, int repeat)
    {
      _pinRelay = pinRelay;
      _onState = onState;
      _mode = mode;
      _onTime = onTime;
      _offTime = offTime;
      _repeat = repeat;

      pinMode(_pinRelay, OUTPUT);
      //  digitalWrite(_pinRelay, !_onState);
      // _relayPickedup = false;
    }


    int offDelay ()// return the countdown number in second for LCD display
    {

      // if relay has not been picked up then turn it on, until timeout(onTime)
      //relay only needs to get energized up once during the onTime period;

      if (_relayPickedup = false)
      {
        _currentMillis = millis();
        digitalWrite(_pinRelay, _onState);
        _relayPickedup = true;
      }

      // Serial.println(millis());

      if (_onTime   + _currentMillis >= millis())
      {
        /*      if ( ((_currentMillis + _onTime  ) - millis()) % 1000 == 0)
               {
                 int a = (_onTime - ((_currentMillis + _onTime) - millis()) / 1000); // returns a countdown in second for lcd display
                 return a;
                 //  Serial.println((a), '\n');
               }
        */
      }
      else // timeout.
      {
        digitalWrite(_pinRelay, !_onState); //relay deenergized
      }

    }

}
;//end if class

TimeRelay k1(4, 0, 1, 10000, 10, 0);
TimeRelay k2(5, 0, 1, 8000, 10, 0);
TimeRelay k3(6, 0, 1, 6000, 10, 0);
TimeRelay k4(7, 0, 1, 11000, 10, 0);


void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);    // serial / USB port

}

void loop() {
  // put your main code here, to run repeatedly:
  k1.offDelay();
  k2.offDelay();
  //  k3.offDelay();
  // k4.offDelay();



  //Serial.println((a), '\n');
  // delay(1000);

}

Why haven't you fixed the error pointed out in Reply #6?

Sorry gfvalvo , I though I corrected actually I did not.

After replacing =with == inside my statement, the relay stay on all the time other than turning off at different time.

//this class is made to control relay on and off as timing based(non blocking delay methods)
// created on Jan14 2018 for use on my greenhouse control

class TimeRelay

//*************************************************time relay**************************************************************


{
  private:

    int _pinRelay;// which pin is used
    bool _onState;// 0 for low level triggered, 1 for high level triggered
    int _mode;// choose 1 for now for off delay(turn relay on for XX second and turn it off)
    int _onTime;// duration in millisecond when powered on(receive 0 for low triggered and vise versa)
    int _offTime;// not use for now
    int _repeat;// 0 for 1 cycle,
    unsigned long _currentMillis;
    bool _relayPickedup ;




  public:

    TimeRelay(int pinRelay, bool onState, int mode, int onTime, int offTime, int repeat)
    {
      _pinRelay = pinRelay;
      _onState = onState;
      _mode = mode;
      _onTime = onTime;
      _offTime = offTime;
      _repeat = repeat;
      pinMode(_pinRelay, OUTPUT);

      //digitalWrite(_pinRelay, !_onState);
      // _relayPickedup = false;
    }


    int offDelay ()// return the countdown number in second for LCD display
    {
      
      // if relay has not been picked up then turn it on, until timeout(onTime)
      //relay only needs to get energized up once during the onTime period;

      if (_relayPickedup == false)
      {
        _currentMillis = millis();
        digitalWrite(_pinRelay, _onState);
        _relayPickedup == true;
      }

      // Serial.println(millis());

      if (_onTime   + _currentMillis >= millis())
      {
        /*      if ( ((_currentMillis + _onTime  ) - millis()) % 1000 == 0)
               {
                 int a = (_onTime - ((_currentMillis + _onTime) - millis()) / 1000); // returns a countdown in second for lcd display
                 return a;
                 //  Serial.println((a), '\n');
               }
        */
      }
      else // timeout.
      {
        digitalWrite(_pinRelay, !_onState); //relay deenergized
      }

    }

};//end of class




TimeRelay k1(4, 0, 1, 10000, 10, 0);
TimeRelay k2(5, 0, 1, 8000, 10, 0);
TimeRelay k3(6, 0, 1, 6000, 10, 0);
TimeRelay k4(7, 0, 1, 11000, 10, 0);
 //Flasher f1(13,100,100);
 //Flasher f2(7,11100,3000);
void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);    // serial / USB port



}

void loop() {

  // put your main code here, to run repeatedly:


  k1.offDelay();
  k2.offDelay();
  k3.offDelay();
  //k4.offDelay();
 //  f1.Update();
 //f2.Update();

  //Serial.println((a), '\n');
   

}