ARMmbed / mbed-events

DEPRECATED! This project has moved to mbed-os
https://github.com/ARMmbed/mbed-os/tree/master/events
Apache License 2.0
11 stars 6 forks source link

Add the Event class for fine-grain control of event dispatch #15

Closed geky closed 7 years ago

geky commented 8 years ago

For a more fine-grain control of event dispatch, the Event class can be manually instantiated and configured. An Event represents an event as a C++ style function object and can be directly passed to other APIs that expect a callback.

From the updated readme:

// Creates an event bound to the specified event queue
EventQueue queue;
Event<void()> event(&queue, doit);

// The event can be manually configured for special timing requirements
// specified in milliseconds
event.delay(10);
event.period(10000);

// Posted events are dispatched in the constext of the queue's
// dispatch function
queue.dispatch();

// Events can also pass arguments to the underlying callback when both
// initially constructed and posted.
Event<void(int, int)> event(&queue, printf, "recieved %d and %d\n");

// Events can be posted multiple times and enqueue gracefully until
// the dispatch function is called.
event.post(1, 2);
event.post(3, 4);
event.post(5, 6);

queue.dispatch();

cc @pan- thoughts?

pan- commented 8 years ago

Maybe I misunderstand something here but I see two different concepts in the same class:

Otherwise it is an interesting enhancement.

geky commented 8 years ago

Yep, I was hoping to kill two birds with one stone.

The goal is to encourage users to move as much code out of interrupt context as possible. Unfortunately, default behaviour is interrupt context, which makes it easy to suddenly find yourself in an interrupt. But we can encourage better behaviour through syntactic sugar.

Currently (with this patch), this is the easiest method to move events out of interrupt context:

Event<> event(queue, func);
ticker.attach(&event, &Event<>::call, 10);

However, this forces the use of method pointers, which can be intimidating to new users. With the operator() overloaded, I would hope to see some syntactic sugar similar to the following emerge in the mbed codebase, allowing deferring from interrupts in a generic manner:

Event<> event(queue, func);
ticker.attach(event, 10);

If we want to avoid this syntax for now, I can split the function-like operations out into a different pr, which can stagnate until such overloads are added to mbed-os.

If we go this route, it would be beneficial to add a post_void sort of member function that works around the return type as you mention. Allowing this as a temporary pattern:

Event<> event(queue, func);
ticker.attach(&event, &Event<>::post_void, 10);
pan- commented 8 years ago

What about:

operator Callback<void(A0,...)>() { 
  return Callback<void(A0,...)>(this, &Event<A0,...>::post_void);
}

It would still be possible to write:

Event<> event(queue, func);
ticker.attach(event, 10);

but not:

Event<> event(queue, func);
event();

What do you think ?

geky commented 8 years ago

Hmm, that is a good idea.

My only concern is a running concern I've had with the clarity of ownership. It would be very easy for even an experienced user to take your example and expect the following to work:

EventQueue queue;

void setup() {
    Event<> event(&queue, func);
    ticker.attach(event, 10);
}

int main() {
    setup();
    queue.dispatch();
}

Looking into related concepts, it looks like the operator& is actually overloadable.

As an alternative, what are you thoughts on instead adding a private EventPtr sort of proxy object that is returned from operator& and supports implicit casts to Callback and Event*? The above could be rewritten as follows, making the memory ownership a bit more clear:

EventQueue queue;

void setup() {
    Event<> event(&queue, func);
    ticker.attach(&event, 10);
}

int main() {
    setup();
    queue.dispatch();
}
pan- commented 8 years ago

My only concern is a running concern I've had with the clarity of ownership. It would be very easy for even an experienced user to take your example and expect the following to work.

100% agreed overloading operator& is much better in that case.

geky commented 7 years ago

@pan-, I've removed the operator() and related functions, with the intention of considering compatibility with the Callback class in a different pr.

Any concerns with the updated pr?