Merge branch 'development' into mohr/timetag_fix

CHANGELOG.md

@@ -10,16 +10,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Fixes
 
-- Important bugfix in CFDP PDU header format: The entity length field and the transaction sequence
-  number fields stored the actual length of the field instead of the length minus 1 like specified
-  in the CFDP standard.
 - PUS Health Service: Size check for set health command.
-  Perform operation completion for announce health command.
+                      Perform operation completion for announce health command.
   https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/746
 - Linux OSAL `getUptime` fix: Check validity of `/proc/uptime` file before reading uptime.
   https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/745
 - Small tweak for version getter
   https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/744
+- PUS Scheduling Service: Use timeval internally
+  PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/735
 
 ## Added
 

src/fsfw/cfdp/pdu/HeaderCreator.cpp

@@ -24,8 +24,8 @@ ReturnValue_t HeaderCreator::serialize(uint8_t **buffer, size_t *size, size_t ma
   *buffer += 1;
   **buffer = pduDataFieldLen & 0x00ff;
   *buffer += 1;
-  **buffer = segmentationCtrl << 7 | ((pduConf.sourceId.getWidth() - 1) << 4) |
-             segmentMetadataFlag << 3 | (pduConf.seqNum.getWidth() - 1);
+  **buffer = segmentationCtrl << 7 | pduConf.sourceId.getWidth() << 4 | segmentMetadataFlag << 3 |
+             pduConf.seqNum.getWidth();
   *buffer += 1;
   *size += 4;
   ReturnValue_t result = pduConf.sourceId.serialize(buffer, size, maxSize, streamEndianness);

src/fsfw/cfdp/pdu/HeaderReader.cpp

@@ -78,11 +78,11 @@ cfdp::SegmentationControl PduHeaderReader::getSegmentationControl() const {
 }
 
 cfdp::WidthInBytes PduHeaderReader::getLenEntityIds() const {
-  return static_cast<cfdp::WidthInBytes>(((pointers.fixedHeader->fourthByte >> 4) & 0b111) + 1);
+  return static_cast<cfdp::WidthInBytes>((pointers.fixedHeader->fourthByte >> 4) & 0x07);
 }
 
 cfdp::WidthInBytes PduHeaderReader::getLenSeqNum() const {
-  return static_cast<cfdp::WidthInBytes>((pointers.fixedHeader->fourthByte & 0b111) + 1);
+  return static_cast<cfdp::WidthInBytes>(pointers.fixedHeader->fourthByte & 0x07);
 }
 
 cfdp::SegmentMetadataFlag PduHeaderReader::getSegmentMetadataFlag() const {

src/fsfw/pus/Service11TelecommandScheduling.h

@@ -91,7 +91,6 @@ class Service11TelecommandScheduling final : public PusServiceBase {
  private:
   struct TelecommandStruct {
     uint64_t requestId{};
-    uint32_t seconds{};
     store_address_t storeAddr;  // uint16
   };
 
@@ -114,7 +113,7 @@ class Service11TelecommandScheduling final : public PusServiceBase {
    * The telecommand map uses the exectution time as a Unix time stamp as
    * the key. This is mapped to a generic telecommand struct.
    */
-  using TelecommandMap = etl::multimap<uint32_t, TelecommandStruct, MAX_NUM_TCS>;
+  using TelecommandMap = etl::multimap<timeval, TelecommandStruct, MAX_NUM_TCS>;
   using TcMapIter = typename TelecommandMap::iterator;
 
   TelecommandMap telecommandMap;

src/fsfw/pus/Service11TelecommandScheduling.tpp

@@ -3,6 +3,7 @@
 #include <cstddef>
 
 #include "fsfw/globalfunctions/CRC.h"
+#include "fsfw/globalfunctions/timevalOperations.h"
 #include "fsfw/objectmanager/ObjectManager.h"
 #include "fsfw/serialize/SerializeAdapter.h"
 #include "fsfw/serviceinterface.h"
@@ -79,7 +80,7 @@ inline ReturnValue_t Service11TelecommandScheduling<MAX_NUM_TCS>::performService
   // NOTE: The iterator is increased in the loop here. Increasing the iterator as for-loop arg
   // does not work in this case as we are deleting the current element here.
   for (auto it = telecommandMap.begin(); it != telecommandMap.end();) {
-    if (it->first <= tNow.tv_sec) {
+    if (it->first <= tNow) {
       if (schedulingEnabled) {
         // release tc
         TmTcMessage releaseMsg(it->second.storeAddr);
@@ -150,17 +151,25 @@ inline ReturnValue_t Service11TelecommandScheduling<MAX_NUM_TCS>::handleResetCom
 template <size_t MAX_NUM_TCS>
 inline ReturnValue_t Service11TelecommandScheduling<MAX_NUM_TCS>::doInsertActivity(
     const uint8_t *data, size_t size) {
-  uint32_t timestamp = 0;
-  ReturnValue_t result = SerializeAdapter::deSerialize(&timestamp, &data, &size, DEF_END);
+  uint32_t seconds = 0;
+  ReturnValue_t result = SerializeAdapter::deSerialize(&seconds, &data, &size, DEF_END);
   if (result != returnvalue::OK) {
     return result;
   }
+  timeval scheduledTime;
+  scheduledTime.tv_sec = seconds;
+  scheduledTime.tv_usec = 0;
 
   // Insert possible if sched. time is above margin
   // (See requirement for Time margin)
   timeval tNow = {};
   Clock::getClock_timeval(&tNow);
-  if (timestamp - tNow.tv_sec <= RELEASE_TIME_MARGIN_SECONDS) {
+  timeval timeDifference = scheduledTime - tNow;
+  // round subseconds up
+  if (timeDifference.tv_usec != 0) {
+    timeDifference.tv_sec++;
+  }
+  if (timeDifference.tv_sec <= RELEASE_TIME_MARGIN_SECONDS) {
 #if FSFW_CPP_OSTREAM_ENABLED == 1
     sif::warning << "Service11TelecommandScheduling::doInsertActivity: Release time too close to "
                     "current time"
@@ -197,11 +206,10 @@ inline ReturnValue_t Service11TelecommandScheduling<MAX_NUM_TCS>::doInsertActivi
 
   // insert into multimap with new store address
   TelecommandStruct tc;
-  tc.seconds = timestamp;
   tc.storeAddr = addr;
   tc.requestId = getRequestIdFromTc();  // TODO: Missing sanity check of the returned request id
 
-  auto it = telecommandMap.insert(std::pair<uint32_t, TelecommandStruct>(timestamp, tc));
+  auto it = telecommandMap.insert(std::pair<timeval, TelecommandStruct>(scheduledTime, tc));
   if (it == telecommandMap.end()) {
     return returnvalue::FAILED;
   }
@@ -391,7 +399,8 @@ inline ReturnValue_t Service11TelecommandScheduling<MAX_NUM_TCS>::doTimeshiftAct
 
   // temporarily hold the item
   TelecommandStruct tempTc(tcToTimeshiftIt->second);
-  uint32_t tempKey = tcToTimeshiftIt->first + relativeTime;
+  timeval tempKey = tcToTimeshiftIt->first;
+  tempKey.tv_sec += relativeTime;
 
   // delete old entry from the mm
   telecommandMap.erase(tcToTimeshiftIt);
@@ -436,7 +445,8 @@ inline ReturnValue_t Service11TelecommandScheduling<MAX_NUM_TCS>::doFilterTimesh
   for (auto it = itBegin; it != itEnd;) {
     // temporarily hold the item
     TelecommandStruct tempTc(it->second);
-    uint32_t tempKey = it->first + relativeTime;
+    timeval tempKey = it->first;
+    tempKey.tv_sec += relativeTime;
 
     // delete the old entry from the mm
     telecommandMap.erase(it++);
@@ -533,12 +543,16 @@ inline ReturnValue_t Service11TelecommandScheduling<MAX_NUM_TCS>::getMapFilterFr
     }
 
     case TypeOfTimeWindow::FROM_TIMETAG: {
-      uint32_t fromTimestamp = 0;
-      result = SerializeAdapter::deSerialize(&fromTimestamp, &data, &dataSize, DEF_END);
+      uint32_t fromSeconds = 0;
+      result = SerializeAdapter::deSerialize(&fromSeconds, &data, &dataSize, DEF_END);
       if (result != returnvalue::OK) {
         return result;
       }
 
+      timeval fromTimestamp;
+      fromTimestamp.tv_sec = fromSeconds;
+      fromTimestamp.tv_usec = 0;
+
       itBegin = telecommandMap.begin();
       while (itBegin->first < fromTimestamp && itBegin != telecommandMap.end()) {
         itBegin++;
@@ -549,11 +563,15 @@ inline ReturnValue_t Service11TelecommandScheduling<MAX_NUM_TCS>::getMapFilterFr
     }
 
     case TypeOfTimeWindow::TO_TIMETAG: {
-      uint32_t toTimestamp;
-      result = SerializeAdapter::deSerialize(&toTimestamp, &data, &dataSize, DEF_END);
+      uint32_t toSeconds = 0;
+      result = SerializeAdapter::deSerialize(&toSeconds, &data, &dataSize, DEF_END);
       if (result != returnvalue::OK) {
         return result;
       }
+      timeval toTimestamp;
+      toTimestamp.tv_sec = toSeconds;
+      toTimestamp.tv_usec = 0;
+
       itBegin = telecommandMap.begin();
       itEnd = telecommandMap.begin();
       while (itEnd->first <= toTimestamp && itEnd != telecommandMap.end()) {
@@ -563,19 +581,27 @@ inline ReturnValue_t Service11TelecommandScheduling<MAX_NUM_TCS>::getMapFilterFr
     }
 
     case TypeOfTimeWindow::FROM_TIMETAG_TO_TIMETAG: {
-      uint32_t fromTimestamp;
-      uint32_t toTimestamp;
+      uint32_t fromSeconds = 0;
+      uint32_t toSeconds = 0;
 
-      result = SerializeAdapter::deSerialize(&fromTimestamp, &data, &dataSize,
+      result = SerializeAdapter::deSerialize(&fromSeconds, &data, &dataSize,
                                              SerializeIF::Endianness::BIG);
       if (result != returnvalue::OK) {
         return result;
       }
-      result = SerializeAdapter::deSerialize(&toTimestamp, &data, &dataSize,
+      result = SerializeAdapter::deSerialize(&toSeconds, &data, &dataSize,
                                              SerializeIF::Endianness::BIG);
       if (result != returnvalue::OK) {
         return result;
       }
+      timeval fromTimestamp;
+      fromTimestamp.tv_sec = fromSeconds;
+      fromTimestamp.tv_usec = 0;
+      timeval toTimestamp;
+      toTimestamp.tv_sec = toSeconds;
+      toTimestamp.tv_usec = 0;
+
+
       if (fromTimestamp > toTimestamp) {
         return INVALID_TIME_WINDOW;
       }

unittests/cfdp/pdu/testCfdpHeader.cpp

@@ -97,7 +97,7 @@ TEST_CASE("CFDP Header", "[cfdp]") {
       REQUIRE(creator.serialize(&serTarget, &serSize, serBuf.size(),
                                 SerializeIF::Endianness::BIG) == returnvalue::OK);
       CHECK(serBuf[0] == 0x3f);
-      CHECK(serBuf[3] == 0x88);
+      CHECK(serBuf[3] == 0x99);
       REQUIRE(creator.getCrcFlag() == true);
       REQUIRE(creator.getDirection() == cfdp::Direction::TOWARDS_SENDER);
       REQUIRE(creator.getLargeFileFlag() == true);
@@ -127,7 +127,7 @@ TEST_CASE("CFDP Header", "[cfdp]") {
       REQUIRE(creator.getTransmissionMode() == cfdp::TransmissionMode::UNACKNOWLEDGED);
       REQUIRE(creator.getSegmentationControl() == true);
       // Last three bits are 2 now (length of seq number) and bit 1 to bit 3 is 4 (len entity IDs)
-      REQUIRE(serBuf[3] == 0b10111001);
+      REQUIRE(serBuf[3] == 0b11001010);
       uint32_t entityId = 0;
       size_t deSerSize = 0;
       SerializeAdapter::deSerialize(&entityId, serBuf.data() + 4, &deSerSize,
@@ -175,7 +175,7 @@ TEST_CASE("CFDP Header", "[cfdp]") {
     REQUIRE(serBuf[1] == 0);
     REQUIRE(serBuf[2] == 0);
     // Entity and Transaction Sequence number are 1 byte large
-    REQUIRE(serBuf[3] == 0b00000000);
+    REQUIRE(serBuf[3] == 0b00010001);
     // Source ID
     REQUIRE(serBuf[4] == 0);
     // Transaction Seq Number
@@ -220,7 +220,7 @@ TEST_CASE("CFDP Header", "[cfdp]") {
     REQUIRE(serBuf[1] == 0);
     REQUIRE(serBuf[2] == 0);
     // Entity and Transaction Sequence number are 1 byte large
-    REQUIRE(serBuf[3] == 0b00000000);
+    REQUIRE(serBuf[3] == 0b00010001);
     REQUIRE(serSize == 7);
     // Deser call not strictly necessary
     auto reader = PduHeaderReader(serBuf.data(), serBuf.size());
@@ -270,7 +270,7 @@ TEST_CASE("CFDP Header", "[cfdp]") {
     REQUIRE(reader.parseData() == returnvalue::OK);
     // Everything except version bit flipped to one now
     REQUIRE(serBuf[0] == 0x3f);
-    REQUIRE(serBuf[3] == 0b10111001);
+    REQUIRE(serBuf[3] == 0b11001010);
     REQUIRE(reader.getWholePduSize() == 14);
 
     REQUIRE(reader.getCrcFlag() == true);

unittests/cfdp/pdu/testFileData.cpp

@@ -68,7 +68,7 @@ TEST_CASE("File Data PDU", "[cfdp][pdu]") {
     // Bits 1 to 3 length of enitity IDs is 2
     // Bit 4: Segment metadata flag is set
     // Bit 5 to seven: length of transaction seq num is 2
-    REQUIRE(fileDataBuffer[3] == 0b10011001);
+    REQUIRE(fileDataBuffer[3] == 0b10101010);
     REQUIRE((fileDataBuffer[10] >> 6) &
             0b11 == cfdp::RecordContinuationState::CONTAINS_START_AND_END);
     // Segment metadata length

unittests/cfdp/pdu/testFileDirective.cpp

@@ -30,7 +30,7 @@ TEST_CASE("CFDP File Directive", "[cfdp][pdu]") {
     REQUIRE(serBuf[1] == 0);
     REQUIRE(serBuf[2] == 5);
     // Entity and Transaction Sequence number are 1 byte large
-    REQUIRE(serBuf[3] == 0b00000000);
+    REQUIRE(serBuf[3] == 0b00010001);
     // Source ID
     REQUIRE(serBuf[4] == 0);
     // Transaction Seq Number
@@ -82,4 +82,4 @@ TEST_CASE("CFDP File Directive", "[cfdp][pdu]") {
     // Invalid file directive
     REQUIRE(fdDeser.parseData() == cfdp::INVALID_DIRECTIVE_FIELD);
   }
-}
+}

unittests/cfdp/testCfdp.cpp

@@ -33,8 +33,8 @@ TEST_CASE("CFDP Base", "[cfdp]") {
     // PDU data field length is 5 (4 + Directive code octet)
     REQUIRE(serBuf[1] == 0);
     REQUIRE(serBuf[2] == 5);
-    // Entity and Transaction Sequence number are 1 byte large, value minus one is stored
-    REQUIRE(serBuf[3] == 0b00000000);
+    // Entity and Transaction Sequence number are 1 byte large
+    REQUIRE(serBuf[3] == 0b00010001);
     // Source ID
     REQUIRE(serBuf[4] == 0);
     // Transaction Seq Number