xtimer: Add reference point for _xtimer_set_absolute

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
@@ -62,10 +62,11 @@ static inline uint32_t _xtimer_lltimer_mask(uint32_t val)
  * @internal
  */
 uint64_t _xtimer_now64(void);
-int _xtimer_set_absolute(xtimer_t *timer, uint32_t target);
+void _xtimer_set_absolute(xtimer_t *timer, uint32_t target, uint32_t now);
 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,88 @@ 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);
+                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);
+                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, now);
+        }
+    } 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 +155,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

@@ -142,8 +142,9 @@ void _xtimer_set(xtimer_t *timer, uint32_t offset)
         _shoot(timer);
     }
     else {
-        uint32_t target = _xtimer_now() + offset;
-        _xtimer_set_absolute(timer, target);
+        uint32_t now = _xtimer_now();
+        uint32_t target = now + offset;
+        _xtimer_set_absolute(timer, target, now);
     }
 }
 
@@ -168,19 +169,18 @@ static inline void _lltimer_set(uint32_t target)
     timer_set_absolute(XTIMER_DEV, XTIMER_CHAN, _xtimer_lltimer_mask(target));
 }
 
-int _xtimer_set_absolute(xtimer_t *timer, uint32_t target)
+void _xtimer_set_absolute(xtimer_t *timer, uint32_t target, uint32_t now)
 {
-    uint32_t now = _xtimer_now();
-    int res = 0;
-
     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);
-        return 0;
+        return;
     }
 
     unsigned state = irq_disable();
@@ -215,8 +215,6 @@ int _xtimer_set_absolute(xtimer_t *timer, uint32_t target)
     }
 
     irq_restore(state);
-
-    return res;
 }
 
 static void _add_timer_to_list(xtimer_t **list_head, xtimer_t *timer)

tests/xtimer_periodic_msg/Makefile

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