Homemade Arduino Library Modification

Tonight I decided I would try to make a very simple library for Arduino. I call it FlashingLED and it works with one function, flash(int on, int off), where on and off are microsecond values fed into a more or less BlinkWithoutDelay style millis() timer. It works fine for one LED, but when try to use it on more then one, the global variable "timer" is not unique to each LED so they screw with each other's timings and get all wonky.

Here is the header file:

#ifndef FlashingLED_h
#define FlashingLED_h

#include "Arduino.h"

class FlashingLED
{
public:
    FlashingLED(int pin);
    void flash(int on, int off);
private:
    int _pin;
};

#endif

And the source file:

#include "Arduino.h"
#include "FlashingLED.h"

FlashingLED::FlashingLED(int pin)
{
    pinMode(pin, OUTPUT);
    _pin = pin;
}

void FlashingLED::flash(int on, int off)
{
    millis();
    unsigned long timer = millis();
    while (millis() - timer < on)
    {
    digitalWrite(_pin, HIGH);
    }
    timer = millis();
    while (millis() - timer < off)
    {
        digitalWrite(_pin, LOW);
    }
    timer = millis();
    
}

As well as my example for one LED:

#include <FlashingLED.h>

FlashingLED myLED(13);

void setup()
{

}

void loop()
{
  myLED.flash(1000, 1000);
}

As far as "what have you tried," I have played with various values for the "on" and "off" values to see if it just looked odd because of my original times, and that wasn't the case. I then thought that maybe the pins were cross-talking due to "_pin = pin" in the source file, but decided that this wasn't necessarily the problem (but I have no real clue). I then decided that my problem was probably the fact that my source file code uses a global variable "timer" for all pins included.

Any ideas on how to restrict the timer variable to each pin?

Is there any way to give each pin its own timer based on something like timer(x) where x could just be a variable based on the pin number to keep the timers separate from pin to pin?

I would like to add that I am very new to coding and Arduino in general, I just like to learn by doing if possible. With that in mind, please don't start slinging techy programmer lingo at me (I assume this exists).

Compare your code:

while (millis() - timer < on)
    {
    digitalWrite(_pin, HIGH);
    }
    timer = millis();
    while (millis() - timer < off)
    {
        digitalWrite(_pin, LOW);
    }

with blink without delay.

No "while" there... :slight_smile:

Your first call to millis() makes no sense.

millis() function returns a value , which you have to assign to some kind of variable or else
use right away in some kind of comparison.

You also have to save the time when you turned the led on or off, so that when you call
millis() at future times, you can calculate how much time has elapsed since you last
flipped the led status.

This would suggest that there should be some private long variable containing the last
flip time, somewhere in your class.

Also, your class will tie up the computer forever, so you can't do anything else.
That's ok, if thats all you want to do.
But in general, thats not what you want to do.

tuxduino:
Compare your code:

while (millis() - timer < on)

{
    digitalWrite(_pin, HIGH);
    }
    timer = millis();
    while (millis() - timer < off)
    {
        digitalWrite(_pin, LOW);
    }




with blink without delay.

No "while" there... :)

Hence the "more or less." I was referring to BlinkWithoutDelay in the way that they both use millis() and a variable to track the times, even though my execution was different because I am setting two parameters for the blink rather than one.

Balls... Hadn't thought about the while statement sucking the life out of this.

michinyon:
Your first call to millis() makes no sense.

For some reason I felt like I had to initialize millis() before I could use it. I'm not sure why... :blush:

michinyon:
This would suggest that there should be some private long variable containing the last
flip time, somewhere in your class.

Woud making the timer variable private in the header make it only usable by one pin even when others were blinking other LEDs?

Beyond this, is there any way to make this work with different times for on and off without using while statements? Can I get away with Ifs?

I think you misunderstood that quote, the 'while' loops will wait or 'delay' until a timeout is reached. By using the looping behaviour provided by the sketch loop() function, you can build your code to to check millis each loop, but only act on it when it reaches a desired value.

It is the same principle you had except the rest of your program needs to be inside the loop, i.e the loop() function.

Can I get away with Ifs?

Does blink without delay use while()s or if()s ? :wink:

This is what I have (thrown together) as far as using ifs:

myLED.flash(200, 300);

//would equate to:

unsigned long timer = millis();

if(millis() - timer < on)
digitalWrite(_pin, HIGH);
else if(millis() - timer >= on && millis() - timer < on + off)
{
  digitalWrite(_pin, LOW);
  if(millis() - timer >= on + off)
    timer = millis();
}

But it isn't working... Hmmm...

PROBLEM SOLVED! XD

Header now reads:

#ifndef FlashingLED_h
#define FlashingLED_h

#include "Arduino.h"

class FlashingLED
{
public:
    FlashingLED(int pin);
    void flash(int on, int off);
private:
    int _pin;
    unsigned long timer;
};

#endif

Source Code:

#include "Arduino.h"
#include "FlashingLED.h"

FlashingLED::FlashingLED(int pin)
{
    pinMode(pin, OUTPUT);
    _pin = pin;
    unsigned long timer = millis();
}

void FlashingLED::flash(int on, int off)
{    
    if(millis() - timer < on)
        digitalWrite(_pin, HIGH);
    else if(millis() - timer >= on && millis() - timer < on + off)
    {
        digitalWrite(_pin, LOW);
    }
    else if(millis() - timer >= on + off)
        timer = millis();
}

And Arduino IDE:

/*   FlashingLEDExample 
 
 Created by Brandon Whittle
 
 Made to emulate Blink and BlinkWithoutDelay examples with 
 
 multiple LEDs with inequivalent on/off times.
 
 */

#include <FlashingLED.h>

FlashingLED myLED(13);
FlashingLED myLED2(12);

void setup()
{

}

void loop()
{
  myLED.flash(250, 250);
  myLED2.flash(1000, 1000);
}

Thank you for replying and sending me in the right direction!