Minor fixes to scheduler

Rebased from remotes/origin/MockTim

Author: StefanCostea

Inc/MockedDrivers/common.hpp

@@ -45,7 +45,7 @@ typedef enum
 
 #define READ_REG(REG)         ((REG))
 
-#define MODIFY_REG(REG, CLEARMASK, SETMASK)  WRITE_REG((REG), (((READ_REG(REG)) & (~(CLEARMASK))) | (SETMASK)))
+#define MODIFY_REG(REG, CLEARMASK, SETMASK)  WRITE_REG((REG), (((READ_REG(REG)) & (~(uint32_t)(CLEARMASK))) | (SETMASK)))
 
 #define POSITION_VAL(VAL)     (__CLZ(__RBIT(VAL)))
 
@@ -65,7 +65,7 @@ typedef enum
   do {                                                       \
     uint32_t val;                                            \
     do {                                                     \
-      val = __LDREXW((__IO uint32_t *)&(REG)) & ~(BIT);      \
+      val = __LDREXW((__IO uint32_t *)&(REG)) & ~(uint32_t)(BIT);      \
     } while ((__STREXW(val,(__IO uint32_t *)&(REG))) != 0U); \
   } while(0)
 
@@ -74,7 +74,7 @@ typedef enum
   do {                                                                     \
     uint32_t val;                                                          \
     do {                                                                   \
-      val = (__LDREXW((__IO uint32_t *)&(REG)) & ~(CLEARMSK)) | (SETMASK); \
+      val = (__LDREXW((__IO uint32_t *)&(REG)) & ~(uint32_t)(CLEARMSK)) | (SETMASK); \
     } while ((__STREXW(val,(__IO uint32_t *)&(REG))) != 0U);               \
   } while(0)
 
@@ -92,7 +92,7 @@ typedef enum
   do {                                                       \
     uint16_t val;                                            \
     do {                                                     \
-      val = __LDREXH((__IO uint16_t *)&(REG)) & ~(BIT);      \
+      val = __LDREXH((__IO uint16_t *)&(REG)) & ~(uint16_t)(BIT);      \
     } while ((__STREXH(val,(__IO uint16_t *)&(REG))) != 0U); \
   } while(0)
 
@@ -101,6 +101,6 @@ typedef enum
   do {                                                                     \
     uint16_t val;                                                          \
     do {                                                                   \
-      val = (__LDREXH((__IO uint16_t *)&(REG)) & ~(CLEARMSK)) | (SETMASK); \
+      val = (__LDREXH((__IO uint16_t *)&(REG)) & ~(uint16_t)(CLEARMSK)) | (SETMASK); \
     } while ((__STREXH(val,(__IO uint16_t *)&(REG))) != 0U);               \
   } while(0)

Src/HALAL/Services/Time/Scheduler.cpp

@@ -40,7 +40,7 @@ uint64_t Scheduler::current_interval_us_{0};
 inline uint8_t Scheduler::get_at(uint8_t idx) {
     int word_idx = idx > 7;
     uint32_t shift = (idx & 7) << 2;
-    return (((uint32_t*)sorted_task_ids_)[word_idx] & (0x0F << shift)) >> shift;
+    return (((uint32_t*)&sorted_task_ids_)[word_idx] & (0x0F << shift)) >> shift;
 }
 inline void Scheduler::set_at(uint8_t idx, uint8_t id) {
     uint32_t shift = idx*4;
@@ -49,7 +49,7 @@ inline void Scheduler::set_at(uint8_t idx, uint8_t id) {
     // sorted_task_ids_ |= ((id & 0x0F) << shift); // This is also an option in case id is incorrect, I don't think it's necessary though
 }
 inline uint8_t Scheduler::front_id() {
-    return ((uint32_t*)sorted_task_ids_)[0] & 0xF;
+    return ((uint32_t*)&sorted_task_ids_)[0] & 0xF;
 }
 inline void Scheduler::pop_front() {
     // O(1) remove of logical index 0
@@ -94,17 +94,17 @@ void Scheduler::start() {
     Scheduler_global_timer->CNT = 0; /* Clear counter value */
 
     NVIC_EnableIRQ(SCHEDULER_GLOBAL_TIMER_IRQn);
-    LL_TIM_ClearFlag_UPDATE(Scheduler_global_timer);
+    CLEAR_BIT(Scheduler_global_timer->SR, LL_TIM_SR_UIF); /* clear update interrupt flag */
     // NOTE(vic): We don't need to set the flag since there won't be any tasks at the start/it will get set in schedule_next_interval()
     Scheduler::global_timer_enable();
     //Scheduler::schedule_next_interval();
 }
 
-SCHEDULER_GLOBAL_TIMER_CALLBACK() { 
-    /* clear update interrupt flag */
-    LL_TIM_ClearFlag_UPDATE(Scheduler_global_timer);
+SCHEDULER_GLOBAL_TIMER_CALLBACK() {
+    CLEAR_BIT(Scheduler_global_timer->SR, TIM_SR_UIF);
     Scheduler::on_timer_update();
 }
+
 void Scheduler::update() {
     while(ready_bitmap_ != 0u) {
         uint32_t bit_index = static_cast<uint32_t>(__builtin_ctz(ready_bitmap_));
@@ -183,6 +183,7 @@ void Scheduler::insert_sorted(uint8_t id) {
     }
 
     sorted_task_ids_ = ((uint64_t)hi << 32) | lo;
+    active_task_count_++;
 }
 
 void Scheduler::remove_sorted(uint8_t id) {
@@ -215,6 +216,7 @@ void Scheduler::remove_sorted(uint8_t id) {
 
     // Remove element (lower part | higher pushing nibble out of mask)
     Scheduler::sorted_task_ids_ = (Scheduler::sorted_task_ids_ & mask) | ((Scheduler::sorted_task_ids_ >> 4) & ~mask);
+    active_task_count_--;
 }
 
 void Scheduler::schedule_next_interval() {

Tests/Time/scheduler_test.cpp

@@ -2,32 +2,95 @@
 #include <thread>
 #include<chrono>
 #include "HALAL/Services/Time/Scheduler.hpp"
+
 int count = 0;
 void fake_workload(){
     count++;
 }
 
-TEST(SchedulerTests, UsedBitmap) {
+class SchedulerTests : public ::testing::Test {
+protected:
+    void SetUp() override {
+        Scheduler::active_task_count_ = 0;
+        Scheduler::used_bitmap_ = 0;
+        Scheduler::ready_bitmap_ = 0;
+        Scheduler::sorted_task_ids_ = 0;
+        Scheduler::global_tick_us_ = 0;
+        Scheduler::current_interval_us_ = 0;
+        
+        // Reset global callback task count
+        count = 0;
+        
+        // Reset Timer
+        TIM2_BASE->CNT = 0;
+        TIM2_BASE->ARR = 0;
+        TIM2_BASE->SR = 0;
+        TIM2_BASE->CR1 = 0;
+        TIM2_BASE->DIER = 0;
+    }
+};
+
+TEST_F(SchedulerTests, UsedBitmap) {
     Scheduler::register_task(10,&fake_workload);
     EXPECT_EQ(Scheduler::used_bitmap_,1);
 }
 
-TEST(SchedulerTests, TaskRegistration) {
+TEST_F(SchedulerTests, TaskRegistration) {
     Scheduler::register_task(10,&fake_workload);
     EXPECT_EQ(Scheduler::tasks_[0].callback,fake_workload);
 }
 
-TEST(SchedulerTests, TaskExecution) {
+TEST_F(SchedulerTests, TaskExecutionShort) {
+    Scheduler::register_task(10,&fake_workload);
+    Scheduler::start();
+    // TIM2_BASE->ARR = 500; 
+    // TIM2_BASE->generate_update();
+    
+    constexpr int NUM_TICKS = 1'000;
+    for(int i = 0; i < NUM_TICKS; i++){
+        TIM2_BASE->CNT++;
+        Scheduler::update();
+    }
+    // 1000 ticks / 10 ticks/task = 100 executions.
+    EXPECT_EQ(count, 100);
+}
+
+TEST_F(SchedulerTests, TaskExecutionLong) {
     Scheduler::register_task(10,&fake_workload);
     Scheduler::start();
+    // TIM2_BASE->ARR = 500;
+    // TIM2_BASE->generate_update();
+    
     constexpr int NUM_TICKS = 1'000'000;
-    TIM2_BASE->ARR = 500;
-    TIM2_BASE->generate_update();
-    for(int i = 0; i <= NUM_TICKS; i++){
+    for(int i = 0; i < NUM_TICKS; i++){
         TIM2_BASE->CNT++;
         Scheduler::update();
     }
-    // one tick is 1us, and we register a task that executes every 10us
-    // thus it should execute NUM_TICKS/10
-    EXPECT_EQ(count,100'000);
-}
+    EXPECT_EQ(count, 100'000);
+}
+
+TEST_F(SchedulerTests, SetTimeout) {
+    Scheduler::set_timeout(10, &fake_workload);
+    Scheduler::start();
+    
+    constexpr int NUM_TICKS = 100;
+    for(int i = 0; i < NUM_TICKS; i++){
+        TIM2_BASE->CNT++;
+        Scheduler::update();
+    }
+    EXPECT_EQ(count, 1);
+}
+
+TEST_F(SchedulerTests, GlobalTickOverflow) {
+    Scheduler::global_tick_us_ = 0xFFFFFFF0ULL; // Near 32-bit max
+    Scheduler::register_task(20, &fake_workload);
+    Scheduler::start();
+    
+    constexpr int NUM_TICKS = 100;
+    for(int i = 0; i < NUM_TICKS; i++){
+        TIM2_BASE->CNT++;
+        Scheduler::update();
+    }
+    // 100 ticks /20 ticks/task = 5 executions.
+    EXPECT_EQ(count, 5);
+}