Optimized next_fire_us to uint32 and fixed bug with simultaneous tasks ticking

Author: StefanCostea

Inc/HALAL/Services/Time/Scheduler.hpp

@@ -39,7 +39,9 @@ struct Scheduler {
 
     static void start();
     static void update();
-    static inline uint64_t get_global_tick();
+    static inline uint64_t get_global_tick() {
+        return global_tick_us_;
+    }
 
     static inline uint8_t register_task(uint32_t period_us, callback_t func) {
         if(period_us == 0) [[unlikely]] period_us = 1;
@@ -72,7 +74,7 @@ struct Scheduler {
     private:
 #endif
     struct Task {
-        uint64_t next_fire_us{0};
+        uint32_t next_fire_us{0};
         callback_t callback{};
         uint32_t period_us{0};
         bool repeating{false};
@@ -92,7 +94,7 @@ struct Scheduler {
     static uint32_t ready_bitmap_;
     static uint32_t free_bitmap_;
     static uint64_t global_tick_us_;
-    static uint64_t current_interval_us_;
+    static uint32_t current_interval_us_;
 
     static inline uint8_t allocate_slot();
     static inline void release_slot(uint8_t id);

Src/HALAL/Services/Time/Scheduler.cpp

@@ -25,7 +25,7 @@
 #define Scheduler_global_timer ((TIM_TypeDef*)SCHEDULER_TIMER_BASE)
 namespace {
 constexpr uint64_t kMaxIntervalUs =
-    static_cast<uint64_t>(std::numeric_limits<uint32_t>::max()) + 1ULL;
+    static_cast<uint64_t>(std::numeric_limits<uint32_t>::max())/2 + 1ULL;
 }
 
 std::array<Scheduler::Task, Scheduler::kMaxTasks> Scheduler::tasks_{};
@@ -35,7 +35,7 @@ uint32_t Scheduler::active_task_count_{0};
 uint32_t Scheduler::ready_bitmap_{0};
 uint32_t Scheduler::free_bitmap_{0xFFFF'FFFF};
 uint64_t Scheduler::global_tick_us_{0};
-uint64_t Scheduler::current_interval_us_{0};
+uint32_t Scheduler::current_interval_us_{0};
 
 inline uint8_t Scheduler::get_at(uint8_t idx) {
     int word_idx = idx > 7;
@@ -160,8 +160,6 @@ void Scheduler::update() {
     }
 }
 
-inline uint64_t Scheduler::get_global_tick() { return global_tick_us_; }
-
 // void Scheduler::global_timer_callback() { on_timer_update(); }
 
 inline uint8_t Scheduler::allocate_slot() {
@@ -186,7 +184,7 @@ void Scheduler::insert_sorted(uint8_t id) {
     while (left < right) {
         std::size_t mid = left + ((right - left) / 2);
         const Task& mid_task = tasks_[Scheduler::get_at(mid)];
-        if (mid_task.next_fire_us <= task.next_fire_us) {
+        if ((int32_t)(task.next_fire_us - mid_task.next_fire_us) >= 0) {
             left = mid + 1;
         } else {
             right = mid;
@@ -265,14 +263,14 @@ void Scheduler::schedule_next_interval() {
     Scheduler::global_timer_enable();
     uint8_t next_id = Scheduler::front_id();  // sorted_task_ids_[0]
     Task& next_task = tasks_[next_id];
-    uint64_t delta = ((next_task.next_fire_us + 1ULL) > global_tick_us_) 
-        ? (next_task.next_fire_us - global_tick_us_) : 1ULL;
-
-    if (delta > kMaxIntervalUs) [[unlikely]] {
-        current_interval_us_ = kMaxIntervalUs;
+    int32_t diff = (int32_t)(next_task.next_fire_us - static_cast<uint32_t>(global_tick_us_));
+    uint32_t delta;
+    if (diff <= 0) [[unlikely]]{
+        delta = 1; // Task is due or overdue
     } else {
-        current_interval_us_ = delta;
+        delta = static_cast<uint32_t>(diff);
     }
+    current_interval_us_ = delta;
 
     configure_timer_for_interval(current_interval_us_);
 }
@@ -288,21 +286,28 @@ inline void Scheduler::configure_timer_for_interval(uint64_t microseconds) {
 void Scheduler::on_timer_update() {
     global_tick_us_ += current_interval_us_;
 
-    uint8_t candidate_id = Scheduler::front_id();
-    Task& task = tasks_[candidate_id];
-    pop_front();
-    ready_bitmap_ |= (1u << candidate_id); // mark task as ready
+    while (active_task_count_ > 0) { //Pop all due tasks, several might be due in the same tick
+        uint8_t candidate_id = Scheduler::front_id();
+        Task& task = tasks_[candidate_id];
+        int32_t diff = (int32_t)(task.next_fire_us - static_cast<uint32_t>(global_tick_us_));
+        if (diff > 0) [[likely]]{
+            break; // Task is in the future, stop processing
+        }
+        pop_front();
+        ready_bitmap_ |= (1u << candidate_id); // mark task as ready
 
-    if (task.repeating) [[likely]] {
-        task.next_fire_us = global_tick_us_ + task.period_us;
-        insert_sorted(candidate_id);
+        if (task.repeating) [[likely]] {
+            task.next_fire_us = static_cast<uint32_t>(global_tick_us_ + task.period_us);
+            insert_sorted(candidate_id);
+        }
     }
 
     schedule_next_interval();
 }
 
 uint8_t Scheduler::register_task(uint32_t period_us, callback_t func, bool repeating) {
     if (func == nullptr) [[unlikely]] return static_cast<uint8_t>(Scheduler::INVALID_ID);
+    if(period_us >= kMaxIntervalUs) [[unlikely]] return static_cast<uint8_t>(Scheduler::INVALID_ID);
 
     uint8_t slot = allocate_slot();
     if(slot == Scheduler::INVALID_ID) return slot;
@@ -311,8 +316,7 @@ uint8_t Scheduler::register_task(uint32_t period_us, callback_t func, bool repea
     task.callback = func;
     task.period_us = period_us;
     task.repeating = repeating;
-    task.next_fire_us = global_tick_us_ + period_us;
-
+    task.next_fire_us = static_cast<uint32_t>(global_tick_us_ + period_us);
     insert_sorted(slot);
     schedule_next_interval();
     return slot;

Tests/Time/scheduler_test.cpp

@@ -162,3 +162,161 @@ TEST_F(SchedulerTests, TaskDe_ReRegistration) {
     EXPECT_EQ(operational_execs, 2);
     EXPECT_EQ(fault_execs, 2);
 }
+
+struct SelfValidatingTask {
+    static uint64_t last_fire_tick;
+    static uint32_t expected_period;
+    static bool error_detected;
+    static int execution_count;
+
+    static void reset(uint64_t start_tick, uint32_t period) {
+        last_fire_tick = start_tick;
+        expected_period = period;
+        error_detected = false;
+        execution_count = 0;
+    }
+
+    static void callback() {
+        uint64_t current_tick = Scheduler::get_global_tick();
+
+        uint32_t actual_interval = static_cast<uint32_t>(current_tick - last_fire_tick);
+
+        if (actual_interval != expected_period) {
+            error_detected = true;
+            printf("Timing Error Exp: %u, Got: %u at Tick: %lu\n", expected_period, actual_interval, current_tick);
+        }
+        printf("Task correct Exp: %u, Got: %u at Tick: %lu\n", expected_period, actual_interval, current_tick);
+
+        last_fire_tick = current_tick;
+        execution_count++;
+    }
+};
+
+uint64_t SelfValidatingTask::last_fire_tick = 0;
+uint32_t SelfValidatingTask::expected_period = 0;
+bool SelfValidatingTask::error_detected = false;
+int SelfValidatingTask::execution_count = 0;
+
+TEST_F(SchedulerTests, ExplicitOverflowBoundaryCrossing) {
+    // Start close to the 32-bit limit
+    // 0xFFFFFFF0 is 16 ticks away from 32-bit overflow
+    uint64_t start_time = 0xFFFFFFF0ULL;
+    Scheduler::global_tick_us_ = start_time;
+    
+    // We expect the task to fire every 10 ticks
+    SelfValidatingTask::reset(start_time, 10);
+
+    Scheduler::register_task(10, &SelfValidatingTask::callback);
+    Scheduler::start();
+    TIM2_BASE->PSC = 2;
+
+    // Start: FFFFFFF0
+    // Fire 1: FFFFFFF0 + 10 = FFFFFFFA (OK)
+    // Fire 2: FFFFFFFA + 10 = 00000004 (OVERFLOW CROSSING)
+    // Fire 3: 00000004 + 10 = 0000000E (OK)
+    // Fire 4: 0000000E + 10 = 00000018 (OK)
+    constexpr int NUM_TICKS = 50;
+    
+    for(int i = 0; i < NUM_TICKS; i++){
+        for(int j = 0; j <= TIM2_BASE->PSC; j++) {
+            TIM2_BASE->inc_cnt_and_check(1);
+        }
+        Scheduler::update();
+        
+        if(SelfValidatingTask::error_detected) break;
+    }
+
+    EXPECT_FALSE(SelfValidatingTask::error_detected);
+
+    EXPECT_EQ(SelfValidatingTask::execution_count, 5);
+
+    EXPECT_EQ(static_cast<uint32_t>(Scheduler::global_tick_us_), 0x00000022);
+}
+
+struct DualTaskValidator {
+    struct Context {
+        uint64_t last_fire_tick;
+        uint32_t expected_period;
+        int exec_count;
+        bool error;
+    };
+
+    static Context ctx_A; // Task that runs BEFORE wrap
+    static Context ctx_B; // Task that runs AFTER wrap
+
+    static void reset(uint64_t start_tick) {
+        ctx_A = {start_tick, 0, 0, false};
+        ctx_B = {start_tick, 0, 0, false};
+    }
+
+    // Callback for the "Pre-Wrap" Task
+    static void callback_A() {
+        validate(ctx_A);
+    }
+
+    // Callback for the "Post-Wrap" Task
+    static void callback_B() {
+        validate(ctx_B);
+    }
+
+    static void validate(Context& ctx) {
+        uint64_t current_tick = Scheduler::get_global_tick();
+        uint32_t interval = static_cast<uint32_t>(current_tick - ctx.last_fire_tick);
+
+        if (interval != ctx.expected_period) {
+            ctx.error = true;
+            printf("Error Expected %u, got %u\n", ctx.expected_period, interval);
+        }
+        ctx.last_fire_tick = current_tick;
+        ctx.exec_count++;
+    }
+};
+
+DualTaskValidator::Context DualTaskValidator::ctx_A;
+DualTaskValidator::Context DualTaskValidator::ctx_B;
+
+
+TEST_F(SchedulerTests, TwoTasksStraddlingOverflow) {
+    //Start close to overflow
+    // Current Time: 0xFFFFFFF0 (16 ticks before wrap)
+    uint64_t start_time = 0xFFFFFFF0ULL;
+    Scheduler::global_tick_us_ = start_time;
+    DualTaskValidator::reset(start_time);
+
+    // Task A: Period 5.  Target = FFFFFFF0 + 5  = FFFFFFF5 (Before Wrap)
+    // Task B: Period 20. Target = FFFFFFF0 + 20 = 00000004 (After Wrap)
+    DualTaskValidator::ctx_A.expected_period = 5;
+    DualTaskValidator::ctx_B.expected_period = 20;
+
+    Scheduler::register_task(20, &DualTaskValidator::callback_B); // Registers "Later" task first
+    Scheduler::register_task(5,  &DualTaskValidator::callback_A); // Registers "Earlier" task second
+    
+    Scheduler::start();
+    TIM2_BASE->PSC = 2; 
+
+    // T=0  (Global FFFFFFF0)
+    // T=5  (Global FFFFFFF5): Task A should fire.
+    // T=10 (Global FFFFFFFA): Task A should fire again.
+    // T=15 (Global FFFFFFFF): Task A should fire again.
+    // T=16 (Global 00000000): --- OVERFLOW ---
+    // T=20 (Global 00000004): Task A fires (4th time). Task B fires (1st time).
+    constexpr int NUM_TICKS = 30;
+    
+    for(int i = 0; i < NUM_TICKS; i++){
+        for(int j = 0; j <= TIM2_BASE->PSC; j++) {
+            TIM2_BASE->inc_cnt_and_check(1);
+        }
+        Scheduler::update();
+    }
+
+    // Task A should have fired every 5 ticks. 30 / 5 = 6 times total
+    EXPECT_FALSE(DualTaskValidator::ctx_A.error) << "Task A (Short Period) timing failed";
+    EXPECT_EQ(DualTaskValidator::ctx_A.exec_count, 6);
+
+    // Task B should have fired once at T=20 (which is 0x00000004)
+    EXPECT_FALSE(DualTaskValidator::ctx_B.error) << "Task B (Long Period) timing failed";
+    EXPECT_EQ(DualTaskValidator::ctx_B.exec_count, 1);
+    
+    // 0xFFFFFFF0 + 30 = 0x10000000E -> cast to 32-bit -> 0x0000000E
+    EXPECT_EQ(static_cast<uint32_t>(Scheduler::global_tick_us_), 0x0000000E);
+}