Improve and debug Xtimer

sys/xtimer/xtimer.c

@@ -73,28 +73,43 @@ void _xtimer_tsleep(uint32_t offset, uint32_t long_offset)
 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;
+    uint32_t mult;
 
     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;
+        /* last_wakeup < target, now overflowed but target not, target passed.
+         * now < last_wakeup < target */
+        /* target <= now , both overflowed, target passed.
+         * target < now < last_wakeup */
+        if ( (*last_wakeup < target) || (target <= now) ) {
+            /* now - target, will always be the difference. (modulo power of two) */
+            mult = ((uint32_t)(now - target)) / period;
+            /* Skip missed targets */
+            *last_wakeup = target + (mult * period);
+            return;
         }
     }
     else {
-        /* base timer did not overflow */
-        if ((((*last_wakeup) <= target) && (target <= now))) {
-            /* target time has already passed */
-            goto out;
+        /* last_wakeup < now, now did not overflow
+         * target <= now AND target did not overflow, target passed
+         * last_wakeup <= target <= now */
+        /* The special case (*last_wakeup) = target happens when a period smaller
+         * then the minimum resolution of the xtimer_hz is requested. */
+        if ( (target <= now) && ((*last_wakeup) <= target) ) {
+            /* now - target, will always be the difference. (modulo power of two) */
+            mult = ((uint32_t)(now - target))/ period;
+            /* Skip missed targets */
+            *last_wakeup = target + (mult * period);
+            return;
         }
     }
 
+    timer.callback = _callback_unlock_mutex;
+    timer.arg = (void*) &mutex;
+
     /*
      * For large offsets, set an absolute target time.
      * As that might cause an underflow, for small offsets, set a relative
@@ -113,9 +128,13 @@ void _xtimer_periodic_wakeup(uint32_t *last_wakeup, uint32_t period) {
      * 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);
+    DEBUG("xps, now: %9" PRIu32 ", tgt: %9" PRIu32 ", lst_wkp: %9" PRIu32 ", off: %9" PRIu32 "\n",
+           now, target, *last_wakeup, offset);
+    if (offset < XTIMER_PERIODIC_SPIN ) {
+        /* for short times skip */
+        if(offset != 0) {
+            _xtimer_spin(offset);
+        }
     }
     else {
         if (offset < XTIMER_PERIODIC_RELATIVE) {
@@ -131,7 +150,7 @@ void _xtimer_periodic_wakeup(uint32_t *last_wakeup, uint32_t period) {
         _xtimer_set_absolute(&timer, target);
         mutex_lock(&mutex);
     }
-out:
+
     *last_wakeup = target;
 }
 

sys/xtimer/xtimer_core.c

@@ -320,20 +320,23 @@ void xtimer_remove(xtimer_t *timer)
     irq_restore(state);
 }
 
+/* target < reference < now */
 static uint32_t _time_left(uint32_t target, uint32_t reference)
 {
     uint32_t now = _xtimer_lltimer_now();
 
-    if (now < reference) {
-        return 0;
-    }
-
-    if (target > now) {
-        return target - now;
+    /* `target` is in next period
+     * AND `now` is at the end ot this period 
+     * OR `now` still smaller than target */
+    if( (target < XTIMER_OVERHEAD)
+             && ((reference < now) || (now < target))) {
+        return _xtimer_lltimer_mask(target-now);
     }
-    else {
-        return 0;
+    /* OR Target is still bigger than now */
+    else if ( (reference < now) && (now < target) ){
+                return target-now;
     }
+    return 0;
 }
 
 static inline int _this_high_period(uint32_t target)
@@ -484,17 +487,13 @@ static void _timer_callback(void)
         _next_period();
 
         reference = 0;
-
-        /* make sure the timer counter also arrived
-         * in the next timer period */
+        /* make sure the timer counter also arrived in the next timer period */
         while (_xtimer_lltimer_now() == _xtimer_lltimer_mask(0xFFFFFFFF)) {}
     }
     else {
-        /* we ended up in _timer_callback and there is
-         * a timer waiting.
-         */
-        /* set our period reference to the current time. */
-        reference = _xtimer_lltimer_now();
+        /* we ended up in _timer_callback and there is a timer waiting.
+         * set our period reference to the expected target. */
+        reference = _xtimer_lltimer_mask(timer_list_head->target - XTIMER_OVERHEAD);
     }
 
 overflow:
@@ -517,26 +516,9 @@ static void _timer_callback(void)
         _shoot(timer);
     }
 
-    /* possibly executing all callbacks took enough
-     * time to overflow.  In that case we advance to
-     * next timer period and check again for expired
-     * timers.*/
-    /* check if the end of this period is very soon */
-    uint32_t now = _xtimer_lltimer_now() + XTIMER_ISR_BACKOFF;
-    if (now < reference) {
-        DEBUG("_timer_callback: overflowed while executing callbacks. %i\n",
-              timer_list_head != NULL);
-        _next_period();
-        /* wait till overflow */
-        while( reference < _xtimer_lltimer_now()){}
-        reference = 0;
-        goto overflow;
-    }
-
     if (timer_list_head) {
         /* schedule callback on next timer target time */
         next_target = timer_list_head->target - XTIMER_OVERHEAD;
-
         /* make sure we're not setting a time in the past */
         if (next_target < (_xtimer_now() + XTIMER_ISR_BACKOFF)) {
             goto overflow;
@@ -546,24 +528,23 @@ static void _timer_callback(void)
         /* there's no timer planned for this timer period */
         /* schedule callback on next overflow */
         next_target = _xtimer_lltimer_mask(0xFFFFFFFF);
-        uint32_t now = _xtimer_lltimer_now();
+    }
 
-        /* check for overflow again */
-        if (now < reference) {
-            _next_period();
-            reference = 0;
-            goto overflow;
-        }
-        else {
-            /* check if the end of this period is very soon */
-            if (_xtimer_lltimer_mask(now + XTIMER_ISR_BACKOFF) < now) {
-                /* spin until next period, then advance */
-                while (_xtimer_lltimer_now() >= now) {}
-                _next_period();
-                reference = 0;
-                goto overflow;
-            }
-        }
+    /* possibly executing all callbacks took enough
+     * time to overflow.  In that case we advance to
+     * next timer period and check again for expired
+     * timers.*/
+    /* check for overflow */
+    /* Or if the end of this period is very soon */
+    uint32_t now = _xtimer_lltimer_now();
+    if (now < reference || _xtimer_lltimer_mask(now + XTIMER_ISR_BACKOFF) < now) {
+        _next_period();
+        DEBUG("_timer_callback: overflowed while executing callbacks. %i\n",
+              timer_list_head != NULL);
+        /* wait till overflow */
+        while( reference < _xtimer_lltimer_now()){}
+        reference = 0;
+        goto overflow;
     }
 
     _in_handler = 0;

tests/xtimer_drift/main.c

@@ -163,7 +163,7 @@ int main(void)
 
     /* create and trigger worker thread */
     kernel_pid_t pid3 = thread_create(worker_stack, sizeof(worker_stack),
-                                      THREAD_PRIORITY_MAIN - 1,
+                                      THREAD_PRIORITY_MAIN - 2,
                                       THREAD_CREATE_STACKTEST,
                                       worker_thread, NULL, "worker");
 

tests/xtimer_hang/Makefile

@@ -17,7 +17,8 @@ TEST_ON_CI_WHITELIST += all
 # Port number should be found in port enum e.g in cpu/include/periph_cpu.h
 #FEATURES_REQUIRED += periph_gpio
 # Jiminy probing Pins
-#CFLAGS += -DWORKER_THREAD_PIN=GPIO_PIN\(5,7\)
-#CFLAGS += -DMAIN_THREAD_PIN=GPIO_PIN\(5,6\)
+#CFLAGS += -DWORKER_THREAD_PIN_1=GPIO_PIN\(5,7\)
+#CFLAGS += -DWORKER_THREAD_PIN_2=GPIO_PIN\(5,6\)
+#CFLAGS += -DMAIN_THREAD_PIN=GPIO_PIN\(5,5\)
 
 include $(RIOTBASE)/Makefile.include

tests/xtimer_hang/main.c

@@ -29,7 +29,7 @@
 #include "thread.h"
 #include "log.h"
 
-#if defined(MAIN_THREAD_PIN) || defined(WORKER_THREAD_PIN)
+#if defined(MAIN_THREAD_PIN) || defined(WORKER_THREAD_PIN_1) || defined(WORKER_THREAD_PIN_2)
 #include "board.h"
 #include "periph/gpio.h"
 #endif
@@ -38,20 +38,35 @@
 #define TEST_INTERVAL_MS        (100LU)
 #define TEST_TIMER_STACKSIZE    (THREAD_STACKSIZE_DEFAULT)
 
+#define TEST_SLEEP_TIME_1             (1000)
+#define TEST_SLEEP_TIME_2             (1100)
+
 char stack_timer1[TEST_TIMER_STACKSIZE];
 char stack_timer2[TEST_TIMER_STACKSIZE];
 
 void* timer_func(void* arg)
 {
-#if defined(WORKER_THREAD_PIN)
-    gpio_t worker_pin = WORKER_THREAD_PIN;
-    gpio_init(worker_pin, GPIO_OUT);
+#if defined(WORKER_THREAD_PIN_1)
+    gpio_t worker_pin_1 = WORKER_THREAD_PIN_1;
+    gpio_init(worker_pin_1, GPIO_OUT);
+#endif
+#if defined(WORKER_THREAD_PIN_2)
+    gpio_t worker_pin_2 = WORKER_THREAD_PIN_2;
+    gpio_init(worker_pin_2, GPIO_OUT);
 #endif
     LOG_DEBUG("run thread %" PRIkernel_pid "\n", thread_getpid());
+
+
     while(1) {
-#if defined(WORKER_THREAD_PIN)
-        gpio_set(worker_pin);
-        gpio_clear(worker_pin);
+#if defined(WORKER_THREAD_PIN_1) && defined(WORKER_THREAD_PIN_2)
+        if( *(uint32_t*)(arg) == TEST_SLEEP_TIME_1) {
+            gpio_set(worker_pin_1);
+            gpio_clear(worker_pin_1);
+        } 
+        else {
+            gpio_set(worker_pin_2);
+            gpio_clear(worker_pin_2);
+        }
 #endif
         xtimer_usleep(*(uint32_t *)(arg));
     }
@@ -65,8 +80,8 @@ int main(void)
 #endif
 
     LOG_DEBUG("[INIT]\n");
-    uint32_t sleep_timer1 = 1000;
-    uint32_t sleep_timer2 = 1100;
+    uint32_t sleep_timer1 = TEST_SLEEP_TIME_1;
+    uint32_t sleep_timer2 = TEST_SLEEP_TIME_2;
 
     thread_create(stack_timer1, TEST_TIMER_STACKSIZE,
                   2, THREAD_CREATE_STACKTEST,
@@ -78,13 +93,16 @@ int main(void)
     uint32_t now = 0;
     uint32_t start = xtimer_now_usec();
     uint32_t until = start + (TEST_TIME_S * US_PER_SEC);
+    uint32_t interval = TEST_INTERVAL_MS * US_PER_MS;
+    xtimer_ticks32_t last_wakeup = xtimer_now();
+
     puts("[START]");
     while((now = xtimer_now_usec()) < until) {
         unsigned percent = (100 * (now - start)) / (until - start);
 #if defined(MAIN_THREAD_PIN)
         gpio_set(main_pin);
 #endif
-        xtimer_usleep(TEST_INTERVAL_MS * US_PER_MS);
+        xtimer_periodic_wakeup(&last_wakeup, interval);
 #if defined(MAIN_THREAD_PIN)
         gpio_clear(main_pin);
 #endif