Problem with own function

Hello everyone!
My problem is as follow: I have written small function which is blinking a led with fixed interval, just like turn indicator in a car. When I use this function only once, let's say for left side blinker it works good, but when I use it twice, then the LED is lit for a fracture of second.
What I have noticed is that when I remove else lightOn = false;this line, then both LEDs are on together.

byte left = 4; 
byte  leftSwitch = 8; 
byte right = 5; 
byte  rightSwitch = 9; 


void setup() {
  Serial.begin(9600);
  pinMode(leftSwitch, INPUT_PULLUP);
  pinMode(left, OUTPUT);
  digitalWrite(left, HIGH);
  pinMode(rightSwitch, INPUT_PULLUP);
  pinMode(right, OUTPUT);
  digitalWrite(right, HIGH);

}

void loop() {
  blinker(leftSwitch, left, 500);
  blinker(rightSwitch, right, 700);
}

void blinker(byte input, byte output, unsigned long blinkTime) {
  static bool lightOn = false;
  static unsigned long timeAfter;
  if (digitalRead(input) == LOW) {
    if (millis() - timeAfter >= blinkTime) {
      lightOn = ! lightOn;
      Serial.println(input);
      timeAfter = millis();
    }
  }
  else lightOn = false;
  
  if (lightOn) {
    digitalWrite(output, LOW);
  }
  else digitalWrite(output, HIGH);
}

You've only got static variables in your function for one LED.

That's why we have classes.

You could do this with an array of structures:

#include <Arduino.h>

//structures
typedef struct structBlinker
{
    uint8_t     pinOut;
    uint8_t     pinIn;
    bool        bState;
    uint32_t    timeBlink;
    uint32_t    timePeriod;    
    
}sBlinker_t;

//includes

//defines

//constants
const uint8_t pinLeft = 4;
const uint8_t pinRight = 5;
const uint8_t pinLeftSw = 8;
const uint8_t pinRightSw = 9;


//variables
sBlinker_t
    blinkers[2];
    
void setup( void )
{
    blinkers[0].pinOut = pinLeft;
    blinkers[0].pinIn = pinLeftSw;
    blinkers[0].bState = false;
    blinkers[0].timePeriod = 500ul;
    pinMode( blinkers[0].pinOut, OUTPUT );
    pinMode( blinkers[0].pinIn, INPUT_PULLUP );
    
    blinkers[1].pinOut = pinRight;
    blinkers[1].pinIn = pinRightSw;
    blinkers[1].bState = false;
    blinkers[1].timePeriod = 700ul;
    pinMode( blinkers[1].pinOut, OUTPUT );
    pinMode( blinkers[1].pinIn, INPUT_PULLUP );
        
}//setup

void loop( void )
{
    Blinkers();
    
}//loop

void Blinkers( void )
{
    static uint8_t
        index = 0;
    uint32_t timeNow = millis();

    if( (timeNow - blinkers[index].timeBlink) >= blinkers[index].timePeriod )
    {        
        blinkers[index].timeBlink = timeNow;
        blinkers[index].bState ^= true;
        if( digitalRead( blinkers[index].pinIn ) == LOW )        
            digitalWrite( blinkers[index].pinOut, blinkers[index].bState ? LOW:HIGH );
        else
            digitalWrite( blinkers[index].pinOut, HIGH );                   
        
    }//if

    index++;
    if( index == 2 )
        index = 0;
    
}//Blinkers

or with a simple class setup like:

#include <Arduino.h>

//classes
class Blinker
{
    public:
        Blinker( uint8_t pinOut, uint8_t pinTrigger, uint32_t delayTime );
        void run();    
        void begin(); 
        
    private:
        uint8_t _pinOut;
        uint8_t _pinTrigger;
        bool    _state;
        uint32_t _delayTime;
        uint32_t _timeBlink;    

};
    
Blinker::Blinker( uint8_t pinOut, uint8_t pinTrigger, uint32_t delayTime )
{
    _pinOut = pinOut;
    _pinTrigger = pinTrigger;
    _delayTime = delayTime;
        
}//constructor

void Blinker::begin( void )
{
    pinMode( _pinOut, OUTPUT );
    pinMode( _pinTrigger, INPUT_PULLUP );
    _state = false;
    _timeBlink = millis();
    
}//begin

void Blinker::run( void )
{
    uint32_t timeNow = millis();

    if( (timeNow - _timeBlink) >= _delayTime )
    {        
        _timeBlink = timeNow;
        _state ^= true;
        if( digitalRead( _pinTrigger ) == LOW )        
            digitalWrite( _pinOut, _state ? LOW:HIGH );
        else
            digitalWrite( _pinOut, HIGH );                   
        
    }//if
        
}//run

//constants
const uint8_t pinLeft = 4;
const uint8_t pinRight = 5;
const uint8_t pinLeftSw = 8;
const uint8_t pinRightSw = 9;

//variables
Blinker leftBlinker( pinLeft, pinLeftSw, 500ul );
Blinker rightBlinker( pinRight, pinRightSw, 700ul );

void setup( void )
{
    leftBlinker.begin();
    rightBlinker.begin();
        
}//setup

void loop( void )
{
    leftBlinker.run();
    rightBlinker.run();
        
}//loop

Normally for a class you'd have a separate .h and .cpp for the class. I don't do that here for clarity.

Both compile/tested on Mega2560. YMMV.

1 Like

Here's the example I'd been working on..

// Auto blinker switch example.
//
// The auto blinker question comes up regulary in the Arduino forum so
// I figured an example of that would be just the ticket. This uses two
// binker objects for the blinking LEDs and three mechButton objects for
// the three buttons. A left, right and Hazard button. The first press
// of a button, say right, turns on that blinker. The second turns it
// back off. If another button is pressed, say left then the code just
// switches from right to left. So each button is one click on, second
// click off.
//
// The buttons should be wired one side to the pin number choosen and
// the other to ground. the mechButton object will do the debounceing
// for you as well as call your callback routines when clicked. The
// blinker objects take care of the blinking. All these behind the
// scenes functions happen when you call idle() in your loop function.
//
// You can add more stuff to your loop() function if you want. But DO
// NOT USE delay(). No fear, this stuff comes with sleep() wich acts
// on the main loop() just like delay(), but lets the background stuff
// keep running.
//
// Have fun!
// jim lee



#include <blinker.h>
#include <mechButton.h>

#define BLINK_PERIOD  1000  // Ms
#define BLINK_LIT     500   // ms
#define BLINK_L_PIN   5     // Pin NUM
#define BLINK_R_PIN   6     // Pin NUM
#define BTN_L_PIN     2     // Pin NUM
#define BTN_R_PIN     3     // Pin NUM
#define BTN_H_PIN     4     // Pin NUM

enum blinkComs { blinkOff, blinkLeft, blinkRight, blinkHazard };  // Set up our list of states.
blinkComs   currentState;                                         // A variable to save our current state.
blinker     leftBlinker(BLINK_L_PIN, BLINK_LIT, BLINK_PERIOD);    // The left blinker object. A "fire and forget" blinker object.
blinker     rightBlinker(BLINK_R_PIN, BLINK_LIT, BLINK_PERIOD);   // The right blinker object. Same as left, different pin.
mechButton  lButton(BTN_L_PIN);                                   // The left blinker button. Another "fire and forget" object.
mechButton  rButton(BTN_R_PIN);                                   // The right blinker button. Same as left, different pin.
mechButton  hButton(BTN_H_PIN);                                   // And your hazard button. Wouldn't be complete without them.


// Standard setup.
void setup() {

  currentState = blinkOff;          // We start with the blinkers off. So note that.
  lButton.setCallback(clickLBtn);   // Hook the button objects to their respective callbacks.
  rButton.setCallback(clickRBtn);   // Hooking..
  hButton.setCallback(clickHBtn);   // Hooking..
}


// When the LEFT button is clicked, this is called..
void clickLBtn(void) {

  if (!lButton.trueFalse()) {         // If the button has been grounded..
    if (currentState == blinkLeft) {  // If we are already doing this mode..
      setBlinkers(blinkOff);          // We just shut off everything.
    } else {                          // Else, doing something else..
      setBlinkers(blinkLeft);         // We switch to our mode.
    }
  }
}


// When the RIGHT button is clicked, this is called..
void clickRBtn(void) {

  if (!rButton.trueFalse()) {         // If the button has been grounded..
    if (currentState == blinkRight) { // If we are already doing this mode..
      setBlinkers(blinkOff);          // We just shut off everything.
    } else {                          // Else, doing something else..
      setBlinkers(blinkRight);        // We switch to our mode.
    }
  }
}


// When the HAZARD button is clicked, this is called..
void clickHBtn(void) {

   if (!hButton.trueFalse()) {            // If the button has been grounded..]
      if (currentState == blinkHazard) {  // If we are already doing this mode..
         setBlinkers(blinkOff);           // We just shut off everything.
      } else {                            // Else, doing something else..
         setBlinkers(blinkHazard);        // We switch to our mode.
      }
   }
}


// Set the blinkers to a new mode. Then note what mode they are in.
void setBlinkers(blinkComs theBlinkCom) {

  switch (theBlinkCom) {              // If the new mode is..
    case blinkOff     :               // blinkOff.
      leftBlinker.setOnOff(false);    // Left blinker off.
      rightBlinker.setOnOff(false);   // Right blinker off.
      break;
    case blinkLeft    :               // blinkLeft
      leftBlinker.setOnOff(true);     // Left blinker on.
      rightBlinker.setOnOff(false);   // Right blinker off.
      break;
    case blinkRight   :               // blinkRight
      leftBlinker.setOnOff(false);    // Left blinker off.
      rightBlinker.setOnOff(true);    // Right blinker on.
      break;
    case blinkHazard  :               // blinkRight
      leftBlinker.setOnOff(false);    // Left blinker off. (Both off to synk them up)
      rightBlinker.setOnOff(false);   // Right blinker off.
      leftBlinker.setOnOff(true);     // Left blinker on.
      rightBlinker.setOnOff(true);    // Right blinker on.
      break;
  }
  currentState = theBlinkCom;         // And note the new state of the universe.
}


// Everything is run behind the scenes so all loop()
// needs to do is call idle() to let it all run.
void loop() {
  idle();
}

You'll need LC_baseTools installed (IDE library manager) to compile it.

Have fun!

-jim lee

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.