sys: add evtimer WIP #5999

kaspar030 · 2016-10-26T20:14:02Z

I have this timer module lying around. It's design goals are:

use less state per timer
don't waste memory for practically useless precision

I ended up with using a single pointer and a 32bit value per timer, making it possible to have a timers that only use 8 bytes on a 32bit platform.

miri64 · 2016-10-26T20:57:09Z

@kaspar030 what about kaspar030#19?

miri64 · 2016-10-26T20:57:30Z

Sorry wrong link. Corrected in edit.

miri64

Some changes I already introduced in kaspar030#19

miri64 · 2016-10-27T09:45:12Z

sys/evtimer/evtimer.c

+    DEBUG("evtimer_add(): adding event with offset %"PRIu32"\n", event->offset);
+
+    _update_head_offset(evtimer);
+    _add_event_to_list((evtimer_event_t*)&evtimer->events, event);


Why the cast and why the address operator here?

miri64 · 2016-10-27T09:46:54Z

sys/evtimer/evtimer.c

+    evtimer->events = NULL;
+}
+
+#if ENABLE_DEBUG == 1


This requires any external caller to edit this file internally (which also causes printing of the messages above). The compile should optimize it away if it isn't called. Alternatively, put this function in another file (if you don't trust the compiler optimization).

miri64 · 2016-10-27T09:48:14Z

sys/include/div.h

+ * @brief Integer divide val by 125
+ *
+ * This function can be used to convert uint64_t microsecond times (or
+ * intervals) to miliseconds and store them in uint32_t variables, with up to


milliseconds

miri64 · 2016-10-27T09:49:59Z

sys/evtimer/evtimer.c

+
+    evtimer_msg_event_t *mevent;
+    while ((mevent = (evtimer_msg_event_t *)_get_next(evtimer))) {
+        msg_send_int(&mevent->msg, mevent->msg.sender_pid);


Send to sender_pid? Also: isn't this value usually uninitialized by the user?

Easy fix: see here

zhuoshuguo · 2016-11-04T12:02:12Z

By playing with it, I think I found a possible bug:
I simply couldn't add the first event timeout:

    evtimer_t evtimer; 
    evtimer_msg_event_t msg_event;

    ......
    msg_event.event.next= NULL;
    msg_event.event.offset = 2000;
    msg_event.msg.type = GNRC_EVTIMEOUT_TYPE;
    msg_event.msg.content.ptr = &msg_event;
    msg_event.msg.sender_pid = gnrc_netdev2->pid;

    evtimer_init(&evtimer, evtimer_msg_handler);
    evtimer_add(&evtimer, (evtimer_event_t *)&msg_event);

Then, I checked and found that _add_event_to_list() (also _del_event_from_list()) seems to have malfunction, please see below and fix

zhuoshuguo · 2016-11-04T12:10:36Z

sys/evtimer/evtimer.c

+{
+    uint32_t delta_sum = 0;
+
+    while (list->next) {


Maybe list-next is wrongly placed here.

when firstly add an event, i.e., the first element of evtimer->events, which is list here, is NULL. But list->next may not be NULL, and the following execution may lead to chaos...

In my test, when adding the first event, it goes into the while loop, which I think it is not as we expected. :-)

zhuoshuguo · 2016-11-04T12:13:24Z

sys/evtimer/evtimer.c

+
+static void _del_event_from_list(evtimer_event_t *list, evtimer_event_t *event)
+{
+    while (list->next) {


Similar issue here.

zhuoshuguo · 2016-11-04T17:00:23Z

sys/include/evtimer.h

+struct evtimer_event {
+    evtimer_event_t *next;
+    uint32_t offset;
+};


Do you think we can add a expired bool value into the evtimer_event_t structure, which I think should be commonly used by users? :-)
@miri64 If not, I think I will add it (expired) into my gnrc_mac timeout module which will probably be the wrapper of evtimer module.

@zhuoshuguo it's more RAM saving to implement expired as a function if you actually need it (though usually it is more advisable to just react via callback to an expired event ;-)): You could check the event list if the event is still contained.

@miri64 I think you are right~ 😄 Thanks!

zhuoshuguo · 2016-11-06T11:22:49Z

Add checking event running/scheduled functionality (see add_check_event_sched) which I think also enables checking event expired. Should be at least useful for me. 😄

kaspar030 · 2016-11-08T15:07:23Z

Then, I checked and found that _add_event_to_list() (also _del_event_from_list()) seems to have malfunction, please see below and fix

Sorry, what was a too-quickly fix in the last commit. Both functions' logics required to get a pointer to pointer to the first list element in order to work, which I diffed out accidentaly. I've moved the pointer logic into the functions now.

zhuoshuguo · 2016-11-08T18:24:21Z

@kaspar030 Sorry for my impetuous action..

kaspar030 · 2016-11-08T18:53:42Z

@kaspar030 Sorry for my impetuous action..

Don't be sorry, thanks for fixing my mess!

miri64 · 2017-01-04T15:27:42Z

@kaspar030 can you provide [edit]documentation[/edit] (and maybe even tests)? I need to use this soon-ish for NDP. Let's merge this with #6000 later on (especially since we need to overhaul IPC to use thread flags for my use-case anyways to make it useful for my context anyways :-/).

miri64 · 2017-01-06T10:12:55Z

sys/include/div.h

+ * @return      (val / 125)
+ */
+static inline uint32_t div_u64_by_125(uint64_t val)
+{


Since when do we use two spaces?

miri64 · 2017-01-06T13:01:08Z

Also, please consider kaspar030#22. This streamlines the message-based API a little bit.

miri64 · 2017-01-10T13:43:34Z

Also doesn't build atm... Will see, if I can provide a fix.

miri64 · 2017-01-11T10:37:02Z

Ah, the changes in #5608 were the problem.

miri64 · 2017-01-24T15:19:14Z

I guess we postpone then.

zhuoshuguo · 2017-01-25T11:03:36Z

I will be also thankful if this PR will get merged soon, since my gnrc_mac's timeout module is also based on evtimer. 😄

emmanuelsearch · 2017-02-13T14:53:02Z

@kaspar030 any news on this? What should be done in #6554 at this point concerning this PR?
Is it still WIP?
If so shall #6554 go forward without it for now?

zhuoshuguo · 2017-02-13T18:54:03Z

If this PR is still WIP, I suggest we push #6554 forward without it (evtimer).

While when this PR get merged someday in the future, I will push #5949 to merge, and reflect evtimer in #6554.

miri64 · 2017-03-16T08:27:16Z

PING?

miri64 · 2017-03-20T14:34:39Z

Adopted in #6767

kaspar030 · 2017-03-21T10:46:41Z

Closed in favour of 6767. Thanks for taking over!

miri64 mentioned this pull request Oct 26, 2016

sys: add asynchronous event system #6000

Merged

miri64 requested changes Oct 27, 2016

View reviewed changes

miri64 reviewed Oct 27, 2016

View reviewed changes

miri64 mentioned this pull request Oct 31, 2016

gnrc_mac: add timeout module. #5949

Merged

zhuoshuguo reviewed Nov 4, 2016

View reviewed changes

miri64 self-assigned this Nov 10, 2016

miri64 added this to the Release 2017.01 milestone Nov 10, 2016

miri64 reviewed Jan 6, 2017

View reviewed changes

sys/include/div.h

* @return (val / 125)

*/

static inline uint32_t div_u64_by_125(uint64_t val)

{

Copy link

Member

miri64 Jan 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since when do we use two spaces?

miri64 modified the milestones: Release 2017.04, Release 2017.01 Jan 24, 2017

zhuoshuguo mentioned this pull request Feb 5, 2017

LWMAC: A simple duty cycling 802.15.4 MAC protocol (2nd try). #6554

Merged

kaspar030 and others added 5 commits March 20, 2017 15:32

sys: div: add div_u64_by_125()

8c0249d

evtimer: initial import

790caf4

evtimer: add msg helper functions

d26eb79

evtimer: add and fix doc

6157f92

tests: provide tests for evtimer

649f63e

miri64 mentioned this pull request Mar 20, 2017

evtimer: initial import #6767

Merged

miri64 and others added 6 commits March 20, 2017 16:07

fixup! evtimer: initial import

da705cb

fixup! evtimer: add msg helper functions

08272fe

fixup! tests: provide tests for evtimer

351a367

squash: evtimer: add callback ptr to evtimer_t

9cb9b20

squash: evtimer_msg: adapt to callback ptr in evtimer_t

4de6547

squash: evtimer: explain hack

2af1b5e

kaspar030 force-pushed the evtimer branch from fc3bc0c to 2af1b5e Compare March 21, 2017 09:46

kaspar030 closed this Mar 21, 2017

sys: add evtimer WIP #5999

sys: add evtimer WIP #5999

Uh oh!

Conversation

kaspar030 commented Oct 26, 2016

Uh oh!

miri64 commented Oct 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miri64 commented Oct 26, 2016

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuoshuguo commented Nov 4, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhuoshuguo Nov 4, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuoshuguo commented Nov 6, 2016

Uh oh!

kaspar030 commented Nov 8, 2016

Uh oh!

zhuoshuguo commented Nov 8, 2016

Uh oh!

kaspar030 commented Nov 8, 2016 via email

Uh oh!

miri64 commented Jan 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miri64 commented Jan 6, 2017

Uh oh!

miri64 commented Jan 10, 2017

Uh oh!

miri64 commented Jan 11, 2017

Uh oh!

miri64 commented Jan 24, 2017

Uh oh!

zhuoshuguo commented Jan 25, 2017

Uh oh!

emmanuelsearch commented Feb 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhuoshuguo commented Feb 13, 2017

Uh oh!

miri64 commented Mar 16, 2017

Uh oh!

miri64 commented Mar 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaspar030 commented Mar 21, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

miri64 commented Oct 26, 2016 •

edited

Loading

zhuoshuguo commented Nov 4, 2016 •

edited

Loading

zhuoshuguo Nov 4, 2016 •

edited

Loading

miri64 commented Jan 4, 2017 •

edited

Loading

emmanuelsearch commented Feb 13, 2017 •

edited

Loading

miri64 commented Mar 20, 2017 •

edited

Loading