Different behavior when class in in main file vs library

I’m trying to learn how to use classes and libraries. I made a test class for flashing LEDs so I can use the various blinking patterns as status indicators in other projects.

Here’s a sketch with the class written at the top. Everything works as expected.

class Flasher {

int _pinLED;
unsigned long shortFlash = 250;
int ledState = 0;
unsigned long previousMillis = 0;

public:
Flasher(int pinLED) {
  _pinLED = pinLED;
  pinMode(_pinLED, OUTPUT);
  digitalWrite(_pinLED, LOW);
}

void shortFlashes() {
  unsigned long currentMillis = millis();

  if ((ledState == 0) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 1;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, HIGH);
  }
  else if ((currentMillis - previousMillis) >= shortFlash) {
    ledState = 0;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, LOW);
  }
}
};


Flasher testLED(13);

void setup() {

}

void loop() {
  testLED.shortFlashes();
}

Here’s the code in .cpp and .h files

#include "Arduino.h"
#include "Flasher.h"

unsigned long shortFlash = 250;
int ledState = 0;
unsigned long previousMillis = 0;

Flasher::Flasher(int pinLED) {
  _pinLED = pinLED;
  pinMode(_pinLED, OUTPUT);
  digitalWrite(_pinLED, LOW);
}

void Flasher::shortFlashes() {
  unsigned long currentMillis = millis();

  if ((ledState == 0) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 1;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, HIGH);
  }
  else if ((currentMillis - previousMillis) >= shortFlash) {
    ledState = 0;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, LOW);
  }
}
#ifndef _FLASHER_H
#define _FLASHER_H

#include "Arduino.h"


class Flasher
{
	public:
		Flasher(int pinLED);
		void shortFlashes();

	private:
		unsigned long shortFlash;
		int ledState;
		unsigned long previousMillis;
		int _pinLED;

};

#endif

And here’s the sketch that uses the library:

#include <Flasher.h>

Flasher testLED(13);

void setup() {

}

void loop() {
  testLED.shortFlashes();
}

This compiles fine but the LED is constantly on rather than flashing. The other weird thing is that the voltage across the LED pin is constantly 2.5 V (compared to 5 V when the class is written directly into the sketch). This makes me think it’s constantly alternating the pin between HIGH/LOW or something, but I can’t figure out why since the code is the same when the class is included vs put in a library.

Does it help if you set the initial state of your output pin explicitly in setup(), rather than in your class constructor?

Nope. Just tried and got the same behavior.

You appear to have defined private instance variables in the .h file and simple global variables (with the same name) in the .cpp file.

Try removing these from the .cpp file and initialize the value (of the instance variables) in the constructor.
unsigned long shortFlash = 250 ;
int ledState = 0 ;
unsigned long previousMillis = 0 ;

You may still have a problem, mentioned earlier, that the constructor is invoked before the Arduino environment is set up, so statements like pinMode() may not work there. The usual technique is to have a begin() method for such initialisation.

Thanks, that makes sense. I removed the global variables and made a begin function. I updated the code below, but I’m still getting the same behavior where the LED is constantly on at 2.5 V.

Sketch:

#include <Flasher.h>

Flasher testLED(13);

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

void loop() {
  
  testLED.shortFlashes();
}

.h file:

#ifndef _FLASHER_H
#define _FLASHER_H

#include "Arduino.h"


class Flasher
{
	public:
		Flasher(int pinLED);
		void begin();
		void shortFlashes();

	private:
		unsigned long shortFlash;
		int ledState;
		unsigned long previousMillis;
		int _pinLED;

};

#endif

.cpp file:

#include "Arduino.h"
#include "Flasher.h"


Flasher::Flasher(int pinLED) {
  _pinLED = pinLED;
  unsigned long shortFlash = 250;
  unsigned long longFlash = 1000;
  int ledState = 0;
  unsigned long previousMillis = 0;
}

void Flasher::begin() {
  pinMode(_pinLED, OUTPUT);
  digitalWrite(_pinLED, LOW);
}

void Flasher::shortFlashes() {
  unsigned long currentMillis = millis();

  if ((ledState == 0) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 1;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, HIGH);
  }
  else if ((currentMillis - previousMillis) >= shortFlash) {
    ledState = 0;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, LOW);
  }
}

and for good measure I checked the sketch that includes the class definition at the top and the LED flashes as intended.

class Flasher {
  private:
    int _pinLED;
    unsigned long shortFlash = 250;
    unsigned long longFlash = 1000;
    int ledState = 0;
    unsigned long previousMillis = 0;

  public:
    Flasher(int pinLED) {
      _pinLED = pinLED;
      unsigned long shortFlash = 250;
      unsigned long longFlash = 1000;
      int ledState = 0;
      unsigned long previousMillis = 0;
    };

    void begin() {
      pinMode(_pinLED, OUTPUT);
      digitalWrite(_pinLED, LOW);
    };

    void shortFlashes() {
      unsigned long currentMillis = millis();

      if ((ledState == 0) && ((currentMillis - previousMillis) >= shortFlash)) {
        ledState = 1;
        previousMillis = currentMillis;
        digitalWrite(_pinLED, HIGH);
      }
      else if ((currentMillis - previousMillis) >= shortFlash) {
        ledState = 0;
        previousMillis = currentMillis;
        digitalWrite(_pinLED, LOW);
      }
    };
};

Flasher testLED(13);

void setup() {
  testLED.begin();

}

void loop() {
  testLED.shortFlashes();
}

Additional details:

  • I've tried both putting the .h and .cpp files in the library directory (C:\Users\me\Documents\Arduino\libraries\Flasher) as well as just making additional tabs to save them in the sketch folder. Both behave the same way.
  • I made a function that uses delay() to blink and the LED blinks as expected.
  • I changed the structure of shortFlashes() to case-switch and I get the constant 2.5 V LED.
#include "Flasher.h"

Flasher testLED(13);

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

void loop() {
 
  testLED.shortFlashes();
  //testLED.delayFlashes();
}
#include "Arduino.h"
#include "Flasher.h"


Flasher::Flasher(int pinLED) {
  _pinLED = pinLED;
  unsigned long shortFlash = 250;
  unsigned long longFlash = 1000;
  int ledState = 0;
  unsigned long previousMillis = 0;
}

void Flasher::begin() {
  pinMode(_pinLED, OUTPUT);
  digitalWrite(_pinLED, LOW);
}

void Flasher::shortFlashes() {
  unsigned long currentMillis = millis();

  if ((currentMillis - previousMillis) >= shortFlash) {
    switch (ledState) {
      case 0:
        digitalWrite(_pinLED, HIGH);
        ledState = 1;
        previousMillis = currentMillis;
        break;
      case 1:
        digitalWrite(_pinLED, LOW);
        ledState = 0;
        previousMillis = currentMillis;
        break;
      default:
        ledState = 0;
    }
  }
};


void Flasher::delayFlashes() {
  digitalWrite(_pinLED, HIGH);
  delay(500);
  digitalWrite(_pinLED, LOW);
  delay(500);
}
#ifndef _FLASHER_H
#define _FLASHER_H

#include "Arduino.h"


class Flasher
{
  public:
    Flasher(int pinLED);
    void begin();
    void shortFlashes();
    void delayFlashes();

  private:
    unsigned long shortFlash;
    int ledState;
    unsigned long previousMillis;
    int _pinLED;

};

#endif

For one thing you're re-declaring the variables in your .cpp file that you've already declared in the .h - fix that and see if it cures the problem.

I suspect the Arduino IDE automagic prototyping is somehow fixing that problem for you when you have it all in the .ino file.

That's it. I thought I had fixed it in the last edit but apparently I'm still confused about what variables exist where. Lesson learned. Thank you!

You’re welcome! The automatic prototyping can sometimes bite you in weird ways when you start putting code in other files.

Post #4 and post #5 still contain unwanted definitions of variables with same name, some of which are local to the constructor.

Try the following, based on post #4. Change the .h file to add the following private variable:

unsigned long longFlash ;

Change the constructor in the .cpp file to:

Flasher::Flasher(int pinLED) {
_pinLED = pinLED;
shortFlash = 250;
longFlash = 1000;
ledState = 0;
previousMillis = 0;
}

Got it. Thanks!

Here are the updated files.

#ifndef _FLASHER_H
#define _FLASHER_H

#include "Arduino.h"


class Flasher
{
	public:
		Flasher(int pinLED);
		void begin();
		void shortFlashes();
		void longFlashes();
		void twoFlashes();
		void threeFlashes();
	private:
		unsigned long shortFlash;
		unsigned long longFlash;
		int ledState;
		unsigned long previousMillis;
		int _pinLED;

};

#endif
#include "Arduino.h"
#include "Flasher.h"


Flasher::Flasher(int pinLED) {
  _pinLED = pinLED;
  shortFlash = 250;
  longFlash = 1000;
  ledState = 0;
  previousMillis = 0;
}

void Flasher::begin() {
  pinMode(_pinLED, OUTPUT);
  digitalWrite(_pinLED, LOW);
}

void Flasher::shortFlashes() {
  unsigned long currentMillis = millis();

  if ((ledState == 0) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 1;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, HIGH);
  }
  else if ((currentMillis - previousMillis) >= shortFlash) {
    ledState = 0;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, LOW);
  }
}

void Flasher::longFlashes() {
  unsigned long currentMillis = millis();

  if ((ledState == 0) && ((currentMillis - previousMillis) >= longFlash)) {
    ledState = 1;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, HIGH);
  }
  else if ((currentMillis - previousMillis) >= longFlash) {
    ledState = 0;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, LOW);
  }
}

void Flasher::twoFlashes() {
  unsigned long currentMillis = millis();

  if ((ledState == 0) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 1;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, HIGH);
  }
  if ((ledState == 1) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 2;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, LOW);
  }
  if ((ledState == 2) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 3;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, HIGH);
  }
  if ((ledState == 3) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 4;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, LOW);
  }
  else if ((currentMillis - previousMillis) >= longFlash) {
    ledState = 0;
  }
}

void Flasher::threeFlashes() {
  unsigned long currentMillis = millis();

  if ((ledState == 0) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 1;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, HIGH);
  }
  if ((ledState == 1) && ((currentMillis - previousMillis) >= shortFlash)) {
      ledState = 2;
      previousMillis = currentMillis;
      digitalWrite(_pinLED, LOW);
  }
  if ((ledState == 2) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 3;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, HIGH);
  }
  if ((ledState == 3) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 4;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, LOW);
  }
  if ((ledState == 4) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 5;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, HIGH);
  }
  if ((ledState == 5) && ((currentMillis - previousMillis) >= shortFlash)) {
    ledState = 6;
    previousMillis = currentMillis;
    digitalWrite(_pinLED, LOW);
  }
  else if ((currentMillis - previousMillis) >= longFlash) {
    ledState = 0;
  }
}