diff --git a/inc/Services/ParameterService.hpp b/inc/Services/ParameterService.hpp
index c331ae2743d7ffbd4e137d043889515f491d0372..5672b3c6bbbc9cca8983937c075f43dae6b733d2 100644
--- a/inc/Services/ParameterService.hpp
+++ b/inc/Services/ParameterService.hpp
@@ -2,9 +2,11 @@
#define ECSS_SERVICES_PARAMETERSERVICE_HPP
#include "Service.hpp"
-// #include "Services/RequestVerificationService.hpp"
+#include "ErrorHandler.hpp"
+#include "etl/map.h"
-#define CONFIGLENGTH 5
+// Number of stored parameters. MAX_PARAMS is just a dummy number for now.
+#define MAX_PARAMS 5
/**
* Implementation of the ST[20] parameter management service,
@@ -18,10 +20,11 @@
* PTC and PFC for each parameter shall be specified as in
* ECSS-E-ST-70-41C, chapter 7.3
*/
+
+typedef uint16_t ParamId; // parameter IDs are given sequentially
struct Parameter {
uint8_t ptc; // Packet field type code (PTC)
uint8_t pfc; // Packet field format code (PFC)
- uint16_t paramId; // Unique ID of the parameter
uint32_t settingData;
// Actual data defining the operation of a peripheral or subsystem.
@@ -34,14 +37,15 @@ struct Parameter {
* Holds the list with the parameters and provides functions
* for parameter reporting and modification.
*
- * @todo Ensure that the parameter list is sorted by ID
+ * The parameter list is stored in a map with the parameter IDs as keys and values
+ * corresponding Parameter structs containing the PTC, PFC and the parameter's value.
*/
+
class ParameterService : public Service {
private:
- Parameter paramsList[CONFIGLENGTH];
- // CONFIGLENGTH is just a dummy number for now, this should be statically set
- static uint16_t numOfValidIds(Message idMsg); //count the valid ids in a given TC[20, 1]
+ etl::map<ParamId, Parameter, MAX_PARAMS> paramsList;
+ uint16_t numOfValidIds(Message idMsg); //count the valid ids in a given TC[20, 1]
public:
/**
@@ -54,14 +58,14 @@ public:
* containing the current configuration
* **for the parameters specified in the carried valid IDs**.
*
- * No sophisticated error checking for now, just whether the package is of the correct type
- * and whether the requested IDs are valid, ignoring the invalid ones. If no IDs are correct,
- * the returned message shall be empty.
+ * No sophisticated error checking for now, just whether the packet is of the correct type
+ * and whether the requested IDs are valid, ignoring the invalid ones.
+ * If the packet has an incorrect header, an InternalError::UnacceptablePacket is raised.
+ * If no IDs are correct, the returned message shall be empty.
*
* @param paramId: a valid TC[20, 1] packet carrying the requested parameter IDs
* @return None (messages are stored using storeMessage())
*
- * @todo Generate failure notifs where needed when ST[01] is ready
*
* NOTES:
* Method for valid ID counting is a hack (clones the message and figures out the number
@@ -69,7 +73,7 @@ public:
*
* Everything apart from the setting data is uint16 (setting data are uint32 for now)
*/
- void reportParameterIds(Message paramIds);
+ void reportParameterIds(Message& paramIds);
/**
* This function receives a TC[20, 3] message and after checking whether its type is correct,
@@ -79,10 +83,9 @@ public:
* @param newParamValues: a valid TC[20, 3] message carrying parameter ID and replacement value
* @return None
*
- * @todo Generate failure notifications where needed (eg. when an invalid ID is encountered)
* @todo Use pointers for changing and storing addresses to comply with the standard
*/
- void setParameterIds(Message newParamValues);
+ void setParameterIds(Message& newParamValues);
};
diff --git a/src/Helpers/CRCHelper.cpp b/src/Helpers/CRCHelper.cpp
index e0cd4ebf914ff885c7f50a52aeec4cd3e42c8da8..8361455071673204dcaa88ab4c3f136615127fdf 100644
--- a/src/Helpers/CRCHelper.cpp
+++ b/src/Helpers/CRCHelper.cpp
@@ -9,7 +9,7 @@ uint16_t CRCHelper::calculateCRC(const uint8_t* message, uint32_t length) {
// CRC16-CCITT generator polynomial (as specified in standard)
uint16_t polynomial = 0x1021u;
- for (int i = 0; i < length; i++) {
+ for (uint32_t i = 0; i < length; i++) {
// "copy" (XOR w/ existing contents) the current msg bits into the MSB of the shift register
shiftReg ^= (message[i] << 8u);
diff --git a/src/Services/MemoryManagementService.cpp b/src/Services/MemoryManagementService.cpp
index f904291c44b171161064a131300c79847b785346..ebb58a115bf2f7a3779708d47f9037ac16f42871 100644
--- a/src/Services/MemoryManagementService.cpp
+++ b/src/Services/MemoryManagementService.cpp
@@ -15,7 +15,7 @@ MemoryManagementService::RawDataMemoryManagement::RawDataMemoryManagement(
// Function declarations for the raw data memory management subservice
void MemoryManagementService::RawDataMemoryManagement::loadRawData(Message &request) {
/**
- * Bare in mind that there is currently no error checking for invalid parameters.
+ * Bear in mind that there is currently no error checking for invalid parameters.
* A future version will include error checking and the corresponding error report/notification,
* as the manual implies.
*
diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp
index bf19881992e6265b8254ae0c09bc118a665a780f..dd0f4c7495d4b925feb1355fe8808415043450c2 100644
--- a/src/Services/ParameterService.cpp
+++ b/src/Services/ParameterService.cpp
@@ -2,13 +2,9 @@
#define DEMOMODE
-#ifdef DEMOMODE
-
#include <ctime>
#include <cstdlib>
-#endif
-
ParameterService::ParameterService() {
#ifdef DEMOMODE
// Test code, setting up some of the parameter fields
@@ -16,56 +12,85 @@ ParameterService::ParameterService() {
time_t currTime = time(nullptr);
struct tm *today = localtime(&currTime);
- paramsList[0].paramId = 0; // random parameter ID
- paramsList[0].settingData = today->tm_hour; // the current hour
- paramsList[0].ptc = 3; // unsigned int
- paramsList[0].pfc = 14; // 32 bits
+ Parameter test1, test2;
+
+ test1.settingData = today->tm_hour; // the current hour
+ test1.ptc = 3; // unsigned int
+ test1.pfc = 14; // 32 bits
+
+ test2.settingData = today->tm_min; // the current minute
+ test2.ptc = 3; // unsigned int
+ test2.pfc = 14; // 32 bits
+
+ // MAKE SURE THE IDS ARE UNIQUE WHEN INSERTING!
+ /**
+ * @todo: Make a separate insert() function for parameter insertion to protect from blunders
+ * if needed to
+ */
+
+ paramsList.insert(std::make_pair(0, test1));
+ paramsList.insert(std::make_pair(1, test2));
- paramsList[1].paramId = 1; // random parameter ID
- paramsList[1].settingData = today->tm_min; // the current minute
- paramsList[1].ptc = 3; // unsigned int
- paramsList[1].pfc = 14; // 32 bits
#endif
}
-void ParameterService::reportParameterIds(Message paramIds) {
+void ParameterService::reportParameterIds(Message& paramIds) {
Message reqParam(20, 2, Message::TM, 1); // empty TM[20, 2] parameter report message
- if (paramIds.packetType == Message::TC && paramIds.serviceType == 20 &&
- paramIds.messageType == 1) {
- uint16_t ids = paramIds.readUint16();
- reqParam.appendUint16(numOfValidIds(paramIds)); // include the number of valid IDs
-
- for (int i = 0; i < ids; i++) {
- uint16_t currId = paramIds.readUint16(); // current ID to be appended
-
- if (currId < CONFIGLENGTH) { // check to prevent out-of-bounds access due to invalid id
- reqParam.appendUint16(currId);
- reqParam.appendUint32(paramsList[currId].settingData);
- } else {
- // generate failure of execution notification for ST[06]
- continue; //ignore the invalid ID
- }
+ paramIds.resetRead(); // since we're passing a reference, the reading position shall be reset
+ // to its default before any read operations (to ensure the correct data is being read)
+
+ // assertion: correct message, packet and service type (at failure throws an
+ // InternalError::UnacceptablePacket)
+ ErrorHandler::assertRequest(paramIds.packetType == Message::TC, paramIds,
+ ErrorHandler::AcceptanceErrorType::UnacceptableMessage);
+ ErrorHandler::assertRequest(paramIds.messageType == 1, paramIds,
+ ErrorHandler::AcceptanceErrorType::UnacceptableMessage);
+ ErrorHandler::assertRequest(paramIds.serviceType == 20, paramIds,
+ ErrorHandler::AcceptanceErrorType::UnacceptableMessage);
+
+ uint16_t ids = paramIds.readUint16();
+ reqParam.appendUint16(numOfValidIds(paramIds)); // include the number of valid IDs
+
+ for (int i = 0; i < ids; i++) {
+ uint16_t currId = paramIds.readUint16(); // current ID to be appended
+
+ if (paramsList.find(currId) != paramsList.end()) {
+ reqParam.appendUint16(currId);
+ reqParam.appendUint32(paramsList[currId].settingData);
+ } else {
+ ErrorHandler::reportError(paramIds,
+ ErrorHandler::ExecutionStartErrorType::UnknownExecutionStartError);
+ continue; // generate failed start of execution notification & ignore
}
}
storeMessage(reqParam);
}
-void ParameterService::setParameterIds(Message newParamValues) {
- if (newParamValues.packetType == Message::TC && newParamValues.serviceType == 20 &&
- newParamValues.messageType == 3) {
- uint16_t ids = newParamValues.readUint16(); //get number of ID's
+void ParameterService::setParameterIds(Message& newParamValues) {
- for (int i = 0; i < ids; i++) {
- uint16_t currId = newParamValues.readUint16();
+ // assertion: correct message, packet and service type (at failure throws an
+ // InternalError::UnacceptablePacket which gets logged)
- if (currId < CONFIGLENGTH) {
- paramsList[currId].settingData = newParamValues.readUint32();
- } else {
- // generate failure of execution notification for ST[06]
- continue; // ignore the invalid ID
- }
+ ErrorHandler::assertRequest(newParamValues.packetType == Message::TC, newParamValues,
+ ErrorHandler::AcceptanceErrorType::UnacceptableMessage);
+ ErrorHandler::assertRequest(newParamValues.messageType == 3, newParamValues,
+ ErrorHandler::AcceptanceErrorType::UnacceptableMessage);
+ ErrorHandler::assertRequest(newParamValues.serviceType == 20, newParamValues,
+ ErrorHandler::AcceptanceErrorType::UnacceptableMessage);
+
+ uint16_t ids = newParamValues.readUint16(); //get number of ID's
+
+ for (int i = 0; i < ids; i++) {
+ uint16_t currId = newParamValues.readUint16();
+
+ if (paramsList.find(currId) != paramsList.end()) {
+ paramsList[currId].settingData = newParamValues.readUint32();
+ } else {
+ ErrorHandler::reportError(newParamValues,
+ ErrorHandler::ExecutionStartErrorType::UnknownExecutionStartError);
+ continue; // generate failed start of execution notification & ignore
}
}
}
@@ -85,7 +110,7 @@ uint16_t ParameterService::numOfValidIds(Message idMsg) {
idMsg.readUint32(); //skip the 32bit settings blocks, we need only the IDs
}
- if (currId < CONFIGLENGTH) {
+ if (paramsList.find(currId) != paramsList.end()) {
validIds++;
}
}
diff --git a/test/Services/ParameterService.cpp b/test/Services/ParameterService.cpp
index 28f59a40837d957d87f78bcd29090a5ed5864b1f..61b96d6c1cc1c93cdc0c013717f401cb01c4abec 100644
--- a/test/Services/ParameterService.cpp
+++ b/test/Services/ParameterService.cpp
@@ -12,7 +12,7 @@ TEST_CASE("Parameter Report Subservice") {
request.appendUint16(2); // number of requested IDs
request.appendUint16(34672); // faulty ID in this context
- request.appendUint16(3); // valid
+ request.appendUint16(1); // valid
pserv.reportParameterIds(request);
report = ServiceTests::get(0);
@@ -27,11 +27,19 @@ TEST_CASE("Parameter Report Subservice") {
}
}
+ // **WARNING**
+ // TODO: Update this test (and all tests in general) to use the error handler's output when
+ // checking for assertions.
SECTION("Wrong Message Type Handling Test") {
Message falseRequest(15, 3, Message::TM, 1); // a totally wrong message
pserv.reportParameterIds(falseRequest);
- Message response = ServiceTests::get(0);
+ Message errorNotif = ServiceTests::get(0);
+ CHECK(errorNotif.messageType == 4); // check for proper failed start of
+ // execution notification
+ CHECK(errorNotif.serviceType == 1);
+
+ Message response = ServiceTests::get(1);
CHECK(response.messageType == 2);
CHECK(response.serviceType == 20);
CHECK(response.packetType == Message::TM);
@@ -44,23 +52,40 @@ TEST_CASE("Parameter Setting Subservice") {
Message setRequest(20, 3, Message::TC, 1);
Message reportRequest(20, 1, Message::TC, 1);
- setRequest.appendUint16(2); // correct number of IDs
- setRequest.appendUint16(3); // correct ID
+ setRequest.appendUint16(2); // total number of IDs
+ setRequest.appendUint16(1); // correct ID in this context
setRequest.appendUint32(3735928559); // 0xDEADBEEF in hex (new setting)
setRequest.appendUint16(16742); // faulty ID in this context
setRequest.appendUint32(3131746989); // 0xBAAAAAAD (this shouldn't be found in the report)
reportRequest.appendUint16(2);
reportRequest.appendUint16(16742);
- reportRequest.appendUint16(3);
+ reportRequest.appendUint16(1); // used to be 3, which pointed the bug with
+ // numOfValidIds out, now is 1 in order to be a valid ID (a separate test for
+ // numOfValidIds shall be introduced)
+
+ // Since every reporting and setting is called with the same (sometimes faulty) parameters,
+ // and there are errors generated (as should be) it is important to catch and check for
+ // them in order to preserve the integrity of the test.
pserv.reportParameterIds(reportRequest);
- Message before = ServiceTests::get(0);
+ Message errorNotif1 = ServiceTests::get(0);
+ CHECK(errorNotif1.messageType == 4);
+ CHECK(errorNotif1.serviceType == 1);
+
+ Message before = ServiceTests::get(1);
pserv.setParameterIds(setRequest);
+ Message errorNotif2 = ServiceTests::get(2);
+ CHECK(errorNotif2.messageType == 4);
+ CHECK(errorNotif2.serviceType == 1);
pserv.reportParameterIds(reportRequest);
- Message after = ServiceTests::get(1);
+ Message errorNotif3 = ServiceTests::get(3);
+ CHECK(errorNotif3.messageType == 4);
+ CHECK(errorNotif3.serviceType == 1);
+
+ Message after = ServiceTests::get(4);
before.readUint16();
after.readUint16(); // skip the number of IDs