xtimer: Skip ahead when target time has already passed

sys/include/xtimer.h

@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2015 Kaspar Schleiser <kaspar@schleiser.de>
- * Copyright (C) 2016 Eistec AB
+ * Copyright (C) 2016-2017 Eistec AB
  *
  * This file is subject to the terms and conditions of the GNU Lesser
  * General Public License v2.1. See the file LICENSE in the top level
@@ -71,8 +71,7 @@ typedef struct xtimer {
     struct xtimer *next;         /**< reference to next timer in timer lists */
     uint32_t target;             /**< lower 32bit absolute target time */
     uint32_t long_target;        /**< upper 32bit absolute target time */
-    xtimer_callback_t callback;  /**< callback function to call when timer
-                                     expires */
+    xtimer_callback_t callback;  /**< callback function to call when timer expires */
     void *arg;                   /**< argument to pass to callback function */
 } xtimer_t;
 
@@ -207,14 +206,46 @@ static inline void xtimer_spin(xtimer_ticks32_t ticks);
  */
 static inline void xtimer_periodic_wakeup(xtimer_ticks32_t *last_wakeup, uint32_t period);
 
+/**
+ * @brief Send a message at absolute time (@p last_wakeup + @p period)
+ *
+ * Functionally similar to xtimer_set_msg, but the target time is computed in
+ * the same way as for xtimer_periodic_wakeup.
+ *
+ * This function can be used to create periodic messages. This is especially
+ * useful if a thread has an event loop for handling asynchronous events, but
+ * some events need to occur periodically on a schedule. Using
+ * xtimer_periodic_wakeup in these kinds of applications would cause the
+ * asynchronous events to be delayed until the next wakeup, instead of processed
+ * immediately.
+ *
+ * @attention The message struct pointed to by @p msg will not be copied,
+ * i.e. it needs to point to valid memory until the message has been delivered.
+ *
+ * If the result of (@p last_wakeup + @p period) would be in the past, the function
+ * sets @p last_wakeup to @p last_wakeup + @p period and sends the message
+ * immediately.
+ *
+ * @param[in] timer         timer struct to work with.
+ *                          Its xtimer_t::target and xtimer_t::long_target
+ *                          fields need to be initialized with 0 on first use.
+ * @param[in] last_wakeup   base time stamp for the wakeup
+ * @param[in] period        time in microseconds that will be added to last_wakeup
+ * @param[in] msg           message that will be sent
+ * @param[in] target_pid    destination thread PID
+ */
+static inline void xtimer_periodic_msg(xtimer_t *timer,
+                                       xtimer_ticks32_t *last_wakeup, uint32_t period,
+                                       msg_t *msg, kernel_pid_t target_pid);
+
 /**
  * @brief Set a timer that sends a message
  *
- * This function sets a timer that will send a message @p offset ticks
+ * This function sets a timer that will send a message @p offset microseconds
  * from now.
  *
- * The mesage struct specified by msg parameter will not be copied, e.g., it
- * needs to point to valid memory until the message has been delivered.
+ * @attention The message struct pointed to by @p msg will not be copied,
+ * i.e. it needs to point to valid memory until the message has been delivered.
  *
  * @param[in] timer         timer struct to work with.
  *                          Its xtimer_t::target and xtimer_t::long_target
@@ -231,8 +262,8 @@ static inline void xtimer_set_msg(xtimer_t *timer, uint32_t offset, msg_t *msg,
  * This function sets a timer that will send a message @p offset microseconds
  * from now.
  *
- * The mesage struct specified by msg parameter will not be copied, e.g., it
- * needs to point to valid memory until the message has been delivered.
+ * @attention The message struct pointed to by @p msg will not be copied,
+ * i.e. it needs to point to valid memory until the message has been delivered.
  *
  * @param[in] timer         timer struct to work with.
  *                          Its xtimer_t::target and xtimer_t::long_target

sys/include/xtimer/implementation.h

@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2015 Kaspar Schleiser <kaspar@schleiser.de>
- * Copyright (C) 2016 Eistec AB
+ * Copyright (C) 2016-2017 Eistec AB
  *
  * This file is subject to the terms and conditions of the GNU Lesser
  * General Public License v2.1. See the file LICENSE in the top level
@@ -66,6 +66,7 @@ int _xtimer_set_absolute(xtimer_t *timer, uint32_t target);
 void _xtimer_set64(xtimer_t *timer, uint32_t offset, uint32_t long_offset);
 void _xtimer_set(xtimer_t *timer, uint32_t offset);
 void _xtimer_periodic_wakeup(uint32_t *last_wakeup, uint32_t period);
+void _xtimer_periodic_msg(xtimer_t *timer, uint32_t *last_wakeup, uint32_t period, msg_t *msg, kernel_pid_t target_pid);
 void _xtimer_set_msg(xtimer_t *timer, uint32_t offset, msg_t *msg, kernel_pid_t target_pid);
 void _xtimer_set_msg64(xtimer_t *timer, uint64_t offset, msg_t *msg, kernel_pid_t target_pid);
 void _xtimer_set_wakeup(xtimer_t *timer, uint32_t offset, kernel_pid_t pid);
@@ -193,6 +194,13 @@ static inline void xtimer_periodic_wakeup(xtimer_ticks32_t *last_wakeup, uint32_
     _xtimer_periodic_wakeup(&last_wakeup->ticks32, _xtimer_ticks_from_usec(period));
 }
 
+static inline void xtimer_periodic_msg(xtimer_t *timer,
+                                       xtimer_ticks32_t *last_wakeup, uint32_t period,
+                                       msg_t *msg, kernel_pid_t target_pid)
+{
+    _xtimer_periodic_msg(timer, &last_wakeup->ticks32, _xtimer_ticks_from_usec(period), msg, target_pid);
+}
+
 static inline void xtimer_set_msg(xtimer_t *timer, uint32_t offset, msg_t *msg, kernel_pid_t target_pid)
 {
     _xtimer_set_msg(timer, _xtimer_ticks_from_usec(offset), msg, target_pid);

sys/xtimer/xtimer.c

@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2015 Kaspar Schleiser <kaspar@schleiser.de>
- * Copyright (C) 2016 Eistec AB
+ * Copyright (C) 2016-2017 Eistec AB
  *
  * This file is subject to the terms and conditions of the GNU Lesser
  * General Public License v2.1. See the file LICENSE in the top level
@@ -58,82 +58,109 @@ void _xtimer_tsleep(uint32_t offset, uint32_t long_offset)
     }
 
     xtimer_t timer;
-    mutex_t mutex = MUTEX_INIT;
+    mutex_t mutex = MUTEX_INIT_LOCKED;
 
     timer.callback = _callback_unlock_mutex;
     timer.arg = (void*) &mutex;
     timer.target = timer.long_target = 0;
 
-    mutex_lock(&mutex);
     _xtimer_set64(&timer, offset, long_offset);
     mutex_lock(&mutex);
 }
 
-void _xtimer_periodic_wakeup(uint32_t *last_wakeup, uint32_t period) {
-    xtimer_t timer;
-    mutex_t mutex = MUTEX_INIT;
-
-    timer.callback = _callback_unlock_mutex;
-    timer.arg = (void*) &mutex;
-
+void _xtimer_periodic(xtimer_t *timer, uint32_t *last_wakeup, uint32_t period)
+{
     uint32_t target = (*last_wakeup) + period;
     uint32_t now = _xtimer_now();
-    /* make sure we're not setting a value in the past */
-    if (now < (*last_wakeup)) {
-        /* base timer overflowed between last_wakeup and now */
-        if (!((now < target) && (target < (*last_wakeup)))) {
-            /* target time has already passed */
-            goto out;
+    do {
+        /* make sure we're not setting a value in the past */
+        if (now < (*last_wakeup)) {
+            /* base timer overflowed between last_wakeup and now */
+            if (!((now < target) && (target < (*last_wakeup)))) {
+                /* target time has already passed */
+                timer->callback(timer->arg);
+                /* Try to skip ahead until we are back on schedule */
+                uint32_t catchup = period;
+                if (target < now) {
+                    while((target < (target + catchup)) && ((target + catchup) <= now)) {
+                        catchup += period;
+                    }
+                }
+                else {
+                    while(!((now < (target + catchup)) && ((target + catchup) < target))) {
+                        catchup += period;
+                    }
+                }
+                target += catchup - period;
+                DEBUG("cu: %" PRIx32 "\n", catchup);
+                break;
+            }
         }
-    }
-    else {
-        /* base timer did not overflow */
-        if ((((*last_wakeup) <= target) && (target <= now))) {
-            /* target time has already passed */
-            goto out;
+        else {
+            /* base timer did not overflow */
+            if ((((*last_wakeup) <= target) && (target <= now))) {
+                /* target time has already passed */
+                timer->callback(timer->arg);
+                /* Try to skip ahead until we are back on schedule */
+                uint32_t catchup = period;
+                while((target < (target + catchup)) && ((target + catchup) <= now)) {
+                    catchup += period;
+                }
+                target += catchup - period;
+                DEBUG("CU: %" PRIx32 "\n", catchup);
+                break;
+            }
         }
-    }
 
-    /*
-     * For large offsets, set an absolute target time.
-     * As that might cause an underflow, for small offsets, set a relative
-     * target time.
-     * For very small offsets, spin.
-     */
-    /*
-     * Note: last_wakeup _must never_ specify a time in the future after
-     * _xtimer_periodic_sleep returns.
-     * If this happens, last_wakeup may specify a time in the future when the
-     * next call to _xtimer_periodic_sleep is made, which in turn will trigger
-     * the overflow logic above and make the next timer fire too early, causing
-     * last_wakeup to point even further into the future, leading to a chain
-     * reaction.
-     *
-     * tl;dr Don't return too early!
-     */
-    uint32_t offset = target - now;
-    DEBUG("xps, now: %9" PRIu32 ", tgt: %9" PRIu32 ", off: %9" PRIu32 "\n", now, target, offset);
-    if (offset < XTIMER_PERIODIC_SPIN) {
-        _xtimer_spin(offset);
-    }
-    else {
-        if (offset < XTIMER_PERIODIC_RELATIVE) {
-            /* NB: This will overshoot the target by the amount of time it took
-             * to get here from the beginning of xtimer_periodic_wakeup()
-             *
-             * Since interrupts are normally enabled inside this function, this time may
-             * be undeterministic. */
-            target = _xtimer_now() + offset;
+        /*
+         * For large offsets, set an absolute target time.
+         * As that might cause an underflow, for small offsets, set a relative
+         * target time.
+         * For very small offsets, spin.
+         */
+        /*
+         * Note: last_wakeup _must never_ specify a time in the future after
+         * _xtimer_periodic_sleep returns.
+         * If this happens, last_wakeup may specify a time in the future when the
+         * next call to _xtimer_periodic_sleep is made, which in turn will trigger
+         * the overflow logic above and make the next timer fire too early, causing
+         * last_wakeup to point even further into the future, leading to a chain
+         * reaction.
+         *
+         * tl;dr Don't return too early!
+         */
+        uint32_t offset = target - now;
+        DEBUG("xps, now: %9" PRIu32 ", tgt: %9" PRIu32 ", off: %9" PRIu32 "\n", now, target, offset);
+        if (offset < XTIMER_PERIODIC_SPIN) {
+            _xtimer_spin(offset);
+            timer->callback(timer->arg);
         }
-        mutex_lock(&mutex);
-        DEBUG("xps, abs: %" PRIu32 "\n", target);
-        _xtimer_set_absolute(&timer, target);
-        mutex_lock(&mutex);
-    }
-out:
+        else {
+            if (offset < XTIMER_PERIODIC_RELATIVE) {
+                /* NB: This will overshoot the target by the amount of time it took
+                 * to get here from the beginning of xtimer_periodic_wakeup()
+                 *
+                 * Since interrupts are normally enabled inside this function, this time may
+                 * be nondeterministic. */
+                target = _xtimer_now() + offset;
+            }
+            DEBUG("xps, abs: %" PRIu32 "\n", target);
+            _xtimer_set_absolute(timer, target);
+        }
+    } while (0);
     *last_wakeup = target;
 }
 
+void _xtimer_periodic_wakeup(uint32_t *last_wakeup, uint32_t period) {
+    xtimer_t timer;
+    mutex_t mutex = MUTEX_INIT_LOCKED;
+
+    timer.callback = _callback_unlock_mutex;
+    timer.arg = (void*) &mutex;
+    _xtimer_periodic(&timer, last_wakeup, period);
+    mutex_lock(&mutex);
+}
+
 static void _callback_msg(void* arg)
 {
     msg_t *msg = (msg_t*)arg;
@@ -149,6 +176,12 @@ static inline void _setup_msg(xtimer_t *timer, msg_t *msg, kernel_pid_t target_p
     msg->sender_pid = target_pid;
 }
 
+void _xtimer_periodic_msg(xtimer_t *timer, uint32_t *last_wakeup, uint32_t period, msg_t *msg, kernel_pid_t target_pid)
+{
+    _setup_msg(timer, msg, target_pid);
+    _xtimer_periodic(timer, last_wakeup, period);
+}
+
 void _xtimer_set_msg(xtimer_t *timer, uint32_t offset, msg_t *msg, kernel_pid_t target_pid)
 {
     _setup_msg(timer, msg, target_pid);

sys/xtimer/xtimer_core.c

@@ -176,7 +176,9 @@ int _xtimer_set_absolute(xtimer_t *timer, uint32_t target)
     DEBUG("timer_set_absolute(): now=%" PRIu32 " target=%" PRIu32 "\n", now, target);
 
     timer->next = NULL;
-    if ((target >= now) && ((target - XTIMER_BACKOFF) < now)) {
+    /* The (target - now) difference works across the long tick rollover, there
+     * is no need to check for (target > now) first. */
+    if ((target - now) < XTIMER_BACKOFF) {
         /* backoff */
         xtimer_spin_until(target + XTIMER_BACKOFF);
         _shoot(timer);

tests/xtimer_periodic_msg/Makefile

@@ -0,0 +1,6 @@
+APPLICATION = xtimer_periodic_msg
+include ../Makefile.tests_common
+
+USEMODULE += xtimer
+
+include $(RIOTBASE)/Makefile.include

tests/xtimer_periodic_msg/main.c

@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2015 Kaspar Schleiser <kaspar@schleiser.de>
+ * Copyright (C) 2017 Eistec AB
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser
+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @ingroup     tests
+ * @{
+ *
+ * @file
+ * @brief       timer test application
+ *
+ * @author      Kaspar Schleiser <kaspar@schleiser.de>
+ * @author      Joakim Nohlgård <joakim.nohlgard@eistec.se>
+ *
+ * @}
+ */
+
+#include <stdio.h>
+#include "thread.h"
+#include "msg.h"
+#include "xtimer.h"
+
+#ifndef NUMOF
+#define NUMOF   (256)
+#endif
+
+int32_t res[NUMOF];
+
+int main(void)
+{
+    puts("xtimer_periodic_msg test application.\n");
+    msg_t msg_q[1];
+    msg_init_queue(msg_q, 1);
+
+    uint32_t interval = NUMOF;
+    int32_t max_diff = INT32_MIN;
+    int32_t min_diff = INT32_MAX;
+
+    for (int i = 0; i < NUMOF; i++) {
+        xtimer_ticks32_t now = xtimer_now();
+        printf("Testing interval %" PRIu32 "... (now=%" PRIu32 ")\n", interval, xtimer_usec_from_ticks(now));
+        xtimer_ticks32_t last_wakeup = xtimer_now();
+        xtimer_ticks32_t before = last_wakeup;
+        xtimer_t timer = { .target = 0, .long_target = 0 };
+        msg_t msg_out = { .type = 0xffff - interval, .content.value = interval };
+        xtimer_periodic_msg(&timer, &last_wakeup, interval, &msg_out, thread_getpid());
+        msg_t msg_in;
+        msg_receive(&msg_in);
+        now = xtimer_now();
+        assert(msg_in.type == msg_out.type);
+        assert(msg_in.content.value == msg_out.content.value);
+        res[i] = (xtimer_usec_from_ticks(now) - xtimer_usec_from_ticks(before)) - interval;
+        interval -= 1;
+    }
+
+    for (int i = 0; i < NUMOF; i++) {
+        printf("%4d diff=%" PRIi32 "\n", NUMOF - i, res[i]);
+
+        if (res[i] > max_diff) {
+            max_diff = res[i];
+        }
+        if (res[i] < min_diff) {
+            min_diff = res[i];
+        }
+    }
+    printf("\nMin/max error: %" PRId32 "/%" PRId32 "\n", min_diff, max_diff);
+
+    if (min_diff < -1000 || max_diff > 1000) {
+        puts("too large difference.\n");
+        puts("Test Failed.\n");
+        return 1;
+    }
+
+    printf("\nTest complete.\n");
+    return 0;
+}