Merge pull request 'Increased TM stack robustness' (#501) from eive/fsfw:mueller/tm-stack-robustness-cherry-picked into master

Reviewed-on: fsfw/fsfw#501
Reviewed-by: Ulrich Mohr <mohr@irs.uni-stuttgart.de>

src/fsfw/tmtcpacket/SpacePacket.h

@@ -19,7 +19,8 @@ public:
 	/**
 	 * The constructor initializes the packet and sets all header information
 	 * according to the passed parameters.
-	 * @param packetDataLength Sets the packet data length field and therefore specifies the size of the packet.
+	 * @param packetDataLength Sets the packet data length field and therefore specifies
+	 * the size of the packet.
 	 * @param isTelecommand Sets the packet type field to either TC (true) or TM (false).
 	 * @param apid Sets the packet's APID field. The default value describes an idle packet.
 	 * @param sequenceCount ets the packet's Source Sequence Count field.

src/fsfw/tmtcpacket/SpacePacketBase.cpp

@@ -3,8 +3,8 @@
 
 #include <cstring>
 
-SpacePacketBase::SpacePacketBase( const uint8_t* set_address ) {
+SpacePacketBase::SpacePacketBase(const uint8_t* setAddress) {
-	this->data = (SpacePacketPointer*) set_address;
+    this->data = reinterpret_cast<SpacePacketPointer*>(const_cast<uint8_t*>(setAddress));
 }
 
 SpacePacketBase::~SpacePacketBase() {
@@ -15,10 +15,21 @@ uint8_t SpacePacketBase::getPacketVersionNumber( void ) {
     return (this->data->header.packet_id_h & 0b11100000) >> 5;
 }
 
-void SpacePacketBase::initSpacePacketHeader(bool isTelecommand,
+ReturnValue_t SpacePacketBase::initSpacePacketHeader(bool isTelecommand,
         bool hasSecondaryHeader, uint16_t apid, uint16_t sequenceCount) {
+    if(data == nullptr) {
+#if FSFW_VERBOSE_LEVEL >= 1
+#if FSFW_CPP_OSTREAM_ENABLED == 1
+        sif::warning << "SpacePacketBase::initSpacePacketHeader: Data pointer is invalid"
+                << std::endl;
+#else
+        sif::printWarning("SpacePacketBase::initSpacePacketHeader: Data pointer is invalid!\n");
+#endif
+#endif
+        return HasReturnvaluesIF::RETURN_FAILED;
+    }
     //reset header to zero:
-	memset(data,0, sizeof(this->data->header) );
+    memset(data, 0, sizeof(this->data->header) );
     //Set TC/TM bit.
     data->header.packet_id_h = ((isTelecommand? 1 : 0)) << 4;
     //Set secondaryHeader bit
@@ -27,7 +38,7 @@ void SpacePacketBase::initSpacePacketHeader(bool isTelecommand,
     //Always initialize as standalone packets.
     data->header.sequence_control_h = 0b11000000;
     setPacketSequenceCount(sequenceCount);
-
+    return HasReturnvaluesIF::RETURN_OK;
 }
 
 bool SpacePacketBase::isTelecommand( void ) {
@@ -49,11 +60,17 @@ uint16_t SpacePacketBase::getAPID( void ) const {
 }
 
 void SpacePacketBase::setAPID( uint16_t new_apid ) {
-	//Use first three bits of new APID, but keep rest of packet id as it was (see specification).
+    // Use first three bits of new APID, but keep rest of packet id as it was (see specification).
-	this->data->header.packet_id_h = (this->data->header.packet_id_h & 0b11111000) | ( ( new_apid & 0x0700 ) >> 8 );
+    this->data->header.packet_id_h = (this->data->header.packet_id_h & 0b11111000) |
+            ( ( new_apid & 0x0700 ) >> 8 );
     this->data->header.packet_id_l = ( new_apid & 0x00FF );
 }
 
+void SpacePacketBase::setSequenceFlags( uint8_t sequenceflags ) {
+    this->data->header.sequence_control_h &= 0x3F;
+    this->data->header.sequence_control_h |= sequenceflags << 6;
+}
+
 uint16_t SpacePacketBase::getPacketSequenceControl( void ) {
     return ( (this->data->header.sequence_control_h) << 8 )
             + this->data->header.sequence_control_l;
@@ -69,7 +86,8 @@ uint16_t SpacePacketBase::getPacketSequenceCount( void ) const {
 }
 
 void SpacePacketBase::setPacketSequenceCount( uint16_t new_count) {
-	this->data->header.sequence_control_h = ( this->data->header.sequence_control_h & 0b11000000 ) | ( ( (new_count%LIMIT_SEQUENCE_COUNT) & 0x3F00 ) >> 8 );
+    this->data->header.sequence_control_h = ( this->data->header.sequence_control_h & 0b11000000 ) |
+            ( ( (new_count%LIMIT_SEQUENCE_COUNT) & 0x3F00 ) >> 8 );
     this->data->header.sequence_control_l = ( (new_count%LIMIT_SEQUENCE_COUNT) & 0x00FF );
 }
 
@@ -84,7 +102,7 @@ void SpacePacketBase::setPacketDataLength( uint16_t new_length) {
 }
 
 size_t SpacePacketBase::getFullSize() {
-	//+1 is done because size in packet data length field is: size of data field -1
+    // +1 is done because size in packet data length field is: size of data field -1
     return this->getPacketDataLength() + sizeof(this->data->header) + 1;
 }
 

src/fsfw/tmtcpacket/SpacePacketBase.h

@@ -2,6 +2,8 @@
 #define FSFW_TMTCPACKET_SPACEPACKETBASE_H_
 
 #include "ccsds_header.h"
+#include "fsfw/returnvalues/HasReturnvaluesIF.h"
+
 #include <cstddef>
 
 /**
@@ -68,7 +70,8 @@ public:
      */
     virtual ~SpacePacketBase();
 
-	//CCSDS Methods:
+    //CCSDS Methods
+
     /**
      * Getter for the packet version number field.
      * @return Returns the highest three bit of the packet in one byte.
@@ -82,7 +85,7 @@ public:
      */
     bool isTelecommand( void );
 
-	void initSpacePacketHeader(bool isTelecommand, bool hasSecondaryHeader,
+    ReturnValue_t initSpacePacketHeader(bool isTelecommand, bool hasSecondaryHeader,
             uint16_t apid, uint16_t sequenceCount = 0);
     /**
      * The CCSDS header provides a secondary header flag (the fifth-highest bit),
@@ -109,6 +112,13 @@ public:
      * 			ignored.
      */
     void setAPID( uint16_t setAPID );
+
+    /**
+     * Sets the sequence flags of a packet, which are bit 17 and 18 in the space packet header.
+     * @param   The sequence flags to set
+     */
+    void setSequenceFlags( uint8_t sequenceflags );
+
     /**
      * Returns the CCSDS packet sequence control field, which are the third and
      * the fourth byte of the CCSDS primary header.
@@ -153,7 +163,7 @@ public:
      */
     void setPacketDataLength( uint16_t setLength );
 
-	//Helper methods:
+    // Helper methods
     /**
      * This method returns a raw uint8_t pointer to the packet.
      * @return	A \c uint8_t pointer to the first byte of the CCSDS primary header.

src/fsfw/tmtcpacket/pus/tm/TmPacketPusC.cpp

@@ -54,11 +54,14 @@ uint8_t* TmPacketPusC::getPacketTimeRaw() const{
 
 }
 
-void TmPacketPusC::initializeTmPacket(uint16_t apid, uint8_t service,
+ReturnValue_t TmPacketPusC::initializeTmPacket(uint16_t apid, uint8_t service,
         uint8_t subservice, uint16_t packetSubcounter, uint16_t destinationId,
         uint8_t timeRefField) {
     //Set primary header:
-    initSpacePacketHeader(false, true, apid);
+    ReturnValue_t result = initSpacePacketHeader(false, true, apid);
+    if(result != HasReturnvaluesIF::RETURN_OK) {
+        return result;
+    }
     //Set data Field Header:
     //First, set to zero.
     memset(&tmData->dataField, 0, sizeof(tmData->dataField));
@@ -78,6 +81,7 @@ void TmPacketPusC::initializeTmPacket(uint16_t apid, uint8_t service,
         timeStamper->addTimeStamp(tmData->dataField.time,
                 sizeof(tmData->dataField.time));
     }
+    return HasReturnvaluesIF::RETURN_OK;
 }
 
 void TmPacketPusC::setSourceDataSize(uint16_t size) {

src/fsfw/tmtcpacket/pus/tm/TmPacketPusC.h

@@ -100,7 +100,7 @@ protected:
      * @param subservice PUS Subservice
      * @param packetSubcounter Additional subcounter used.
      */
-    void initializeTmPacket(uint16_t apid, uint8_t service, uint8_t subservice,
+    ReturnValue_t initializeTmPacket(uint16_t apid, uint8_t service, uint8_t subservice,
             uint16_t packetSubcounter, uint16_t destinationId = 0, uint8_t timeRefField = 0);
 
     /**

src/fsfw/tmtcpacket/pus/tm/TmPacketStoredBase.cpp

@@ -91,4 +91,34 @@ void TmPacketStoredBase::checkAndReportLostTm() {
     }
 }
 
-
+void TmPacketStoredBase::handleStoreFailure(const char *const packetType, ReturnValue_t result,
+        size_t sizeToReserve) {
+    checkAndReportLostTm();
+#if FSFW_VERBOSE_LEVEL >= 1
+    switch(result) {
+#if FSFW_CPP_OSTREAM_ENABLED == 1
+    case(StorageManagerIF::DATA_STORAGE_FULL): {
+        sif::warning << "TmPacketStoredPus" << packetType << ": " <<
+                "Store full for packet with size" << sizeToReserve << std::endl;
+        break;
+    }
+    case(StorageManagerIF::DATA_TOO_LARGE): {
+        sif::warning << "TmPacketStoredPus" << packetType << ": Data with size " <<
+                sizeToReserve << " too large" <<  std::endl;
+        break;
+    }
+#else
+    case(StorageManagerIF::DATA_STORAGE_FULL): {
+        sif::printWarning("TmPacketStoredPus%s: Store full for packet with "
+                "size %d\n", packetType, sizeToReserve);
+        break;
+    }
+    case(StorageManagerIF::DATA_TOO_LARGE): {
+        sif::printWarning("TmPacketStoredPus%s: Data with size "
+                "%d too large\n", packetType, sizeToReserve);
+        break;
+    }
+#endif
+#endif
+    }
+}

src/fsfw/tmtcpacket/pus/tm/TmPacketStoredBase.h

@@ -82,6 +82,9 @@ protected:
     bool checkAndSetStore();
 
     void checkAndReportLostTm();
+
+    void handleStoreFailure(const char* const packetType, ReturnValue_t result,
+            size_t sizeToReserve);
 };
 
 

src/fsfw/tmtcpacket/pus/tm/TmPacketStoredPusA.cpp

@@ -5,24 +5,25 @@
 
 #include <cstring>
 
-TmPacketStoredPusA::TmPacketStoredPusA(store_address_t setAddress) :
+TmPacketStoredPusA::TmPacketStoredPusA(store_address_t setAddress):
         TmPacketStoredBase(setAddress), TmPacketPusA(nullptr){
 }
 
 TmPacketStoredPusA::TmPacketStoredPusA(uint16_t apid, uint8_t service,
         uint8_t subservice, uint8_t packetSubcounter, const uint8_t *data,
-		uint32_t size, const uint8_t *headerData, uint32_t headerSize) :
+        uint32_t size, const uint8_t *headerData, uint32_t headerSize):
 		TmPacketPusA(nullptr) {
     storeAddress.raw = StorageManagerIF::INVALID_ADDRESS;
     if (not TmPacketStoredBase::checkAndSetStore()) {
         return;
     }
     uint8_t *pData = nullptr;
+    size_t sizeToReserve = getPacketMinimumSize() + size + headerSize;
     ReturnValue_t returnValue = store->getFreeElement(&storeAddress,
-			(getPacketMinimumSize() + size + headerSize), &pData);
+            sizeToReserve, &pData);
 
     if (returnValue != store->RETURN_OK) {
-	    TmPacketStoredBase::checkAndReportLostTm();
+        handleStoreFailure("A", returnValue, sizeToReserve);
         return;
     }
     setData(pData);
@@ -41,32 +42,33 @@ TmPacketStoredPusA::TmPacketStoredPusA(uint16_t apid, uint8_t service,
         return;
     }
     size_t sourceDataSize = 0;
-	if (content != NULL) {
+    if (content != nullptr) {
         sourceDataSize += content->getSerializedSize();
     }
-	if (header != NULL) {
+    if (header != nullptr) {
         sourceDataSize += header->getSerializedSize();
     }
-	uint8_t *p_data = NULL;
+    uint8_t *pData = nullptr;
+    size_t sizeToReserve = getPacketMinimumSize() + sourceDataSize;
     ReturnValue_t returnValue = store->getFreeElement(&storeAddress,
-			(getPacketMinimumSize() + sourceDataSize), &p_data);
+            sizeToReserve, &pData);
     if (returnValue != store->RETURN_OK) {
-	    TmPacketStoredBase::checkAndReportLostTm();
+        handleStoreFailure("A", returnValue, sizeToReserve);
+        return;
     }
-	setData(p_data);
+    setData(pData);
     initializeTmPacket(apid, service, subservice, packetSubcounter);
     uint8_t *putDataHere = getSourceData();
     size_t size = 0;
-	if (header != NULL) {
+    if (header != nullptr) {
         header->serialize(&putDataHere, &size, sourceDataSize,
                 SerializeIF::Endianness::BIG);
     }
-	if (content != NULL) {
+    if (content != nullptr) {
         content->serialize(&putDataHere, &size, sourceDataSize,
                 SerializeIF::Endianness::BIG);
     }
-	setPacketDataLength(
+    setPacketDataLength(sourceDataSize + sizeof(PUSTmDataFieldHeaderPusA) + CRC_SIZE - 1);
-			sourceDataSize + sizeof(PUSTmDataFieldHeaderPusA) + CRC_SIZE - 1);
 }
 
 uint8_t* TmPacketStoredPusA::getAllTmData() {

src/fsfw/tmtcpacket/pus/tm/TmPacketStoredPusA.h

@@ -15,7 +15,7 @@
  *	packets in a store with the help of a storeAddress.
  *	@ingroup tmtcpackets
  */
-class TmPacketStoredPusA :
+class TmPacketStoredPusA:
         public TmPacketStoredBase,
         public TmPacketPusA {
 public:

src/fsfw/tmtcpacket/pus/tm/TmPacketStoredPusC.cpp

@@ -19,11 +19,12 @@ TmPacketStoredPusC::TmPacketStoredPusC(uint16_t apid, uint8_t service,
         return;
     }
     uint8_t *pData = nullptr;
+    size_t sizeToReserve = getPacketMinimumSize() + size + headerSize;
     ReturnValue_t returnValue = store->getFreeElement(&storeAddress,
-            (getPacketMinimumSize() + size + headerSize), &pData);
+            sizeToReserve, &pData);
 
     if (returnValue != store->RETURN_OK) {
-        TmPacketStoredBase::checkAndReportLostTm();
+        handleStoreFailure("C", returnValue, sizeToReserve);
         return;
     }
     setData(pData);
@@ -42,27 +43,28 @@ TmPacketStoredPusC::TmPacketStoredPusC(uint16_t apid, uint8_t service,
         return;
     }
     size_t sourceDataSize = 0;
-    if (content != NULL) {
+    if (content != nullptr) {
         sourceDataSize += content->getSerializedSize();
     }
-    if (header != NULL) {
+    if (header != nullptr) {
         sourceDataSize += header->getSerializedSize();
     }
-    uint8_t *p_data = NULL;
+    uint8_t *pData = nullptr;
-    ReturnValue_t returnValue = store->getFreeElement(&storeAddress,
+    size_t sizeToReserve = getPacketMinimumSize() + sourceDataSize;
-            (getPacketMinimumSize() + sourceDataSize), &p_data);
+    ReturnValue_t returnValue = store->getFreeElement(&storeAddress, sizeToReserve, &pData);
     if (returnValue != store->RETURN_OK) {
-        TmPacketStoredBase::checkAndReportLostTm();
+        handleStoreFailure("C", returnValue, sizeToReserve);
+        return;
     }
-    setData(p_data);
+    setData(pData);
     initializeTmPacket(apid, service, subservice, packetSubcounter, destinationId, timeRefField);
     uint8_t *putDataHere = getSourceData();
     size_t size = 0;
-    if (header != NULL) {
+    if (header != nullptr) {
         header->serialize(&putDataHere, &size, sourceDataSize,
                 SerializeIF::Endianness::BIG);
     }
-    if (content != NULL) {
+    if (content != nullptr) {
         content->serialize(&putDataHere, &size, sourceDataSize,
                 SerializeIF::Endianness::BIG);
     }