From eca4e15547a941ba1a66416808c39872fce7b8f7 Mon Sep 17 00:00:00 2001
From: thodkatz <thodkatz@gmail.com>
Date: Fri, 14 Dec 2018 23:53:16 +0200
Subject: [PATCH] Implement code review suggestions
---
inc/Services/TimeManagementService.hpp | 6 ++----
src/Helpers/TimeHelper.cpp | 10 +++++++---
src/Services/TimeManagementService.cpp | 14 +++++++++++---
src/main.cpp | 3 ++-
test/Services/TimeManagementService.cpp | 17 ++++-------------
5 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/inc/Services/TimeManagementService.hpp b/inc/Services/TimeManagementService.hpp
index 8b1a31fc..d006a5d7 100644
--- a/inc/Services/TimeManagementService.hpp
+++ b/inc/Services/TimeManagementService.hpp
@@ -2,6 +2,7 @@
#define ECSS_SERVICES_TIMEMANAGEMENTSERVICE_HPP
#include <Service.hpp>
+#include <ctime>
#include "Helpers/TimeHelper.hpp"
/**
@@ -23,16 +24,13 @@ public:
/**
* TM[9,2] CUC time report
*
- * @param seconds the seconds provided from the RTC. This function in general should have
- * parameters corresponding with the RTC. For the time being we assume that the RTC has a
- * 32-bit counter that counts seconds(the RTC in Nucleo F103RB!)
* @todo check if we need spacecraft time reference status
* @todo ECSS standard claims: <<The time reports generated by the time reporting subservice
* are spacecraft time packets. A spacecraft time packet does not carry the message type,
* consisting of the service type and message subtype.>> Check if we need to implement that
* or should ignore the standard?
*/
- void cucTimeReport(uint32_t seconds);
+ void cucTimeReport();
};
diff --git a/src/Helpers/TimeHelper.cpp b/src/Helpers/TimeHelper.cpp
index f8e5c158..462c7e8b 100644
--- a/src/Helpers/TimeHelper.cpp
+++ b/src/Helpers/TimeHelper.cpp
@@ -30,10 +30,14 @@ uint64_t TimeHelper::implementCUCTimeFormat(uint32_t seconds) {
/**
* Define time format including P-field and T-Field
*
- * Note: Only the 40 bits of the 64 will be used for the timeFormat(0-7 : P-field, 8-39:
- * T-field)
+ * Notes:
+ * Only the 40 bits of the 64 will be used for the timeFormat(0-7 : P-field, 8-39: T-field)
+ *
+ * Shift operators have high priority. That's why we should do a type-casting first so we
+ * don't lose valuable bits
*/
- uint64_t timeFormat = (totalSeconds << 8 | pField);
+ uint64_t timeFormat = (static_cast<uint64_t>(totalSeconds) << 8 | pField);
+
return timeFormat;
}
diff --git a/src/Services/TimeManagementService.cpp b/src/Services/TimeManagementService.cpp
index b2ad7ebd..2b3feb68 100644
--- a/src/Services/TimeManagementService.cpp
+++ b/src/Services/TimeManagementService.cpp
@@ -1,12 +1,20 @@
#include "Services/TimeManagementService.hpp"
-void TimeManagementService::cucTimeReport(uint32_t seconds) {
+void TimeManagementService::cucTimeReport() {
// TM[9,2] CUC time report
Message timeReport = createTM(2);
- timeReport.appendByte(TimeHelper::implementCUCTimeFormat(seconds)); // append the P-field
- timeReport.appendWord(TimeHelper::implementCUCTimeFormat(seconds) >> 8); // append the T-field
+ /**
+ * For the time being we will use C++ functions to get a time value, but this will change
+ * when the RTC will be implemented
+ */
+ uint32_t seconds;
+ seconds = time(nullptr); // seconds have passed since 00:00:00 GMT, Jan 1, 1970
+ uint64_t timeFormat = TimeHelper::implementCUCTimeFormat(seconds); // store the return value
+
+ timeReport.appendByte(timeFormat); // append the P-field
+ timeReport.appendWord(timeFormat >> 8); // append the T-field
storeMessage(timeReport);
}
diff --git a/src/main.cpp b/src/main.cpp
index 46b0eb38..23501aef 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -164,6 +164,7 @@ int main() {
// ST[09] test
TimeManagementService timeReport;
- timeReport.cucTimeReport(60);
+ timeReport.cucTimeReport();
+
return 0;
}
diff --git a/test/Services/TimeManagementService.cpp b/test/Services/TimeManagementService.cpp
index 5bdec1b4..714c99e1 100644
--- a/test/Services/TimeManagementService.cpp
+++ b/test/Services/TimeManagementService.cpp
@@ -5,21 +5,12 @@
TEST_CASE("TM[9,2]", "[service][st09]") {
TimeManagementService timeFormat;
- uint32_t seconds = 60;
- timeFormat.cucTimeReport(seconds);
- Message response = ServiceTests::get(0);
- CHECK(response.readByte() == 50);
- CHECK(response.readWord() == seconds);
+ uint32_t seconds;
+ seconds = time(nullptr);
- seconds = 100;
- timeFormat.cucTimeReport(seconds);
- response = ServiceTests::get(1);
+ timeFormat.cucTimeReport();
+ Message response = ServiceTests::get(0);
CHECK(response.readByte() == 50);
CHECK(response.readWord() == seconds);
- seconds = 1000;
- timeFormat.cucTimeReport(seconds);
- response = ServiceTests::get(2);
- CHECK(response.readByte() == 50);
- CHECK(response.readWord() == seconds);
}
--
GitLab