2

I'm trying to have a millis() timer run to emulate a hardware timer inside a library, but I'm having issues compiling the code. I want the callback (pseudo ISR) to be part of the class, and not use global variables.

I'm using Christensen's / Monk's Timer library, but I'm open to suggestions if there are other better/easier timer libraries out there.

This is what I have so far. It's just an example for me to understand how this might work. I'm sure there are better ways of making an LED blink, but this is just for illustrative purposes.

Compiling gives a bunch of "invalid use of member variables in static functions" errors.

error: invalid use of member 'Blink::_led_status' in static member function int _led_status;

EDIT

After trying to add glue routines as per your other answer I can seem to get one instance working, but adding another stops the first light from blinking, and the second one blinks on and off at the wrong times.

The test library is on github

Blink.h

#ifndef Blink_h
#define Blink_h

#include "Arduino.h"
#include <Timer.h> // https://github.com/JChristensen/Timer/master
    // Timer.h uses millis() instead of built in timers 

class Blink {
public:
  Blink (int led_pin, int duration);
  void begin ();
  void updateTimer();

private:
  static void _callbackGlue();
  void _callback();

  static Blink *_instance;
  Timer *_blink_timer;
  int _led_pin;
  int _duration;
  int _led_status;
  int _blink_event;
};
#endif

blink.cpp

#include "Arduino.h"
#include "Blink.h"

Blink *Blink::_instance;

Blink::Blink (int led_pin, int duration) {
  _blink_timer = new Timer();
  _led_status = 0;
  _led_pin = led_pin;
  _duration = duration;
  pinMode(_led_pin, OUTPUT);
  digitalWrite(_led_pin, LOW);
}

void Blink::begin () {
  _blink_event = _blink_timer->every(_duration, _callbackGlue);
  _instance = this;
}

void Blink::_callbackGlue() {
  _instance->_callback();
}

void Blink::_callback() {
  if (_led_status == 0) {
    digitalWrite(_led_pin, HIGH);
    _led_status = 1;
  } else if (_led_status == 1) {
    digitalWrite(_led_pin, LOW);
    _led_status = 0;
  }
}

void Blink::updateTimer() {
  _blink_timer->update();
}

test/test.ino

#include "Blink.h"

Blink first_blink (13, 500);
//Blink second_blink (12, 1000);

void setup() {
  Serial.begin(9600);
  Serial.println("Starting...");
  first_blink.begin();
  //second_blink.begin();
} 

void loop() {
  first_blink.updateTimer();
  //second_blink.updateTimer();
}
  • 2
    I think this is a duplicate of Pass class's public function as an argument to external command - see my answer there. – Nick Gammon Jun 15 '16 at 5:36
  • @NickGammon Thanks for pointing me to your other answer. After trying to follow your example from the other answer, I was seemingly able to get a single instance working, but adding another blink object stops the first one from working, and the second one blinks randomly. – waspinator Jun 15 '16 at 18:46
  • Yes, well you only have one instance variable. You can't easily work around this. Classes (can) have multiple instances. For a static function to know which one to use you need to help it. The instance variable is static so there is only one copy of it. – Nick Gammon Jun 15 '16 at 22:02
1

Nick Gammon's answer is good for one Timer - but now you want two!

OK, then you can take advantage of the Timer Library's context parameter. What's that? If you take a look at the header for every(), you'll see as well as passing in the function to call, you can also pass in another value (called context) to pass back to that function when it fires*. So instead of you remembering just one copy of instance, get each Timer to remember it for you.

*If you can't see the context parameter in Timer.h, then make sure you're using the v2.1 branch, not master.

So:

  1. Get rid of _instance everywhere.
  2. Change:
    static void _callbackGlue();
    to
    static void _callbackGlue(void *context);
  3. Change:
    void Blink::_callbackGlue()
    {
    _instance->_callback();
    }
    to
    void Blink::_callbackGlue(void *context)
    {
    ((Blink *)context)->_callback();
    }
  4. Change:
    _blink_event = _blink_timer->every(_duration, _callbackGlue);
    to
    _blink_event = _blink_timer->every(_duration, _callbackGlue, this);

Note that I've submitted these changes as a Pull Request on your GitHub repo.


Incidentally:

You don't need _blink_timer to be a pointer. Get rid of the *, get rid of the line with new Timer() in it, and replace all your _blink_timer-> with _blink_timer.

  • Thanks, that worked great. Is the only downside to using pointers to objects that you have to manually delete the pointer when deleting its parent? – waspinator Jun 18 '16 at 16:15
  • Sort of. Yes, that's a concern, but new also uses heap (which is inefficient in both memory and time). By having it as a sub-object it merely makes the parent larger. Plus with pointers you have to use -> everywhere. Once you program enough, you'll get into the habit of thinking "Wait! Is that pointer actually 0?" No pointer: no test. – John Burger Jun 18 '16 at 16:30
0

You may add handler (AKA hook) functions using function pointer to the Timer0 ISR inside the core wiring.c source file shipped with Arduino core, and assign latter (attache) this function pointer to any desired function in the other library/class or in the main application code.

More about this trick in this article here.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.