From 2e379d422bf188f677a542d31ade52fc15fd1efc Mon Sep 17 00:00:00 2001 From: Lefucjusz Date: Tue, 15 Oct 2024 11:36:54 +0200 Subject: [PATCH] [BH-2076] Fix timer interval overflow Fix of the issue that SystemTimer interval computation would overflow when trying to set interval longer than 4,294,967ms. That resulted in setting invalid timeout value. --- module-sys/Service/SystemTimer.cpp | 19 ++++++++----------- .../Service/include/Timers/SystemTimer.hpp | 18 +++++++++--------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/module-sys/Service/SystemTimer.cpp b/module-sys/Service/SystemTimer.cpp index e325d8be90cff413fd97f8fe2472fd3a9286c42f..a75245f528f3e5a0aa834b90fb90dc836ac05b16 100644 --- a/module-sys/Service/SystemTimer.cpp +++ b/module-sys/Service/SystemTimer.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include #if DEBUG_TIMER == 1 @@ -16,12 +16,9 @@ namespace sys::timer { - SystemTimer::SystemTimer(Service *parent, - const std::string &name, - std::chrono::milliseconds interval, - timer::Type type) - : cpp_freertos::Timer(name.c_str(), pdMS_TO_TICKS(interval.count()), type == timer::Type::Periodic), name{name}, - interval{interval}, type{type}, parent{parent} + SystemTimer::SystemTimer(Service *parent, const std::string &name, std::chrono::milliseconds interval, Type type) + : cpp_freertos::Timer(name.c_str(), cpp_freertos::Ticks::MsToTicks(interval.count()), type == Type::Periodic), + name{name}, interval{interval}, type{type}, parent{parent} { attachToService(); log_debug("%s %s timer created", name.c_str(), type == Type::Periodic ? "periodic" : "single-shot"); @@ -84,9 +81,9 @@ namespace sys::timer void SystemTimer::setInterval(std::chrono::milliseconds value) { - log_debug("Timer %s set interval to %ld ms!", name.c_str(), static_cast(value.count())); + log_debug("Timer %s set interval to %" PRIi64 " ms!", name.c_str(), value.count()); interval = value; - cpp_freertos::Timer::SetPeriod(pdMS_TO_TICKS(interval.count()), 0); + cpp_freertos::Timer::SetPeriod(cpp_freertos::Ticks::MsToTicks(interval.count()), 0); } void SystemTimer::onTimeout() @@ -101,7 +98,7 @@ namespace sys::timer return; } log_debug("Timer %s runs callback", name.c_str()); - if (type == timer::Type::SingleShot) { + if (type == Type::SingleShot) { stop(); } callback(*this); @@ -112,7 +109,7 @@ namespace sys::timer return active; } - void SystemTimer::connect(timer::TimerCallback &&newCallback) noexcept + void SystemTimer::connect(TimerCallback &&newCallback) noexcept { callback = std::move(newCallback); } diff --git a/module-sys/Service/include/Timers/SystemTimer.hpp b/module-sys/Service/include/Timers/SystemTimer.hpp index 8bf1bbcb1553e5e7688d5aa160956aaa3584fd2e..b1ac558605606d04def2970a12c08998ed4575f8 100644 --- a/module-sys/Service/include/Timers/SystemTimer.hpp +++ b/module-sys/Service/include/Timers/SystemTimer.hpp @@ -4,11 +4,11 @@ #pragma once #include "FreeRTOS.h" -#include "portmacro.h" // for TickType_t -#include // for Timer +#include "portmacro.h" #include -#include // for function -#include // for string +#include +#include +#include #include namespace sys @@ -26,7 +26,7 @@ namespace sys::timer /// @param name this will be name of timer + postfix /// @param interval time for next timer event in /// @param type type of timer - SystemTimer(Service *parent, const std::string &name, std::chrono::milliseconds interval, timer::Type type); + SystemTimer(Service *parent, const std::string &name, std::chrono::milliseconds interval, Type type); SystemTimer(const SystemTimer &) = delete; SystemTimer(SystemTimer &&) noexcept = delete; SystemTimer &operator=(const SystemTimer &) = delete; @@ -36,10 +36,10 @@ namespace sys::timer void start() override; void restart(std::chrono::milliseconds newInterval) override; void stop() override; - bool isActive() const noexcept override; + [[nodiscard]] bool isActive() const noexcept override; void setInterval(std::chrono::milliseconds value); - void connect(timer::TimerCallback &&newCallback) noexcept; + void connect(TimerCallback &&newCallback) noexcept; void onTimeout(); private: @@ -52,9 +52,9 @@ namespace sys::timer void attachToService(); std::string name; - timer::TimerCallback callback; + TimerCallback callback; std::chrono::milliseconds interval; - timer::Type type; + Type type; Service *parent = nullptr; std::atomic_bool active = false; };