From 10f2b49d3f3696f367bccfc0fd7b687feee7c9e2 Mon Sep 17 00:00:00 2001 From: M Starch Date: Tue, 2 Sep 2025 15:22:07 -0700 Subject: [PATCH] Modify LinuxTimer interface for consistency (#4087) * Use Fw::TimeInterval in LinuxTimer * Fix Linux FD typo * Fix interval timer UT * Format * Fix assert casts * Fix ComLogger UTs * Fix FppTest microseconds * Fix casting * Fix overflow --- .../active/test/ut/ActiveTestTester.cpp | 2 +- .../passive/test/ut/PassiveTestTester.cpp | 2 +- .../queued/test/ut/QueuedTestTester.cpp | 2 +- .../FppTest/component/tests/TimeTests.cpp | 2 +- Fw/Time/Time.cpp | 2 ++ Fw/Time/TimeInterval.cpp | 6 +++++- Ref/Top/RefTopology.cpp | 4 ++-- Ref/Top/RefTopology.hpp | 2 +- Svc/ComLogger/test/ut/ComLoggerTester.cpp | 18 +++++++++--------- Svc/LinuxTimer/LinuxTimer.hpp | 3 ++- Svc/LinuxTimer/LinuxTimerFd.cpp | 16 ++++++++-------- Svc/LinuxTimer/LinuxTimerTaskDelay.cpp | 6 ++---- Svc/LinuxTimer/test/ut/LinuxTimerTester.cpp | 2 +- 13 files changed, 36 insertions(+), 31 deletions(-) diff --git a/FppTestProject/FppTest/component/active/test/ut/ActiveTestTester.cpp b/FppTestProject/FppTest/component/active/test/ut/ActiveTestTester.cpp index 20b2848ee2..1081656d5a 100644 --- a/FppTestProject/FppTest/component/active/test/ut/ActiveTestTester.cpp +++ b/FppTestProject/FppTest/component/active/test/ut/ActiveTestTester.cpp @@ -20,7 +20,7 @@ ActiveTestTester ::ActiveTestTester() arrayBuf(arrayData, sizeof(arrayData)), structBuf(structData, sizeof(structData)), serialBuf(serialData, sizeof(serialData)), - time(STest::Pick::any(), STest::Pick::any()) { + time(STest::Pick::any(), STest::Pick::lowerUpper(0, 999999)) { this->initComponents(); this->connectPorts(); this->connectAsyncPorts(); diff --git a/FppTestProject/FppTest/component/passive/test/ut/PassiveTestTester.cpp b/FppTestProject/FppTest/component/passive/test/ut/PassiveTestTester.cpp index a94858909a..02460e0064 100644 --- a/FppTestProject/FppTest/component/passive/test/ut/PassiveTestTester.cpp +++ b/FppTestProject/FppTest/component/passive/test/ut/PassiveTestTester.cpp @@ -20,7 +20,7 @@ PassiveTestTester ::PassiveTestTester() arrayBuf(arrayData, sizeof(arrayData)), structBuf(structData, sizeof(structData)), serialBuf(serialData, sizeof(serialData)), - time(STest::Pick::any(), STest::Pick::any()) { + time(STest::Pick::any(), STest::Pick::lowerUpper(0, 999999)) { this->initComponents(); this->connectPorts(); this->component.registerExternalParameters(&this->paramTesterDelegate); diff --git a/FppTestProject/FppTest/component/queued/test/ut/QueuedTestTester.cpp b/FppTestProject/FppTest/component/queued/test/ut/QueuedTestTester.cpp index 5ed4c0fd67..63b623abe6 100644 --- a/FppTestProject/FppTest/component/queued/test/ut/QueuedTestTester.cpp +++ b/FppTestProject/FppTest/component/queued/test/ut/QueuedTestTester.cpp @@ -20,7 +20,7 @@ QueuedTestTester ::QueuedTestTester() arrayBuf(arrayData, sizeof(arrayData)), structBuf(structData, sizeof(structData)), serialBuf(serialData, sizeof(serialData)), - time(STest::Pick::any(), STest::Pick::any()) { + time(STest::Pick::any(), STest::Pick::lowerUpper(0, 999999)) { this->initComponents(); this->connectPorts(); this->connectAsyncPorts(); diff --git a/FppTestProject/FppTest/component/tests/TimeTests.cpp b/FppTestProject/FppTest/component/tests/TimeTests.cpp index bffea2cfab..05703765a6 100644 --- a/FppTestProject/FppTest/component/tests/TimeTests.cpp +++ b/FppTestProject/FppTest/component/tests/TimeTests.cpp @@ -19,7 +19,7 @@ // ---------------------------------------------------------------------- void Tester ::testTime() { - Fw::Time random_time(STest::Pick::any(), STest::Pick::any()); + Fw::Time random_time(STest::Pick::any(), STest::Pick::lowerUpper(0, 999999)); Fw::Time zero_time(TimeBase::TB_NONE, 0, 0); Fw::Time result; diff --git a/Fw/Time/Time.cpp b/Fw/Time/Time.cpp index 6d072fa7a8..acd47686f3 100644 --- a/Fw/Time/Time.cpp +++ b/Fw/Time/Time.cpp @@ -39,6 +39,8 @@ Time::Time(TimeBase timeBase, FwTimeContextStoreType context, U32 seconds, U32 u } void Time::set(TimeBase timeBase, FwTimeContextStoreType context, U32 seconds, U32 useconds) { + // Assert microseconds portion is less than 10^6 + FW_ASSERT(useconds < 1000000, static_cast(useconds)); this->m_val.set(timeBase, context, seconds, useconds); } diff --git a/Fw/Time/TimeInterval.cpp b/Fw/Time/TimeInterval.cpp index d480acfe6f..e351b195e5 100644 --- a/Fw/Time/TimeInterval.cpp +++ b/Fw/Time/TimeInterval.cpp @@ -11,6 +11,8 @@ TimeInterval::TimeInterval(U32 seconds, U32 useconds) : Serializable() { } void TimeInterval::set(U32 seconds, U32 useconds) { + // Assert microseconds portion is less than 10^6 + FW_ASSERT(useconds < 1000000, static_cast(useconds)); this->m_val.set(seconds, useconds); } @@ -121,6 +123,8 @@ void TimeInterval::add(U32 seconds, U32 useconds) { newSeconds += 1; newUSeconds -= 1000000; } + // Assert microseconds portion is less than 10^6 + FW_ASSERT(newUSeconds < 1000000, static_cast(newUSeconds)); this->m_val.set(newSeconds, newUSeconds); } @@ -131,4 +135,4 @@ std::ostream& operator<<(std::ostream& os, const TimeInterval& val) { } #endif -} // namespace Fw \ No newline at end of file +} // namespace Fw diff --git a/Ref/Top/RefTopology.cpp b/Ref/Top/RefTopology.cpp index b32f1b631e..7d88746ed5 100644 --- a/Ref/Top/RefTopology.cpp +++ b/Ref/Top/RefTopology.cpp @@ -85,12 +85,12 @@ void setupTopology(const TopologyState& state) { } } -void startRateGroups(Fw::TimeInterval interval) { +void startRateGroups(const Fw::TimeInterval& interval) { // This timer drives the fundamental tick rate of the system. // Svc::RateGroupDriver will divide this down to the slower rate groups. // This call will block until the stopRateGroups() call is made. // For this Linux demo, that call is made from a signal handler. - linuxTimer.startTimer(interval.getSeconds()*1000+interval.getUSeconds()/1000); + linuxTimer.startTimer(interval); } void stopRateGroups() { diff --git a/Ref/Top/RefTopology.hpp b/Ref/Top/RefTopology.hpp index 9fd39628d8..f083a0ca28 100644 --- a/Ref/Top/RefTopology.hpp +++ b/Ref/Top/RefTopology.hpp @@ -77,7 +77,7 @@ void teardownTopology(const TopologyState& state); * This loop is stopped via a stopRateGroups call. * */ -void startRateGroups(Fw::TimeInterval interval); +void startRateGroups(const Fw::TimeInterval& interval); /** * \brief stop the rate groups diff --git a/Svc/ComLogger/test/ut/ComLoggerTester.cpp b/Svc/ComLogger/test/ut/ComLoggerTester.cpp index 59f0e605e9..62749715b0 100644 --- a/Svc/ComLogger/test/ut/ComLoggerTester.cpp +++ b/Svc/ComLogger/test/ut/ComLoggerTester.cpp @@ -76,9 +76,9 @@ void ComLoggerTester ::testLogging() { for (int j = 0; j < 3; j++) { // Test times for the different iterations: - Fw::Time testTime(TimeBase::TB_NONE, j, 9876543); - Fw::Time testTimePrev(TimeBase::TB_NONE, j - 1, 9876543); - Fw::Time testTimeNext(TimeBase::TB_NONE, j + 1, 9876543); + Fw::Time testTime(TimeBase::TB_NONE, j, 987654); + Fw::Time testTimePrev(TimeBase::TB_NONE, j - 1, 987654); + Fw::Time testTimeNext(TimeBase::TB_NONE, j + 1, 987654); // File names for the different iterations: memset(fileName, 0, sizeof(fileName)); @@ -312,7 +312,7 @@ void ComLoggerTester ::openError() { const U8 data[COM_BUFFER_LENGTH] = {0xde, 0xad, 0xbe, 0xef}; Fw::ComBuffer buffer(data, sizeof(data)); - Fw::Time testTime(TimeBase::TB_NONE, 4, 9876543); + Fw::Time testTime(TimeBase::TB_NONE, 4, 987654); setTestTime(testTime); snprintf(fileName, sizeof(fileName), "%s_%d_%d_%06d.com", filePrefix, static_cast(testTime.getTimeBase()), @@ -384,7 +384,7 @@ void ComLoggerTester ::writeError() { const U8 data[4] = {0xde, 0xad, 0xbe, 0xef}; Fw::ComBuffer buffer(data, sizeof(data)); - Fw::Time testTime(TimeBase::TB_NONE, 5, 9876543); + Fw::Time testTime(TimeBase::TB_NONE, 5, 987654); setTestTime(testTime); for (int i = 0; i < 3; i++) { @@ -457,7 +457,7 @@ void ComLoggerTester ::closeFileCommand() { Os::File::Status ret; // Form filenames: - Fw::Time testTime(TimeBase::TB_NONE, 6, 9876543); + Fw::Time testTime(TimeBase::TB_NONE, 6, 987654); setTestTime(testTime); memset(fileName, 0, sizeof(fileName)); snprintf(fileName, sizeof(fileName), "%s_%d_%d_%06d.com", FILE_STR, @@ -541,9 +541,9 @@ void ComLoggerTester ::testLoggingWithInit() { for (int j = 0; j < 3; j++) { // Test times for the different iterations: - Fw::Time testTime(TimeBase::TB_NONE, j, 9876543); - Fw::Time testTimePrev(TimeBase::TB_NONE, j - 1, 9876543); - Fw::Time testTimeNext(TimeBase::TB_NONE, j + 1, 9876543); + Fw::Time testTime(TimeBase::TB_NONE, j, 987654); + Fw::Time testTimePrev(TimeBase::TB_NONE, j - 1, 987654); + Fw::Time testTimeNext(TimeBase::TB_NONE, j + 1, 987654); // File names for the different iterations: memset(fileName, 0, sizeof(fileName)); diff --git a/Svc/LinuxTimer/LinuxTimer.hpp b/Svc/LinuxTimer/LinuxTimer.hpp index 0acd5d9d5a..33c4e232db 100644 --- a/Svc/LinuxTimer/LinuxTimer.hpp +++ b/Svc/LinuxTimer/LinuxTimer.hpp @@ -13,6 +13,7 @@ #ifndef LinuxTimer_HPP #define LinuxTimer_HPP +#include "Fw/Time/TimeInterval.hpp" #include "Os/Mutex.hpp" #include "Os/RawTime.hpp" #include "Svc/LinuxTimer/LinuxTimerComponentAc.hpp" @@ -35,7 +36,7 @@ class LinuxTimer final : public LinuxTimerComponentBase { ~LinuxTimer(); //! Start timer - void startTimer(FwSizeType interval); //!< interval in milliseconds + void startTimer(const Fw::TimeInterval& interval); //!< interval in milliseconds //! Quit timer void quit(); diff --git a/Svc/LinuxTimer/LinuxTimerFd.cpp b/Svc/LinuxTimer/LinuxTimerFd.cpp index 2514bcf1cf..2d7c738edb 100644 --- a/Svc/LinuxTimer/LinuxTimerFd.cpp +++ b/Svc/LinuxTimer/LinuxTimerFd.cpp @@ -20,19 +20,19 @@ namespace Svc { -void LinuxTimer::startTimer(FwSizeType interval) { +void LinuxTimer::startTimer(const Fw::TimeInterval& interval) { int fd; struct itimerspec itval; /* Create the timer */ fd = timerfd_create(CLOCK_MONOTONIC, 0); - const FwSizeType interval_secs = interval / 1000; - FW_ASSERT(static_cast(std::numeric_limits::max()) >= interval_secs, - static_cast(interval)); - itval.it_interval.tv_sec = static_cast(interval_secs); - itval.it_interval.tv_nsec = static_cast((interval * 1000000) % 1000000000); - itval.it_value.tv_sec = static_cast(interval_secs); - itval.it_value.tv_nsec = static_cast((interval * 1000000) % 1000000000); + time_t seconds_value = static_cast(interval.getSeconds()); + // Ensure an overflow did not occur + FW_ASSERT(seconds_value == interval.getSeconds()); + itval.it_interval.tv_sec = static_cast(seconds_value); + itval.it_interval.tv_nsec = static_cast(interval.getUSeconds() * 1000); + itval.it_value.tv_sec = static_cast(seconds_value); + itval.it_value.tv_nsec = static_cast(interval.getUSeconds() * 1000); timerfd_settime(fd, 0, &itval, nullptr); diff --git a/Svc/LinuxTimer/LinuxTimerTaskDelay.cpp b/Svc/LinuxTimer/LinuxTimerTaskDelay.cpp index f1cbd28b07..889243b155 100644 --- a/Svc/LinuxTimer/LinuxTimerTaskDelay.cpp +++ b/Svc/LinuxTimer/LinuxTimerTaskDelay.cpp @@ -16,11 +16,9 @@ namespace Svc { -void LinuxTimer::startTimer(FwSizeType interval) { - FW_ASSERT(std::numeric_limits::max() / 1000 >= interval); // Overflow +void LinuxTimer::startTimer(const Fw::TimeInterval& interval) { while (true) { - Os::Task::delay( - Fw::TimeInterval(static_cast(interval / 1000), static_cast((interval % 1000) * 1000))); + Os::Task::delay(interval); this->m_mutex.lock(); bool quit = this->m_quit; this->m_mutex.unLock(); diff --git a/Svc/LinuxTimer/test/ut/LinuxTimerTester.cpp b/Svc/LinuxTimer/test/ut/LinuxTimerTester.cpp index 138ad64337..8502b7ddc5 100644 --- a/Svc/LinuxTimer/test/ut/LinuxTimerTester.cpp +++ b/Svc/LinuxTimer/test/ut/LinuxTimerTester.cpp @@ -35,7 +35,7 @@ LinuxTimerTester ::~LinuxTimerTester() {} void LinuxTimerTester ::runCycles() { this->m_numCalls = 5; - this->component.startTimer(1000); + this->component.startTimer(Fw::TimeInterval(1, 0)); } // ----------------------------------------------------------------------