Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whish: Make callbacks (e.g. of timer) fully compatible to lambda functions #17

Closed
euphi opened this issue Jun 28, 2016 · 5 comments
Closed

Comments

@euphi
Copy link
Collaborator

euphi commented Jun 28, 2016

In a project I make use of automaton in several classes. There is another class where I just need a timer to call a function repeatedly (other said: I just need a scheduler).

There are several solutions:

  1. Make the used class (named "Safety" in my case) a Machine to use atm_timer_millis and check in loop() function if timer has expired.
  2. Use Atm_timer and add callback to call member function of my class "Safety".

I prefer the second solution, but the atm_cb_push_t does not allow to call a member function of the class.

Possible solutions:

  1. Create an "interface" class (pure virtual) for callbacks and store pointers to objects of this type. E.g. something like this:

    class atm_callback { void callback(int idx, int v, int up) = 0; };

    This class then could be inherited by classes that needs to provide a callback.

    Drawback: Only on callback possible per class.

  2. Allow use of lambda functions that captures this pointer.
    I tried to use this, but the existing signature atm_cb_push_t does not allow a lambda function that capture this. I don't have experience with lambda functions, so I don't know what needs to be changed:

    timer.onTimer([this] ( int idx, int v, int up ) {this->timerexpired();}, 0);

    results in a compile error:
    src/Safety.cpp:19:79: error: no matching function for call to 'Atm_timer::onTimer(Safety::init()::__lambda0, int)'

    However, without this, the code compiles fine:
    timer.onTimer([] ( int idx, int v, int up ) {Serial.print("Lambda!");}, 0);

@tinkerspy
Copy link
Owner

Hi euphi,

Like you I tried to do that capture thing earlier, but I failed as well. :-(

Could you store your classes in an array and then use the idx parameter to access the right one?

Perhaps the best solution would be to model your class as a fully fledged state machine, so everything fits seamlessly. I just created a tutorial that shows how to do that with the Machine Editor tool:

https://github.com/tinkerspy/Automaton/wiki/Machine-building-tutorial-2

Rgdz,
Tinkerspy

@euphi
Copy link
Collaborator Author

euphi commented Jun 28, 2016

I played a little with lambda functions and tried the following:

To atm_connector.hpp I added:

include <functional>
typedef std::function<void (int idx, int v, int up)> atm_cb_lambda_t;

In the anonymous union I store a pointer to such an object:

union {
    struct {
      union {
        atm_cb_push_t push_callback;
        atm_cb_pull_t pull_callback;
        atm_cb_lambda_t* lambda_callback; // ### Test
      };
      int callback_idx;  // +2 = 5 bytes per connector/object
    };
[....]
    void set( atm_cb_lambda_t* callback, int idx, int8_t logOp = 0, int8_t relOp = 0 ); // ### Test
[...]

To atm_connector.cpp I added:

In bool atm_connector::push(..)

 case MODE_LAMBDA:
        (*lambda_callback)( callback_idx, v, up );
        return true;

At least, this compiles without warning, but I'm not able to test yet (no time).

The reason why I use a pointer to atm_cb_lambda_t is to save memory overhead. It is not possible to add atm_cb_lambda_t to the anonymous union, because atm_cb_lambda_t is a non-trivial class.
However, it may be better to add a member of type atm_cb_lambda_t to the atm_connector class to avoid some subtle problems with null-pointer dereferences etc.?!

@tinkerspy
Copy link
Owner

Hi Euphi,

That looks interesting. I'm not sure I quite grasp the consequences yet. Perhaps you could create a fork and work it out? I'm open to input if we could improve things.

Alternatively as a quick solution you could perhaps just 'fork' the Atm_timer machine into your own variety that holds a pointer to your object class.

Rgdz,
Tinkerspy

@euphi
Copy link
Collaborator Author

euphi commented Jul 1, 2016

I really like to fork and implement this, but I fear I don't have the time to do so within the next months.

For now, I will use a simple solution (probably just use millis() in loop().

Automaton still is really, really useful for me for the core functionality of my current projects (up to now a project for irrigation control, but till autumn I also need to finish a Heating control / thermostat).

@tinkerspy
Copy link
Owner

Hi Ian,

It's good to hear you find Automaton so useful, hope you'll get round to forking some time. Another way to combine Automaton code with regular code is to use the delay statement. Pauses your process but keeps Automaton running. Perhaps that can be useful.

Rgdz,
Tinkerspy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants