Added more unittest coverage

Added Mutex for gmtime functions
Moved Statics used in ClockCommon to ClockCommon

src/fsfw/osal/freertos/Clock.cpp

@@ -11,9 +11,6 @@
 // TODO sanitize input?
 // TODO much of this code can be reused for tick-only systems
 
-uint16_t Clock::leapSeconds = 0;
-MutexIF* Clock::timeMutex = nullptr;
-
 uint32_t Clock::getTicksPerSecond(void) { return 1000; }
 
 ReturnValue_t Clock::setClock(const TimeOfDay_t* time) {

src/fsfw/osal/host/Clock.cpp

@@ -4,6 +4,7 @@
 
 #include "fsfw/platform.h"
 #include "fsfw/serviceinterface/ServiceInterface.h"
+#include "fsfw/ipc/MutexGuard.h"
 
 #if defined(PLATFORM_WIN)
 #include <sysinfoapi.h>
@@ -11,9 +12,6 @@
 #include <fstream>
 #endif
 
-uint16_t Clock::leapSeconds = 0;
-MutexIF* Clock::timeMutex = NULL;
-
 using SystemClock = std::chrono::system_clock;
 
 uint32_t Clock::getTicksPerSecond(void) {
@@ -127,6 +125,13 @@ ReturnValue_t Clock::getDateAndTime(TimeOfDay_t* time) {
   auto seconds = std::chrono::time_point_cast<std::chrono::seconds>(now);
   auto fraction = now - seconds;
   time_t tt = SystemClock::to_time_t(now);
+    ReturnValue_t result = checkOrCreateClockMutex();
+  if(result != HasReturnvaluesIF::RETURN_OK){
+    return result;
+  }
+  MutexGuard helper(timeMutex);
+  // gmtime writes its output in a global buffer which is not Thread Safe
+  // Therefore we have to use a Mutex here
   struct tm* timeInfo;
   timeInfo = gmtime(&tt);
   time->year = timeInfo->tm_year + 1900;

src/fsfw/osal/linux/Clock.cpp

@@ -9,9 +9,7 @@
 #include <fstream>
 
 #include "fsfw/serviceinterface/ServiceInterface.h"
-
+#include "fsfw/ipc/MutexGuard.h"
-uint16_t Clock::leapSeconds = 0;
-MutexIF* Clock::timeMutex = NULL;
 
 uint32_t Clock::getTicksPerSecond(void) {
   uint32_t ticks = sysconf(_SC_CLK_TCK);
@@ -117,7 +115,13 @@ ReturnValue_t Clock::getDateAndTime(TimeOfDay_t* time) {
     // TODO errno
     return HasReturnvaluesIF::RETURN_FAILED;
   }
-
+  ReturnValue_t result = checkOrCreateClockMutex();
+  if(result != HasReturnvaluesIF::RETURN_OK){
+    return result;
+  }
+  MutexGuard helper(timeMutex);
+  // gmtime writes its output in a global buffer which is not Thread Safe
+  // Therefore we have to use a Mutex here
   struct tm* timeInfo;
   timeInfo = gmtime(&timeUnix.tv_sec);
   time->year = timeInfo->tm_year + 1900;

src/fsfw/osal/rtems/Clock.cpp

@@ -6,8 +6,6 @@
 #include "fsfw/ipc/MutexGuard.h"
 #include "fsfw/osal/rtems/RtemsBasic.h"
 
-uint16_t Clock::leapSeconds = 0;
-MutexIF* Clock::timeMutex = nullptr;
 
 uint32_t Clock::getTicksPerSecond(void) {
   rtems_interval ticks_per_second = rtems_clock_get_ticks_per_second();

src/fsfw/timemanager/Clock.h

@@ -173,6 +173,7 @@ class Clock {
 
   static MutexIF *timeMutex;
   static uint16_t leapSeconds;
+  static bool leapSecondsSet;
 };
 
 #endif /* FSFW_TIMEMANAGER_CLOCK_H_ */

src/fsfw/timemanager/ClockCommon.cpp

@@ -3,6 +3,10 @@
 #include "fsfw/ipc/MutexGuard.h"
 #include "fsfw/timemanager/Clock.h"
 
+uint16_t Clock::leapSeconds = 0;
+MutexIF* Clock::timeMutex = nullptr;
+bool Clock::leapSecondsSet = false;
+
 ReturnValue_t Clock::convertUTCToTT(timeval utc, timeval* tt) {
   uint16_t leapSeconds;
   ReturnValue_t result = getLeapSeconds(&leapSeconds);
@@ -29,12 +33,16 @@ ReturnValue_t Clock::setLeapSeconds(const uint16_t leapSeconds_) {
   MutexGuard helper(timeMutex);
 
   leapSeconds = leapSeconds_;
+  leapSecondsSet = true;
 
   return HasReturnvaluesIF::RETURN_OK;
 }
 
 ReturnValue_t Clock::getLeapSeconds(uint16_t* leapSeconds_) {
-  if (timeMutex == nullptr) {
+  if(not leapSecondsSet){
+    return HasReturnvaluesIF::RETURN_FAILED;
+  }
+  if (checkOrCreateClockMutex() != HasReturnvaluesIF::RETURN_OK) {
     return HasReturnvaluesIF::RETURN_FAILED;
   }
   MutexGuard helper(timeMutex);
@@ -49,6 +57,13 @@ ReturnValue_t Clock::convertTimevalToTimeOfDay(const timeval* from, TimeOfDay_t*
   // According to https://en.cppreference.com/w/c/chrono/gmtime, the implementation of gmtime_s
   // in the Windows CRT is incompatible with the C standard but this should not be an issue for
   // this implementation
+  ReturnValue_t result = checkOrCreateClockMutex();
+  if(result != HasReturnvaluesIF::RETURN_OK){
+    return result;
+  }
+  MutexGuard helper(timeMutex);
+  // gmtime writes its output in a global buffer which is not Thread Safe
+  // Therefore we have to use a Mutex here
   timeInfo = gmtime(&from->tv_sec);
   to->year = timeInfo->tm_year + 1900;
   to->month = timeInfo->tm_mon + 1;

tests/src/fsfw_tests/unit/globalfunctions/CMakeLists.txt

@@ -3,4 +3,5 @@ target_sources(${FSFW_TEST_TGT} PRIVATE
     testOpDivider.cpp
     testBitutil.cpp
     testCRC.cpp
+    testTimevalOperations.cpp
 )

tests/src/fsfw_tests/unit/globalfunctions/testTimevalOperations.cpp

@@ -0,0 +1,125 @@
+#include <catch2/catch_test_macros.hpp>
+#include <catch2/catch_approx.hpp>
+
+#include <fsfw/globalfunctions/timevalOperations.h>
+
+#include "fsfw_tests/unit/CatchDefinitions.h"
+
+TEST_CASE("TimevalTest", "[timevalOperations]"){
+    SECTION("Comparison"){
+        timeval t1;
+        t1.tv_sec = 1648227422;
+        t1.tv_usec = 123456;
+        timeval t2;
+        t2.tv_sec = 1648227422;
+        t2.tv_usec = 123456;
+        REQUIRE(t1 == t2);
+        REQUIRE(t2 == t1);
+        REQUIRE_FALSE(t1 != t2);
+        REQUIRE_FALSE(t2 != t1);
+        REQUIRE(t1 <= t2);
+        REQUIRE(t2 <= t1);
+        REQUIRE(t1 >= t2);
+        REQUIRE(t2 >= t1);
+        REQUIRE_FALSE(t1 < t2);
+        REQUIRE_FALSE(t2 < t1);
+        REQUIRE_FALSE(t1 > t2);
+        REQUIRE_FALSE(t2 > t1);
+
+        timeval t3;
+        t3.tv_sec = 1648227422;
+        t3.tv_usec = 123457;
+        REQUIRE_FALSE(t1 == t3);
+        REQUIRE(t1 != t3);
+        REQUIRE(t1 <= t3);
+        REQUIRE_FALSE(t3 <= t1);
+        REQUIRE_FALSE(t1 >= t3);
+        REQUIRE(t3 >= t1);
+        REQUIRE(t1 < t3);
+        REQUIRE_FALSE(t3 < t1);
+        REQUIRE_FALSE(t1 > t3);
+        REQUIRE(t3 > t1);
+
+        timeval t4;
+        t4.tv_sec = 1648227423;
+        t4.tv_usec = 123456;
+        REQUIRE_FALSE(t1 == t4);
+        REQUIRE(t1 != t4);
+        REQUIRE(t1 <= t4);
+        REQUIRE_FALSE(t4 <= t1);
+        REQUIRE_FALSE(t1 >= t4);
+        REQUIRE(t4 >= t1);
+        REQUIRE(t1 < t4);
+        REQUIRE_FALSE(t4 < t1);
+        REQUIRE_FALSE(t1 > t4);
+        REQUIRE(t4 > t1);
+    }
+    SECTION("Operators"){
+        timeval t1;
+        t1.tv_sec = 1648227422;
+        t1.tv_usec = 123456;
+        timeval t2;
+        t2.tv_sec = 1648227422;
+        t2.tv_usec = 123456;
+        timeval t3 = t1 - t2;
+        REQUIRE(t3.tv_sec == 0);
+        REQUIRE(t3.tv_usec == 0);
+        timeval t4 = t1 - t3;
+        REQUIRE(t4.tv_sec == 1648227422);
+        REQUIRE(t4.tv_usec == 123456);
+        timeval t5 = t3 - t1;
+        REQUIRE(t5.tv_sec == -1648227422);
+        REQUIRE(t5.tv_usec == -123456);
+
+        timeval t6;
+        t6.tv_sec = 1648227400;
+        t6.tv_usec = 999999;
+
+        timeval t7 = t6 + t1;
+        REQUIRE(t7.tv_sec == (1648227422ull + 1648227400ull + 1ull));
+        REQUIRE(t7.tv_usec == 123455);
+
+        timeval t8 = t1 - t6;
+        REQUIRE(t8.tv_sec == 1648227422 - 1648227400 - 1);
+        REQUIRE(t8.tv_usec == 123457);
+
+        double scalar = 2;
+        timeval t9 = t1 * scalar;
+        REQUIRE(t9.tv_sec == 3296454844);
+        REQUIRE(t9.tv_usec == 246912);
+        timeval t10 = scalar * t1;
+        REQUIRE(t10.tv_sec == 3296454844);
+        REQUIRE(t10.tv_usec == 246912);
+        timeval t11 = t6 * scalar;
+        REQUIRE(t11.tv_sec == (3296454800 + 1));
+        REQUIRE(t11.tv_usec == 999998);
+
+        timeval t12 = t1 / scalar;
+        REQUIRE(t12.tv_sec == 824113711);
+        REQUIRE(t12.tv_usec == 61728);
+
+        timeval t13 = t6 / scalar;
+        REQUIRE(t13.tv_sec == 824113700);
+        // Rounding issue
+        REQUIRE(t13.tv_usec == 499999);
+
+        double scalar2 = t9 / t1;
+        REQUIRE(scalar2 == Catch::Approx(2.0));
+        double scalar3 = t1 / t6;
+        REQUIRE(scalar3 == Catch::Approx(1.000000013));
+        double scalar4 = t3 / t1;
+        REQUIRE(scalar4 == Catch::Approx(0));
+        double scalar5 = t12 / t1;
+        REQUIRE(scalar5 == Catch::Approx(0.5));
+    }
+
+    SECTION("timevalOperations::toTimeval"){
+        double seconds = 1648227422.123456;
+        timeval t1 = timevalOperations::toTimeval(seconds);
+        REQUIRE(t1.tv_sec == 1648227422);
+        // Allow 1 usec rounding tolerance
+        REQUIRE(t1.tv_usec >= 123455);
+        REQUIRE(t1.tv_usec <= 123457);
+    }
+
+}

tests/src/fsfw_tests/unit/osal/CMakeLists.txt

@@ -1,4 +1,5 @@
 target_sources(${FSFW_TEST_TGT} PRIVATE
 	TestMessageQueue.cpp
 	TestSemaphore.cpp
+	TestClock.cpp
 )

tests/src/fsfw_tests/unit/osal/TestClock.cpp

@@ -0,0 +1,92 @@
+#include <fsfw/timemanager/Clock.h>
+#include <fsfw/globalfunctions/timevalOperations.h>
+
+#include <array>
+#include <catch2/catch_test_macros.hpp>
+#include <catch2/catch_approx.hpp>
+
+#include "fsfw_tests/unit/CatchDefinitions.h"
+
+TEST_CASE("OSAL::Clock Test", "[OSAL::Clock Test]"){
+    SECTION("Test getClock"){
+        timeval time;
+        ReturnValue_t result = Clock::getClock_timeval(&time);
+        REQUIRE(result == HasReturnvaluesIF::RETURN_OK);
+        Clock::TimeOfDay_t timeOfDay;
+        result = Clock::getDateAndTime(&timeOfDay);
+        REQUIRE(result == HasReturnvaluesIF::RETURN_OK);
+        timeval timeOfDayAsTimeval;
+        result = Clock::convertTimeOfDayToTimeval(&timeOfDay, &timeOfDayAsTimeval);
+        REQUIRE(result == HasReturnvaluesIF::RETURN_OK);
+        // We require timeOfDayAsTimeval to be larger than time as it 
+        // was request a few ns later
+        double difference = timevalOperations::toDouble(timeOfDayAsTimeval-time);
+        CHECK(difference>=0.0);
+        CHECK(difference<=0.005);
+
+        // Conversion in the other direction
+        Clock::TimeOfDay_t timevalAsTimeOfDay;
+        result = Clock::convertTimevalToTimeOfDay(&time, &timevalAsTimeOfDay);
+        REQUIRE(result == HasReturnvaluesIF::RETURN_OK);
+        CHECK(timevalAsTimeOfDay.year <= timeOfDay.year);
+        // TODO We should write TimeOfDay operators! 
+    }
+    SECTION("Leap seconds"){
+        uint16_t leapSeconds = 0;
+        ReturnValue_t result = Clock::getLeapSeconds(&leapSeconds);
+        REQUIRE(result == HasReturnvaluesIF::RETURN_FAILED);
+        REQUIRE(leapSeconds == 0);
+        result = Clock::setLeapSeconds(18);
+        REQUIRE(result == HasReturnvaluesIF::RETURN_OK);
+        result = Clock::getLeapSeconds(&leapSeconds);
+        REQUIRE(result == HasReturnvaluesIF::RETURN_OK);
+        REQUIRE(leapSeconds == 18);
+    }
+    SECTION("usec Test"){
+        timeval timeAsTimeval;
+        ReturnValue_t result = Clock::getClock_timeval(&timeAsTimeval);
+        REQUIRE(result == HasReturnvaluesIF::RETURN_OK); 
+        uint64_t timeAsUsec = 0;
+        result = Clock::getClock_usecs(&timeAsUsec);
+        REQUIRE(result == HasReturnvaluesIF::RETURN_OK); 
+        double timeAsUsecDouble = static_cast<double>(timeAsUsec)/1000000.0;
+        timeval timeAsUsecTimeval = timevalOperations::toTimeval(timeAsUsecDouble);
+        double difference = timevalOperations::toDouble(timeAsTimeval - timeAsUsecTimeval);
+        // We accept 5 ms difference
+        CHECK(abs(difference) <= 0.005);
+        uint64_t timevalAsUint64 = static_cast<uint64_t>(timeAsTimeval.tv_sec)*1000000ull + static_cast<uint64_t>(timeAsTimeval.tv_usec);
+        if(timeAsUsec > timevalAsUint64){
+            // This should not be the case but we can see some rounding issue sometimes
+            // This is the case if used in valgrind. This might indicate an other issue
+            CHECK((timeAsUsec - timevalAsUint64)>=0);
+            CHECK((timeAsUsec - timevalAsUint64)<=(5*1000));
+        }else{
+            CHECK((timevalAsUint64 - timeAsUsec)>=0);
+            CHECK((timevalAsUint64 - timeAsUsec)<=(5*1000));
+        }
+
+    }
+    SECTION("Test j2000"){
+        double j2000;
+        timeval time;
+        time.tv_sec = 1648208539;
+        time.tv_usec = 0;
+        ReturnValue_t result = Clock::convertTimevalToJD2000(time, &j2000);
+        REQUIRE(result == HasReturnvaluesIF::RETURN_OK); 
+        double correctJ2000 = 2459663.98772 -  2451545.0;
+        CHECK(j2000 == Catch::Approx(correctJ2000).margin(1.2*1e-8));
+    }
+    SECTION("Convert to TT"){
+        timeval utcTime;
+        utcTime.tv_sec = 1648208539;
+        utcTime.tv_usec = 999000;
+        timeval tt;
+        ReturnValue_t result = Clock::setLeapSeconds(27);
+        REQUIRE(result == HasReturnvaluesIF::RETURN_OK);
+        result = Clock::convertUTCToTT(utcTime, &tt);
+        REQUIRE(result == HasReturnvaluesIF::RETURN_OK);
+        CHECK(tt.tv_usec == 183000);
+        // The plus 1 is a own forced overflow of usecs
+        CHECK(tt.tv_sec == (1648208539 + 27 + 10 + 32 + 1));
+    }
+}

tests/src/fsfw_tests/unit/timemanager/TestCCSDSTime.cpp

@@ -120,6 +120,8 @@ TEST_CASE("CCSDSTime Tests", "[TestCCSDSTime]") {
     CHECK(cdsTime.msDay_l == 0xC5);
     CHECK(cdsTime.msDay_ll == 0xC3);
 
+    
+
     // Conversion back to timeval
     timeval timeReturnAsTimeval;
     result = CCSDSTime::convertFromCDS(&timeReturnAsTimeval, &cdsTime);
@@ -128,5 +130,17 @@ TEST_CASE("CCSDSTime Tests", "[TestCCSDSTime]") {
     timeval difference = timeAsTimeval - timeReturnAsTimeval;
     CHECK(difference.tv_usec == 456);
     CHECK(difference.tv_sec == 0);
+
+    Clock::TimeOfDay_t timeReturnAsTimeOfDay;
+    result = CCSDSTime::convertFromCDS(&timeReturnAsTimeOfDay, &cdsTime);
+    CHECK(result == HasReturnvaluesIF::RETURN_OK);
+    CHECK(timeReturnAsTimeOfDay.year == 2020);
+    CHECK(timeReturnAsTimeOfDay.month == 2);
+    CHECK(timeReturnAsTimeOfDay.day == 29);
+    CHECK(timeReturnAsTimeOfDay.hour == 13);
+    CHECK(timeReturnAsTimeOfDay.minute == 24);
+    CHECK(timeReturnAsTimeOfDay.second == 45);
+    // micro seconds precision is lost
+    CHECK(timeReturnAsTimeOfDay.usecond == 123000);
   }
 }