Blinking with class

Hi,

So i learned blinking and other stuff , which led me to use millis() instead of delay and i found this tutorial on adafruit that ends up using C class to define the whole thing and than use instance of it to repeat same actions

https://learn.adafruit.com/multi-tasking-the-arduino-part-1/a-classy-solution

Question:

My question is that following codes called the flasher instance outside Void setup and loop, but what if I want to change flashing rates of leds depending on events. For example, if this happens flash led 1 at X rate and led2 at Y rate.

I tried to replace the number with integers so i can replace them instead of this Flasher led1(12, 100, 400); tried this to be Flasher led1(12, flashrate, 400);

void loop (){ int flashrate = 200 }

, but that attempt failed and light turned on continually.

class Flasher
{
    // Class Member Variables
    // These are initialized at startup
    int ledPin;      // the number of the LED pin
    long OnTime;     // milliseconds of on-time
    long OffTime;    // milliseconds of off-time

    // These maintain the current state
    int ledState;                     // ledState used to set the LED
    unsigned long previousMillis;      // will store last time LED was updated

  // Constructor - creates a Flasher 
  // and initializes the member variables and state
  public:
  Flasher(int pin, long on, long off)
  {
    ledPin = pin;
    pinMode(ledPin, OUTPUT);     
      
    OnTime = on;
    OffTime = off;

    ledState = LOW; 
    previousMillis = 0;
  }

  void Update()
  {
    // check to see if it's time to change the state of the LED
    unsigned long currentMillis = millis();
     
    if((ledState == HIGH) && (currentMillis - previousMillis >= OnTime))
    {
      ledState = LOW;  // Turn it off
      previousMillis = currentMillis;  // Remember the time
      digitalWrite(ledPin, ledState);  // Update the actual LED
    }
    else if ((ledState == LOW) && (currentMillis - previousMillis >= OffTime))
    {
      ledState = HIGH;  // turn it on
      previousMillis = currentMillis;   // Remember the time
      digitalWrite(ledPin, ledState);      // Update the actual LED
    }
  }
};


Flasher led1(12, 100, 400);
Flasher led2(13, 350, 350);

void setup()
{
}

void loop()
{
    led1.Update();
    led2.Update();
}

Hello,

One solution would be to make OnTime and OffTime public variables, so you could then do:

led1.OnTime = 200;

Globals are initialised BEFORE the function main which is where the Arduino API is initialized.

This means that the constructors for led1 and led2, which internally call to 'pinMode', is being called before the Arduino API is initialized. 'main' is then called which in turn initializes the Arduino API resetting the pins as inputs.

This is why thing like instances of Serial and others have a 'begin' method which sets Arduino dependent API's within 'setup'.

So, you should probably add a 'begin' routine to your Flasher class where you do the 'pinMode' setup and then add a call it with in 'setup' to finish any initialization for the class that is dependent upon Arduino API calls.

Thank you, YOU GUYS ARE GREAT! 8) made some changes, simulations seems to run fine and no error in compiler, plz check if i am doing it right

Changes:

  • moved onTime and offTime under public heading
  • removed led 2 instance for making it simpler and working first
  • remove ontime from " Flasher(int pin, long off)" as we are setting its value in loop
  • Declared led valuse as suggested, in the loop “led1.OnTime = 200;”
class Flasher
{
    // Class Member Variables,  These are initialized at startup
    int ledPin;      // the number of the LED pin
    // These maintain the current state
    int ledState;                 // ledState used to set the LED
    unsigned long previousMillis;   // will store last time LED was updated

    // Constructor - creates a Flasher and initializes the member variables and state
  public:
    long OnTime;     // milliseconds of on-time
    long OffTime;    // milliseconds of off-time
    Flasher(int pin, long off)
    {
      ledPin = pin;
      pinMode(ledPin, OUTPUT);

      
      OffTime = off;

      ledState = LOW;
      previousMillis = 0;
    }

    void Update()
    {
      // check to see if it's time to change the state of the LED
      unsigned long currentMillis = millis();

      if ((ledState == HIGH) && (currentMillis - previousMillis >= OnTime))
      {
        ledState = LOW;  // Turn it off
        previousMillis = currentMillis;  // Remember the time
        digitalWrite(ledPin, ledState);  // Update the actual LED
      }
      else if ((ledState == LOW) && (currentMillis - previousMillis >= OffTime))
      {
        ledState = HIGH;  // turn it on
        previousMillis = currentMillis;   // Remember the time
        digitalWrite(ledPin, ledState);   // Update the actual LED
      }
    }
};


Flasher led1(12, 100);


void setup()
{
}

void loop(){
  led1.OnTime = 200;
  led1.Update();
}

Usually, you do not make member fields public in order to allow access. Instead, you provide getters and setters that are public:

   public:
     long getOnTime();
     long getOffTime();

     void setOnTime(long onTime);
     void setOffTime(long offTime);

   private:
     long onTime;
     long offTime;

When you implement the setter, it can access (and overwrite) the private member field.

I also would make the onTime and offTime variable a unsigned long instead of a long. A long does work without a problem but you restrict yourself to a length of 2.147.483.647 seconds while with the same memory you can have a range of 4.294.967.295 seconds. Not to mention the error you have when you assign a negative onTime ;)

PaulS: Usually, you do not make member fields public in order to allow access. Instead, you provide getters and setters that are public:

Interesting point.

Usually, with a lot more RAM and cycles to play with, I would agree with you.

Given the resource limits of most Arduino boards and the tendency of OOP to consume comparatively more resources. I don't think the benefit of getter and setter functions, are worth the cost of the pointers. The limited resources self limits the complexity anyway.

When you implement the setter, it can access (and overwrite) the private member field.

So, the member property might as well be public.

Private declarations block inheritance, prevent code re-use and can be very frustrating. Private should be the last resort, when there is a very good reason to prevent a descendent accessing the member.

So, the member property might as well be public.

Except that a setter can, for instance, prevent assigning a negative time to the onTime or offTime variables.

Private declarations block inheritance, prevent code re-use and can be very frustrating.

They have there places. When a class should not be derived, for instance.

Not being able to access private member fields is not a reason for frustration. If the class developer thought the member should be private, there was probably good reason.

PaulS: Except that a setter can, for instance, prevent assigning a negative time to the onTime or offTime variables. They have there places. When a class should not be derived, for instance.

Not being able to access private member fields is not a reason for frustration. If the class developer thought the member should be private, there was probably good reason.

I agree with the point about using property accessors to provide error checking. Also trivial accessors can be declared inline in the class declaration and can have no additional overhead (complex data can be returned by reference if safe to do so).

However, I have noticed a trend for library writers to indiscriminately mark properties as private, where they could safely be protected. Deriving an object could be for a basic task, like adding another function to the class.

Deriving an object could be for a basic task, like adding another function to the class.

That's what an interface is for. 8)

PaulS: That's what an interface is for. 8)

And in the case of adding functionality which the original author has not considered (to provide virtual interfaces) deriving a class is how its done.

Maybe I'm missing what you are putting forward, but I consider an interface to be an abstract base class which one derives to fulfill the inheritance contract by providing implementations for the virtual members. And an ABC is not something that can be instantiated directly, due to not having implementation for certain members, as in an interface for something else (requires a derivative).

Objects can implement interfaces, without the object knowing anything about the interface that it is implementing. That's something I do everyday when developing CATIA applications. There is an interface, CATIAlias, for instance, that lets any object that should have an alias provide one, without having to modify the base code, because the developer didn't think, initially, that his/her object needed an alias. Anyone else can then decide that Joe's object should have an alias, and can then say that Joe's object implements the interface CATIAlias, by providing the implementation of that interface. Joe doesn't need to know that his object has an alias (but usually does, because before Tom implements the interface he says "Hey, Joe, you a*e, you forgot..."). C++, C#, and other languages provide this capability.

PaulS: Objects can implement interfaces, without the object knowing anything about the interface that it is implementing. That's something I do everyday when developing CATIA applications. There is an interface, CATIAlias, for instance, that lets any object that should have an alias provide one, without having to modify the base code, because the developer didn't think, initially, that his/her object needed an alias. Anyone else can then decide that Joe's object should have an alias, and can then say that Joe's object implements the interface CATIAlias, by providing the implementation of that interface. Joe doesn't need to know that his object has an alias (but usually does, because before Tom implements the interface he says "Hey, Joe, you a*e, you forgot..."). C++, C#, and other languages provide this capability.

This really makes little to no sense with regards to C++. Please provide an example of what you are trying to describe.

Objects can implement interfaces but they can do it without the object knowing anything about the interface... Uh huh

More like: A base class can provide interfaces, without the derived entity knowing the implementation (non-virtual functions).

And if you're talking about something deriving a class (using its interface, which it might not know) then aren't you simply reiterating what I previously posted?

This really makes little to no sense with regards to C++.

To you, maybe. It does to me.

Never mind, though. I'm not 100% certain on all the details, and I don't want to get something wrong in my description of how it works. It involves late-binding, though, which is something that the Arduino isn't capable of.

Your posted revision didn't take what I said into consideration.

So without much thought your code rephrased slightly and corrected -

class Flasher
{
    const uint8_t   pinLED;

    uint8_t         stateLED;
    unsigned long   tmsPrevious;

public:
    enum { LED_OFF = LOW, LED_ON = HIGH };

    unsigned long   tmsOnDuration;
    unsigned long   tmsOffDuration;


    // USE INITIALIZATION LIST SYNTAX
    // AND IN ORDER THE ORDER INTERNAL MEMBERS ARE DECLARED

    Flasher(int pin, unsigned long const tmsOff)
        : pinLED(pin)
        , stateLED(LED_OFF)
        , tmsPrevious(0UL)
        , tmsOffDuration(tmsOff)
    {   }

    void begin(uint8_t const state = Flasher::LED_OFF)
    {
        pinMode(pinLED, OUTPUT);

        digitalWrite(pinLED, state);
    }

    void Update()
    {
        unsigned long tms = millis();

        if ( (stateLED == LED_ON) && (tms - tmsPrevious >= tmsOnDuration) )
        {
            tmsPrevious = tms;

            stateLED = LED_OFF;
            digitalWrite(pinLED, stateLED);
        }
        else if ( (stateLED == LED_OFF) && (tms - tmsPrevious >= tmsOffDuration))
        {
            tmsPrevious = tms;

            stateLED = LED_ON;
            digitalWrite(pinLED, stateLED);
        }
    }
};


Flasher led1(12, 100);

void loop()
{
    led1.tmsOnDuration = 200UL;
    led1.Update();
}

void setup()
{
    led1.begin();
}

PaulS: To you, maybe. It does to me.

Never mind, though. I'm not 100% certain on all the details, and I don't want to get something wrong in my description of how it works. It involves late-binding, though, which is something that the Arduino isn't capable of.

Right...

PaulS: C++, C#, and other languages provide this capability.

Late binding is exactly what I explained: deriving an abstract base class and providing virtual method implementations. Otherwise known as dynamic dispatch.

And all this has little to do with the fact I mentioned deriving a class so as to add functionality that is not present in the base object. Absolutely nothing to do with virtual members/late binding/aliases.

Its just pure inheritance.

And back to the original point: Its a good reason to have protected members so someone can extend your class when you have [u]not provided the interface[/u] for the needed functionality.