Improve kernel timers implementation, allowing to stop timer from its…

… handler.

* Allow to cancel a timer by returning false from its handler.

examples/ArduinoDiningPhilisophers/ArduinoDiningPhilisophers.ino

@@ -314,9 +314,11 @@ static Philosopher philosophers[5u] = {
 struct k_timer timer;
 struct k_sem sem;
 
-void timer_handler(struct k_timer *timer)
+int timer_handler(struct k_timer *timer)
 {
 	k_sem_give(&sem);
+
+	return 0; // continue
 }
 
 

examples/ArduinoPIODiningPhilisophers/DiningPhilisophers.cpp

@@ -315,9 +315,11 @@ static Philosopher philosophers[5u] = {
 struct k_timer timer;
 struct k_sem sem;
 
-void timer_handler(struct k_timer *timer)
+int timer_handler(struct k_timer *timer)
 {
 	k_sem_give(&sem);
+
+	return 0; // continue
 }
 
 

examples/timers/CMakeLists.txt

@@ -9,10 +9,8 @@ target_compile_definitions(${PROJECT_NAME} PUBLIC
 	CONFIG_KERNEL_SYSLOCK_HW_TIMER=1
 	CONFIG_KERNEL_TIME_SLICE_US=10000
 	CONFIG_KERNEL_TIMERS=1
-	CONFIG_SYSTEM_WORKQUEUE_ENABLE=1
-	CONFIG_INTERRUPT_POLICY=1
 	CONFIG_KERNEL_THREAD_IDLE_ADD_STACK=0x60
-
+	CONFIG_STDIO_PRINTF_TO_USART=0
 )
 
 target_link_avrtos(${PROJECT_NAME})

examples/timers/main.c

@@ -13,29 +13,32 @@
 #include <avr/io.h>
 #include <util/delay.h>
 
-volatile uint16_t counter  = 0;
-volatile uint16_t counter2 = 0;
+#define TASKS_COUNT 10u
 
-void handler1(struct k_timer *timer);
-void handler2(struct k_timer *timer);
-void thread2(void *context);
-void thread_canaries(void *ctx)
+struct periodic_task {
+	struct k_timer timer;
+	uint16_t counter; // counter
+};
+
+int periodic_task_handler(struct k_timer *timer)
 {
-	for (;;) {
-		k_dump_stack_canaries();
-		k_sleep(K_SECONDS(10));
-	}
+	struct periodic_task *task = CONTAINER_OF(timer, struct periodic_task, timer);
+
+	task->counter++;
+
+	return 0; // continue timer
 }
 
-void work_handler(struct k_work *work);
+static struct periodic_task tasks[TASKS_COUNT];
 
-K_WORK_DEFINE(mywork, work_handler);
+int mytimer1_handler(struct k_timer *timer)
+{
+	printf("mytimer1_handler()\n");
 
-K_TIMER_DEFINE(mytimer1, handler1, K_MSEC(100), 0);
-K_TIMER_DEFINE(mytimer2, handler2, K_MSEC(100), K_TIMER_STOPPED);
-K_THREAD_DEFINE(cantid, thread_canaries, 0x200, K_COOPERATIVE, NULL, 'C');
+	return -1; // stop timer
+}
 
-K_THREAD_DEFINE(th2, thread2, 0x100, K_PREEMPTIVE, NULL, 'A');
+K_TIMER_DEFINE(mytimer1, mytimer1_handler, K_MSEC(500), 0);
 
 int main(void)
 {
@@ -44,46 +47,20 @@ int main(void)
 
 	k_thread_dump_all();
 
-	k_timer_start(&mytimer2, K_NO_WAIT);
-
-	sei();
-
-	for (;;) {
-		irq_disable();
-		serial_print_p(PSTR("MAIN : "));
-		serial_u16(counter);
-		serial_transmit('\n');
-		irq_enable();
-
-		led_toggle();
-
-		k_sleep(K_MSEC(1000));
+	for (uint8_t i = 0; i < ARRAY_SIZE(tasks); i++) {
+		k_timer_init(&tasks[i].timer, periodic_task_handler, K_MSEC(100lu * (i + 1)), K_NO_WAIT);
 	}
-}
-
-void handler1(struct k_timer *timer)
-{
-	counter++;
-}
 
-void handler2(struct k_timer *timer)
-{
-	counter2++;
-	k_system_workqueue_submit(&mywork);
-}
+	for (;;) {
+		for (uint8_t i = 0; i < ARRAY_SIZE(tasks); i++) {
+			printf("Task %u counter = %u\n", i, tasks[i].counter);
+		}
 
-void work_handler(struct k_work *work)
-{
-	k_sleep(K_MSEC(200));
+		k_dump_stack_canaries();
 
-	serial_print_p(PSTR("WORKQUEUE : "));
-	serial_u16(counter2);
-	serial_transmit('\n');
-}
+		printf("Restart mytimer1\n");
+		k_timer_start(&mytimer1, mytimer1.timeout);
 
-void thread2(void *context)
-{
-	k_sleep(K_SECONDS(5));
-	k_timer_stop(&mytimer1);
-	k_sleep(K_FOREVER);
+		k_sleep(K_SECONDS(5));
+	}
 }

examples/uptime/main.c

@@ -9,9 +9,11 @@
 #include <avrtos/misc/led.h>
 #include <avrtos/misc/serial.h>
 
-void timer_handler(struct k_timer *timer)
+int timer_handler(struct k_timer *timer)
 {
 	printf("uptime: %lu seconds\n", k_uptime_get());
+
+	return 0;
 }
 
 K_TIMER_DEFINE(mstimer, timer_handler, K_MSEC(1000), 1000);

platformio.ini

@@ -331,6 +331,7 @@ build_flags =
     ${env.build_flags}
 	-DCONFIG_KERNEL_COOPERATIVE_THREADS=0
 	-DCONFIG_KERNEL_THREAD_IDLE=0
+	-DCONFIG_STDIO_PRINTF_TO_USART=0
 
 [env:DemoButton]
 build_src_filter =
@@ -1141,9 +1142,8 @@ build_flags =
 	-DCONFIG_KERNEL_SYSLOCK_HW_TIMER=1
 	-DCONFIG_KERNEL_TIME_SLICE_US=10000
 	-DCONFIG_KERNEL_TIMERS=1
-	-DCONFIG_SYSTEM_WORKQUEUE_ENABLE=1
-	-DCONFIG_INTERRUPT_POLICY=1
 	-DCONFIG_KERNEL_THREAD_IDLE_ADD_STACK=0x60
+	-DCONFIG_STDIO_PRINTF_TO_USART=0
 
 [env:Uptime]
 build_src_filter =

readme.md

@@ -78,9 +78,11 @@ Planned Features (TODOs):
 - Implementation of builtin_ctz for 8-bit variables
 - Removal of outdated samples
 - Tutorial
-- Kconfig
+- Kconfig to configure the kernel and generate the configuration file
 - make the kernel ISR aware with a dedicated ISR stack (can IDLE thread be reused?)
 - Sample for discovering the I2C bus
+- Doubly linked list implementation for tqueue for optimized removal
+- Set `CONFIG_STDIO_PRINTF_TO_USART=0` by default
 
 ### Description
 

src/avrtos/dstruct/tqueue.c

@@ -24,7 +24,11 @@ void z_tqueue_schedule(struct titem **root, struct titem *item)
 		/* next of previous become current */
 		struct titem *p_current = *prev_next_p;
 
-		/* if new element expires after we go to next */
+		/* if new element expires after we go to next.
+		 *
+		 * Note: if two items expire at the same time, the item that was inserted first
+		 * will be processed first. This justify the "<=" in the condition.
+		 */
 		if (p_current->delay_shift <= item->delay_shift) {
 			item->delay_shift -= p_current->delay_shift;
 			prev_next_p = &(p_current->next);

src/avrtos/dstruct/tqueue.h

@@ -35,6 +35,7 @@ struct titem {
 };
 
 typedef struct titem titem_t;
+typedef struct titem tqueue_t;
 
 #define DEFINE_TQUEUE(name) struct titem *name = NULL
 #define INIT_TITEM(timeout_ms)                                                           \
@@ -80,7 +81,6 @@ void tqueue_schedule(struct titem **root, struct titem *item, k_delta_t timeout)
  * @brief Shift the queue time of {time_passed}
  *
  * Assumptions :
- *  - time_passed is not null
  *  - root is not null
  *
  * @param root
@@ -91,7 +91,7 @@ void tqueue_shift(struct titem **root, k_delta_t time_passed);
 /**
  * @brief Pop an item from the time queue.
  *
- * Note: this function doesn't set the poped item->next parameter to null.
+ * Note: this function doesn't alter/reset the item->tie nor the item->next members.
  *
  * Assumptions:
  * - root is not null
@@ -101,18 +101,6 @@ void tqueue_shift(struct titem **root, k_delta_t time_passed);
  */
 struct titem *tqueue_pop(struct titem **root);
 
-/**
- * @brief Shift the queue time of {time_passed}
- * and pop an item from the time queue.
- *
- * @see tqueue_shift, tqueue_pop
- *
- * @param root
- * @param time_passed
- * @return struct titem*
- */
-// struct titem *tqueue_shift_pop(struct titem **root, k_delta_t time_passed);
-
 /**
  * @brief
  *

src/avrtos/timer.c

@@ -38,9 +38,7 @@ void z_timer_start(struct k_timer *timer, k_timeout_t starting_delay)
 	__ASSERT_NOTNULL(timer);
 
 	const uint8_t key = irq_lock();
-
 	tqueue_schedule(&z_timers_runqueue, &timer->tie, starting_delay.value);
-
 	irq_unlock(key);
 }
 
@@ -55,21 +53,33 @@ void z_timers_process(void)
 
 	while (!!(item = tqueue_pop(&z_timers_runqueue))) {
 		timer = CONTAINER_OF(item, struct k_timer, tie);
-		timer->handler(timer);
-		timer->tie.next	   = NULL;
-		timer->tie.timeout = timer->timeout.value;
-		z_tqueue_schedule(&z_timers_runqueue, &timer->tie);
+
+		int ret = timer->handler(timer);
+
+		/* stop timer if handler returns non-zero */
+		if (ret != 0) {
+			timer->tie.timeout = K_TIMER_STOPPED;
+		}
+
+		/* reschedule if not stopped */
+		if (timer->tie.timeout != K_TIMER_STOPPED) {
+			timer->tie.next	   = NULL;
+			timer->tie.timeout = timer->timeout.value;
+			z_tqueue_schedule(&z_timers_runqueue, &timer->tie);
+		}
 	}
 }
 
-void k_timer_init(struct k_timer *timer,
-				  k_timer_handler_t handler,
-				  k_timeout_t timeout,
-				  k_timeout_t starting_delay)
+int8_t k_timer_init(struct k_timer *timer,
+					k_timer_handler_t handler,
+					k_timeout_t timeout,
+					k_timeout_t starting_delay)
 {
 	__ASSERT_NOTNULL(timer);
 	__ASSERT_NOTNULL(handler);
 
+	if (K_TIMEOUT_EQ(timeout, K_NO_WAIT)) return -EINVAL;
+
 	timer->handler = handler;
 	timer->timeout = timeout;
 
@@ -78,6 +88,8 @@ void k_timer_init(struct k_timer *timer,
 	} else {
 		timer->tie.timeout = K_FOREVER.value;
 	}
+
+	return 0;
 }
 
 bool k_timer_started(struct k_timer *timer)
@@ -87,9 +99,7 @@ bool k_timer_started(struct k_timer *timer)
 	bool ret;
 
 	const uint8_t key = irq_lock();
-
-	ret = timer->tie.timeout != K_TIMER_STOPPED;
-
+	ret				  = timer->tie.timeout != K_TIMER_STOPPED;
 	irq_unlock(key);
 
 	return ret;
@@ -99,15 +109,19 @@ int8_t k_timer_stop(struct k_timer *timer)
 {
 	__ASSERT_NOTNULL(timer);
 
-	if (!k_timer_started(timer)) {
-		return -1;
+	int ret;
+
+	if (timer->tie.timeout != K_TIMER_STOPPED) {
+		const uint8_t key = irq_lock();
+		tqueue_remove(&z_timers_runqueue, &timer->tie);
+		timer->tie.timeout = K_TIMER_STOPPED;
+		irq_unlock(key);
+		ret = 0;
+	} else {
+		ret = -1;
 	}
 
-	const uint8_t key = irq_lock();
-	tqueue_remove(&z_timers_runqueue, &timer->tie);
-	irq_unlock(key);
-	timer->timeout = K_FOREVER;
-	return 0;
+	return ret;
 }
 
 int8_t k_timer_start(struct k_timer *timer, k_timeout_t starting_delay)

src/avrtos/timer.h

@@ -27,7 +27,7 @@ struct k_timer;
  * k_yield_from_isr_cond() from a timer handler.
  *
  */
-typedef void (*k_timer_handler_t)(struct k_timer *);
+typedef int (*k_timer_handler_t)(struct k_timer *);
 
 struct k_timer {
 	struct titem tie;
@@ -53,13 +53,22 @@ __kernel void z_timers_process(void);
 
 __kernel void z_timer_start(struct k_timer *timer, k_timeout_t starting_delay);
 
-__kernel void k_timer_init(struct k_timer *timer,
-						   k_timer_handler_t handler,
-						   k_timeout_t timeout,
-						   k_timeout_t starting_delay);
+__kernel int8_t k_timer_init(struct k_timer *timer,
+							 k_timer_handler_t handler,
+							 k_timeout_t timeout,
+							 k_timeout_t starting_delay);
 
 __kernel bool k_timer_started(struct k_timer *timer);
 
+/**
+ * @brief Stop a timer
+ *
+ * Note: Usage of this function is discouraged in timer handlers.
+ * Prefer returning any non-zero value from the handler to stop the timer.
+ *
+ * @param timer
+ * @return __kernel
+ */
 __kernel int8_t k_timer_stop(struct k_timer *timer);
 
 __kernel int8_t k_timer_start(struct k_timer *timer, k_timeout_t starting_delay);