From f75f5bd6877d716c35e099f7ebb29fb4a22307f5 Mon Sep 17 00:00:00 2001
From: thodkatz <thodkatz@gmail.com>
Date: Thu, 14 Mar 2019 17:50:19 +0200
Subject: [PATCH] Implement code review suggestions
---
.idea/codeStyles/Project.xml | 19 +++++++++
inc/Helpers/TimeHelper.hpp | 59 ++++++++++++++------------
inc/Services/TimeManagementService.hpp | 11 ++---
src/Helpers/TimeHelper.cpp | 22 +++-------
src/Services/TimeManagementService.cpp | 8 +++-
test/Helpers/TimeHelper.cpp | 4 +-
6 files changed, 70 insertions(+), 53 deletions(-)
diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml
index f25060c0..86ab48c8 100644
--- a/.idea/codeStyles/Project.xml
+++ b/.idea/codeStyles/Project.xml
@@ -3,6 +3,25 @@
<option name="RIGHT_MARGIN" value="100" />
<option name="WRAP_WHEN_TYPING_REACHES_RIGHT_MARGIN" value="true" />
<Objective-C-extensions>
+ <file>
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="Import" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="Macro" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="Typedef" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="Enum" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="Constant" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="Global" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="Struct" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="FunctionPredecl" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="Function" />
+ </file>
+ <class>
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="Property" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="Synthesize" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="InitMethod" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="StaticMethod" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="InstanceMethod" />
+ <option name="com.jetbrains.cidr.lang.util.OCDeclarationKind" value="DeallocMethod" />
+ </class>
<extensions>
<pair source="cpp" header="hpp" fileNamingConvention="PASCAL_CASE" />
<pair source="c" header="h" fileNamingConvention="NONE" />
diff --git a/inc/Helpers/TimeHelper.hpp b/inc/Helpers/TimeHelper.hpp
index 8ce7707a..099f3fcc 100644
--- a/inc/Helpers/TimeHelper.hpp
+++ b/inc/Helpers/TimeHelper.hpp
@@ -5,9 +5,9 @@
#include <Message.hpp>
/**
- * The time and date provided from Real Time Clock(Real Time Clock)
+ * The time and date provided from Real Time Clock(Real Time Clock).
*
- * Note:
+ * @notes
* This struct is similar to the `struct tm` of <ctime> library but it is more embedded-friendly
*
* For the current implementation this struct takes dummy values, because RTC hasn't been
@@ -24,11 +24,12 @@ struct TimeAndDate {
/**
* This class formats the spacecraft time and cooperates closely with the ST[09] time management.
+ *
* The ECSS standard supports two time formats: the CUC and CSD that are described in
* CCSDS 301.0-B-4 standard. The chosen time format is CDS and it is UTC-based(UTC: Coordinated
* Universal Time)
*
- * Note:
+ * @note
* Since this code is UTC-based, the leap second correction must be made. The leap seconds that
* have been occurred between timestamps should be considered if a critical time-difference is
* needed
@@ -36,23 +37,24 @@ struct TimeAndDate {
*/
class TimeHelper {
private:
- uint8_t SecondsPerMinute = 60;
- uint16_t SecondsPerHour = 3600;
- uint32_t SecondsPerDay = 86400;
- uint8_t DaysOfMonth[12] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
+ const uint8_t SecondsPerMinute = 60;
+ const uint16_t SecondsPerHour = 3600;
+ const uint32_t SecondsPerDay = 86400;
+ const uint8_t DaysOfMonth[12] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
/**
* @param year The year that will be examined if it is a leap year(366 days)
- * @return if the /p is a leap year returns true and if it isn't returns false
+ * @return if the \p year is a leap year returns true and if it isn't returns false
*/
bool IsLeapYear(uint16_t year);
/**
- * Convert UTC date to elapsed seconds since Unix epoch(1/1/1970 00:00:00). This is a
- * reimplemented mktime() of <ctime> library in an embedded systems way
+ * Convert UTC date to elapsed seconds since Unix epoch(1/1/1970 00:00:00).
+ *
+ * This is a reimplemented mktime() of <ctime> library in an embedded systems way
*
- * Note:
- * This function can convert UTC dates after 1 January 2019
+ * @note
+ * This function can convert UTC dates after 1 January 2019 to elapsed seconds since Unix epoch
*
* @param TimeInfo the time information/data from the RTC(UTC format)
* @return the elapsed seconds between a given UTC date(after the Unix epoch) and Unix epoch
@@ -62,14 +64,15 @@ private:
uint32_t mkUTCtime(struct TimeAndDate &TimeInfo);
/**
- * Convert elapsed seconds since Unix epoch to UTC date. This is a reimplemented gmtime() of
- * <ctime> library in an embedded systems way
+ * Convert elapsed seconds since Unix epoch to UTC date.
+ *
+ * This is a reimplemented gmtime() of <ctime> library in an embedded systems way
*
- * Note:
- * This function can convert elapsed seconds after 1 January 2019 since Unix epoch
+ * @note
+ * This function can convert elapsed seconds since Unix epoch to UTC dates after 1 January 2019
*
* @param seconds elapsed seconds since Unix epoch
- * @return the UTC date based on the /p
+ * @return the UTC date based on the \p seconds
* @todo check if we need to change the epoch to ,the recommended from the standard, 1 January
* 1958
*/
@@ -80,28 +83,30 @@ public:
TimeHelper() = default;
/**
- * Generate the CDS time format(3.3 in CCSDS 301.0-B-4 standard)
+ * Generate the CDS time format(3.3 in CCSDS 301.0-B-4 standard).
*
- * @details The CDS time format consists of two main fields: the time code preamble field
- * (P-field) and the time specification field(T-field). The P-Field is the metadata for the
- * T-Field. The T-Field is consisted of two segments: 1)the `DAY` and the 2)`ms of
- * day` segments. The P-field won't be included in the code, because as the ECSS standards
- * claims, it can be just implicitly declared.
+ * The CDS time format consists of two main fields: the time code preamble field(P-field) and
+ * the time specification field(T-field). The P-Field is the metadata for the T-Field. The
+ * T-Field is consisted of two segments: 1)the `DAY` and the 2)`ms of day` segments. The
+ * P-field won't be included in the code, because as the ECSS standards claims, it can be
+ * just implicitly declared.
* @param TimeInfo is the data provided from RTC(Real Time Clock)
+ * @return TimeFormat the CDS time format. More specific, 48 bits are used for the T-field
+ * (16 for the `DAY` and 32 for the `ms of day`)
* @todo time security for critical time operations
* @todo declare the implicit P-field
* @todo check if we need milliseconds
*/
- static uint64_t implementCDStimeFormat(struct TimeAndDate &TimeInfo);
+ static uint64_t generateCDStimeFormat(struct TimeAndDate &TimeInfo);
/**
* Parse the CDS time format(3.3 in CCSDS 301.0-B-4 standard)
*
- * @param data time information provided from the ground segment
- * @param length the size of the time information
+ * @param data time information provided from the ground segment. The length of the data is a
+ * fixed size of 48 bits
* @return the UTC date
*/
- static struct TimeAndDate parseCDStimeFormat(const uint8_t *data, uint8_t length);
+ static struct TimeAndDate parseCDStimeFormat(const uint8_t *data);
/**
* Dummy function created only to access `mkUTCtime` for testing
diff --git a/inc/Services/TimeManagementService.hpp b/inc/Services/TimeManagementService.hpp
index da58e67d..6f504425 100644
--- a/inc/Services/TimeManagementService.hpp
+++ b/inc/Services/TimeManagementService.hpp
@@ -6,9 +6,9 @@
#include "Helpers/TimeHelper.hpp"
/**
- * Implementation of the ST[09] time management
+ * Implementation of the ST[09] time management.
*
- * Notes:
+ * @notes
* There is a noticeable difference between setting the time using GPS and setting the time
* using space packets from the ground segment. The GPS module sent the actual time of UTC(123519
* is 12:35:19 UTC),while space packets,for time configuration,sent the elapsed time units
@@ -35,7 +35,7 @@ public:
}
/**
- * TM[9,3] CDS time report
+ * TM[9,3] CDS time report.
*
* This function sends reports with the spacecraft time that is formatted according to the CDS
* time code format(check class `TimeHelper` for the format)
@@ -51,14 +51,15 @@ public:
void cdsTimeReport(struct TimeAndDate &TimeInfo);
/**
- * TC[9,128] CDS time request
+ * TC[9,128] CDS time request.
*
* This function is a custom subservice(mission specific) with message type 128(as defined
* from the standard for custom message types, 5.3.3.1.f) and it parses the data of the
* time-management telecommand packet. This data is formatted according to the CDS time code
* format(check class `TimeHelper` for the format)
*
- * @param message the Message that will be parsed for its time-data
+ * @param message the message that will be parsed for its time-data. The data of the \p message
+ * should be a fixed size of 48 bits
*/
struct TimeAndDate cdsTimeRequest(Message &message);
};
diff --git a/src/Helpers/TimeHelper.cpp b/src/Helpers/TimeHelper.cpp
index a6ba81b3..81aa12a2 100644
--- a/src/Helpers/TimeHelper.cpp
+++ b/src/Helpers/TimeHelper.cpp
@@ -11,8 +11,8 @@ bool TimeHelper::IsLeapYear(uint16_t year) {
}
uint32_t TimeHelper::mkUTCtime(struct TimeAndDate &TimeInfo) {
- uint32_t secs = 1546300800; // elapsed seconds since 1/1/2019 00:00:00 (UTC date)
- for (uint16_t y = 2019; y < TimeInfo.year; ++y) { //
+ uint32_t secs = 1546300800; // elapsed seconds from Unix epoch until 1/1/2019 00:00:00(UTC date)
+ for (uint16_t y = 2019; y < TimeInfo.year; ++y) {
secs += (IsLeapYear(y) ? 366 : 365) * SecondsPerDay;
}
for (uint16_t m = 1; m < TimeInfo.month; ++m) {
@@ -29,7 +29,7 @@ uint32_t TimeHelper::mkUTCtime(struct TimeAndDate &TimeInfo) {
}
struct TimeAndDate TimeHelper::utcTime(uint32_t seconds) {
- seconds -= 1546300800; // elapsed seconds between 1/1/2019 00:00:00 (UTC date)
+ seconds -= 1546300800; // elapsed seconds from Unix epoch until 1/1/2019 00:00:00(UTC date)
struct TimeAndDate TimeInfo = {0};
TimeInfo.year = 2019;
TimeInfo.month = 1;
@@ -79,7 +79,7 @@ struct TimeAndDate TimeHelper::utcTime(uint32_t seconds) {
return TimeInfo;
}
-uint64_t TimeHelper::implementCDStimeFormat(struct TimeAndDate &TimeInfo) {
+uint64_t TimeHelper::generateCDStimeFormat(struct TimeAndDate &TimeInfo) {
/**
* Define the T-field. The total number of octets for the implementation of T-field is 6(2 for
* the `DAY` and 4 for the `ms of day`
@@ -100,24 +100,12 @@ uint64_t TimeHelper::implementCDStimeFormat(struct TimeAndDate &TimeInfo) {
*/
auto msOfDay = static_cast<uint32_t >((seconds % 86400) * 1000);
- /**
- * Define CDS time format
- *
- * Notes:
- * Only the 48 bits of the 64 will be used for the timeFormat
- *
- * 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 = (static_cast<uint64_t>(elapsedDays) << 32 | msOfDay);
return timeFormat;
}
-struct TimeAndDate TimeHelper::parseCDStimeFormat(const uint8_t *data, uint8_t length) {
- // check if we have the correct length of the packet data
- assertI(length != 48, ErrorHandler::InternalErrorType::UnknownInternalError);
-
+struct TimeAndDate TimeHelper::parseCDStimeFormat(const uint8_t *data) {
uint16_t elapsedDays = (static_cast<uint16_t >(data[0])) << 8 | static_cast<uint16_t >
(data[1]);
uint32_t msOfDay = (static_cast<uint32_t >(data[2])) << 24 |
diff --git a/src/Services/TimeManagementService.cpp b/src/Services/TimeManagementService.cpp
index cf409508..33301a70 100644
--- a/src/Services/TimeManagementService.cpp
+++ b/src/Services/TimeManagementService.cpp
@@ -5,7 +5,7 @@ void TimeManagementService::cdsTimeReport(struct TimeAndDate &TimeInfo) {
Message timeReport = createTM(3);
- uint64_t timeFormat = TimeHelper::implementCDStimeFormat(TimeInfo);
+ uint64_t timeFormat = TimeHelper::generateCDStimeFormat(TimeInfo);
timeReport.appendHalfword(static_cast<uint16_t >(timeFormat >> 32));
timeReport.appendWord(static_cast<uint32_t >(timeFormat));
@@ -16,7 +16,11 @@ void TimeManagementService::cdsTimeReport(struct TimeAndDate &TimeInfo) {
struct TimeAndDate TimeManagementService::cdsTimeRequest(Message &message) {
// TC{9,128] CDS time request
- struct TimeAndDate timeInfo = TimeHelper::parseCDStimeFormat(message.data, 48);
+ // check if we have the correct size of the data. The size should be 6(48 bits)
+ ErrorHandler::assertRequest(message.dataSize == 6, message,
+ ErrorHandler::AcceptanceErrorType::UnacceptableMessage);
+
+ struct TimeAndDate timeInfo = TimeHelper::parseCDStimeFormat(message.data);
return timeInfo;
}
diff --git a/test/Helpers/TimeHelper.cpp b/test/Helpers/TimeHelper.cpp
index 8b5b21e8..cc124eae 100644
--- a/test/Helpers/TimeHelper.cpp
+++ b/test/Helpers/TimeHelper.cpp
@@ -19,7 +19,7 @@ TEST_CASE("Time format implementation", "[CUC]") {
uint16_t elapsedDays = currTime / 86400;
uint32_t msOfDay = currTime % 86400 * 1000;
uint64_t timeFormat = (static_cast<uint64_t>(elapsedDays) << 32 | msOfDay);
- CHECK(TimeHelper::implementCDStimeFormat(TimeInfo) == timeFormat);
+ CHECK(TimeHelper::generateCDStimeFormat(TimeInfo) == timeFormat);
// 1/1/2019 00:00:00
TimeInfo.year = 2019;
@@ -34,7 +34,7 @@ TEST_CASE("Time format implementation", "[CUC]") {
elapsedDays = currTime / 86400;
msOfDay = currTime % 86400 * 1000;
timeFormat = (static_cast<uint64_t>(elapsedDays) << 32 | msOfDay);
- CHECK(TimeHelper::implementCDStimeFormat(TimeInfo) == timeFormat);
+ CHECK(TimeHelper::generateCDStimeFormat(TimeInfo) == timeFormat);
}
SECTION("Convert elapsed seconds since Unix epoch to UTC date"){
--
GitLab