From e174276556ea3b100097b51f6946e6c0e5ee41b8 Mon Sep 17 00:00:00 2001 From: John Wellbelove Date: Thu, 19 Oct 2017 10:31:27 +0100 Subject: [PATCH] Fix bug if timer list is empty for 49+ days. (Internal tick count rollover) --- library.properties | 2 +- src/callback_timer.h | 42 +++++++++++++++++--------------- src/message_timer.h | 46 ++++++++++++++++++++---------------- test/test_callback_timer.cpp | 38 +++++++++++++++++++++++++++++ test/test_message_timer.cpp | 38 +++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 41 deletions(-) diff --git a/library.properties b/library.properties index 0d293558..28e0d889 100644 --- a/library.properties +++ b/library.properties @@ -1,5 +1,5 @@ name=Embedded Template Library -version=10.0.0 +version=10.0.1 author= John Wellbelove maintainer=John Wellbelove sentence=A C++ template library tailored for embedded systems. diff --git a/src/callback_timer.h b/src/callback_timer.h index 545c414a..7a4c9f09 100644 --- a/src/callback_timer.h +++ b/src/callback_timer.h @@ -467,34 +467,37 @@ namespace etl { if (enabled) { - tick_count += count; - if (process_semaphore == 0) { // We have something to do? - while (!active_list.empty() && (tick_count >= active_list.front().delta)) + if (!active_list.empty()) { - etl::callback_timer_data& timer = active_list.front(); + tick_count += count; - tick_count -= timer.delta; - - active_list.remove(timer.id, true); - - if (timer.repeating) + while (!active_list.empty() && (tick_count >= active_list.front().delta)) { - timer.delta = timer.period; - active_list.insert(timer.id); - } - - if (timer.p_callback != nullptr) - { - if (timer.is_c_callback) + etl::callback_timer_data& timer = active_list.front(); + + tick_count -= timer.delta; + + active_list.remove(timer.id, true); + + if (timer.repeating) { - reinterpret_cast(timer.p_callback)(); + timer.delta = timer.period; + active_list.insert(timer.id); } - else + + if (timer.p_callback != nullptr) { - (*reinterpret_cast*>(timer.p_callback))(); + if (timer.is_c_callback) + { + reinterpret_cast(timer.p_callback)(); + } + else + { + (*reinterpret_cast*>(timer.p_callback))(); + } } } } @@ -529,6 +532,7 @@ namespace etl active_list.remove(timer.id, false); } + // Compensate for current tick count. timer.delta = tick_count; if (!immediate_) diff --git a/src/message_timer.h b/src/message_timer.h index ed238086..6e84db55 100644 --- a/src/message_timer.h +++ b/src/message_timer.h @@ -431,37 +431,40 @@ namespace etl { if (enabled) { - tick_count += count; - if (process_semaphore == 0) { // We have something to do? - while (!active_list.empty() && (tick_count >= active_list.front().delta)) + if (!active_list.empty()) { - etl::message_timer_data& timer = active_list.front(); + tick_count += count; - tick_count -= timer.delta; - - active_list.remove(timer.id, true); - - if (timer.repeating) + while (!active_list.empty() && (tick_count >= active_list.front().delta)) { - timer.delta = timer.period; - active_list.insert(timer.id); - } + etl::message_timer_data& timer = active_list.front(); - if (timer.p_router != nullptr) - { - if (timer.p_router->get_message_router_id() == etl::imessage_router::MESSAGE_BUS) + tick_count -= timer.delta; + + active_list.remove(timer.id, true); + + if (timer.repeating) { - // Send to a message bus. - etl::imessage_bus& bus = static_cast(*(timer.p_router)); - bus.receive(timer.destination_router_id, *(timer.p_message)); + timer.delta = timer.period; + active_list.insert(timer.id); } - else + + if (timer.p_router != nullptr) { - // Send to a router. - timer.p_router->receive(*(timer.p_message)); + if (timer.p_router->get_message_router_id() == etl::imessage_router::MESSAGE_BUS) + { + // Send to a message bus. + etl::imessage_bus& bus = static_cast(*(timer.p_router)); + bus.receive(timer.destination_router_id, *(timer.p_message)); + } + else + { + // Send to a router. + timer.p_router->receive(*(timer.p_message)); + } } } } @@ -496,6 +499,7 @@ namespace etl active_list.remove(timer.id, false); } + // Compensate for current tick count. timer.delta = tick_count; if (!immediate_) diff --git a/test/test_callback_timer.cpp b/test/test_callback_timer.cpp index 09b0f850..7ab47d95 100644 --- a/test/test_callback_timer.cpp +++ b/test/test_callback_timer.cpp @@ -439,6 +439,44 @@ namespace CHECK_ARRAY_EQUAL(compare2.data(), free_tick_list2.data(), compare2.size()); } + //========================================================================= + TEST(callback_timer_one_shot_empty_list_huge_tick_before_insert) + { + etl::callback_timer<3> timer_controller; + + etl::timer::id::type id1 = timer_controller.register_timer(free_callback1, 5, etl::timer::mode::SINGLE_SHOT); + + free_tick_list1.clear(); + + timer_controller.start(id1); + + timer_controller.enable(true); + + ticks = 0; + + const uint32_t step = 5; + + for (uint32_t i = 0; i < step; ++i) + { + ++ticks; + timer_controller.tick(1); + } + + // Huge tick count. + timer_controller.tick(UINT32_MAX - step + 1); + + timer_controller.start(id1); + + for (uint32_t i = 0; i < step; ++i) + { + ++ticks; + timer_controller.tick(1); + } + std::vector compare1 = { 5, 10 }; + + CHECK_ARRAY_EQUAL(compare1.data(), free_tick_list1.data(), compare1.size()); + } + //========================================================================= #if REALTIME_TEST etl::callback_timer<3> controller; diff --git a/test/test_message_timer.cpp b/test/test_message_timer.cpp index eac388b8..9445b5bd 100644 --- a/test/test_message_timer.cpp +++ b/test/test_message_timer.cpp @@ -510,6 +510,44 @@ namespace CHECK_ARRAY_EQUAL(compare2.data(), router1.message2.data(), compare2.size()); } + //========================================================================= + TEST(callback_timer_one_shot_empty_list_huge_tick_before_insert) + { + etl::message_timer<3> timer_controller; + + etl::timer::id::type id1 = timer_controller.register_timer(message1, router1, 5, etl::timer::mode::SINGLE_SHOT); + + router1.clear(); + + timer_controller.start(id1); + + timer_controller.enable(true); + + ticks = 0; + + const uint32_t step = 5; + + for (uint32_t i = 0; i < step; ++i) + { + ++ticks; + timer_controller.tick(1); + } + + // Huge tick count. + timer_controller.tick(UINT32_MAX - step + 1); + + timer_controller.start(id1); + + for (uint32_t i = 0; i < step; ++i) + { + ++ticks; + timer_controller.tick(1); + } + std::vector compare1 = { 5, 10 }; + + CHECK_ARRAY_EQUAL(compare1.data(), router1.message1.data(), compare1.size()); + } + //========================================================================= #if REALTIME_TEST etl::message_timer<3> controller;