From 899d021e00784951fc3caa119de540d7c6341a69 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 25 Jul 2022 11:15:45 +0200 Subject: [PATCH] using uint32_t as store_address requires explicit cast --- src/fsfw/cfdp/CFDPMessage.cpp | 2 +- src/fsfw/datapoollocal/HasLocalDataPoolIF.h | 4 ++-- src/fsfw/parameters/ParameterMessage.cpp | 2 +- src/fsfw/storagemanager/storeAddress.h | 18 ++++++++++++------ src/fsfw/tmtcservices/TmStoreHelper.cpp | 2 ++ src/fsfw/tmtcservices/TmStoreHelper.h | 1 + src/fsfw/tmtcservices/TmTcBridge.cpp | 2 +- unittests/action/TestActionHelper.cpp | 4 ++-- .../datapoollocal/LocalPoolManagerTest.cpp | 4 ++-- unittests/datapoollocal/LocalPoolOwnerBase.cpp | 4 ++-- unittests/storagemanager/TestNewAccessor.cpp | 2 +- unittests/tmtcservices/testStoreHelper.cpp | 17 +++++++++++++++++ 12 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/fsfw/cfdp/CFDPMessage.cpp b/src/fsfw/cfdp/CFDPMessage.cpp index fee55715a..da6fe15b7 100644 --- a/src/fsfw/cfdp/CFDPMessage.cpp +++ b/src/fsfw/cfdp/CFDPMessage.cpp @@ -10,7 +10,7 @@ void CFDPMessage::setCommand(CommandMessage *message, store_address_t cfdpPacket store_address_t CFDPMessage::getStoreId(const CommandMessage *message) { store_address_t storeAddressCFDPPacket; - storeAddressCFDPPacket = message->getParameter(); + storeAddressCFDPPacket = static_cast(message->getParameter()); return storeAddressCFDPPacket; } diff --git a/src/fsfw/datapoollocal/HasLocalDataPoolIF.h b/src/fsfw/datapoollocal/HasLocalDataPoolIF.h index a2925a46e..acbe16976 100644 --- a/src/fsfw/datapoollocal/HasLocalDataPoolIF.h +++ b/src/fsfw/datapoollocal/HasLocalDataPoolIF.h @@ -80,7 +80,7 @@ class HasLocalDataPoolIF { * clearing the store automatically */ virtual void handleChangedDataset(sid_t sid, - store_address_t storeId = storeId::INVALID_STORE_ADDRESS, + store_address_t storeId = store_address_t::invalid(), bool* clearMessage = nullptr) { if (clearMessage != nullptr) { *clearMessage = true; @@ -100,7 +100,7 @@ class HasLocalDataPoolIF { * after the callback. */ virtual void handleChangedPoolVariable(gp_id_t gpid, - store_address_t storeId = storeId::INVALID_STORE_ADDRESS, + store_address_t storeId = store_address_t::invalid(), bool* clearMessage = nullptr) { if (clearMessage != nullptr) { *clearMessage = true; diff --git a/src/fsfw/parameters/ParameterMessage.cpp b/src/fsfw/parameters/ParameterMessage.cpp index 9dc58365b..9a7b5288f 100644 --- a/src/fsfw/parameters/ParameterMessage.cpp +++ b/src/fsfw/parameters/ParameterMessage.cpp @@ -44,7 +44,7 @@ store_address_t ParameterMessage::getParameterLoadCommand(const CommandMessage* *pfc = packedParamSettings >> 16 & 0xff; *rows = packedParamSettings >> 8 & 0xff; *columns = packedParamSettings & 0xff; - return message->getParameter2(); + return static_cast(message->getParameter2()); } void ParameterMessage::clear(CommandMessage* message) { diff --git a/src/fsfw/storagemanager/storeAddress.h b/src/fsfw/storagemanager/storeAddress.h index 41f70573d..a656cfe10 100644 --- a/src/fsfw/storagemanager/storeAddress.h +++ b/src/fsfw/storagemanager/storeAddress.h @@ -3,26 +3,26 @@ #include -namespace storeId { -static constexpr uint32_t INVALID_STORE_ADDRESS = 0xffffffff; -} - /** * This union defines the type that identifies where a data packet is * stored in the store. It comprises of a raw part to read it as raw value and * a structured part to use it in pool-like stores. */ union store_address_t { + public: + static constexpr uint32_t INVALID_RAW = 0xffffffff; /** * Default Constructor, initializing to INVALID_ADDRESS */ - store_address_t() : raw(storeId::INVALID_STORE_ADDRESS) {} + store_address_t() : raw(INVALID_RAW) {} /** * Constructor to create an address object using the raw address * * @param rawAddress */ - store_address_t(uint32_t rawAddress) : raw(rawAddress) {} + explicit store_address_t(uint32_t rawAddress) : raw(rawAddress) {} + + static store_address_t invalid() { return {}; }; /** * Constructor to create an address object using pool @@ -52,6 +52,12 @@ union store_address_t { uint32_t raw; bool operator==(const store_address_t& other) const { return raw == other.raw; } + bool operator!=(const store_address_t& other) const { return raw != other.raw; } + + store_address_t& operator=(const uint32_t rawAddr) { + raw = rawAddr; + return *this; + } }; #endif /* FSFW_STORAGEMANAGER_STOREADDRESS_H_ */ diff --git a/src/fsfw/tmtcservices/TmStoreHelper.cpp b/src/fsfw/tmtcservices/TmStoreHelper.cpp index a14530139..5f766c3f5 100644 --- a/src/fsfw/tmtcservices/TmStoreHelper.cpp +++ b/src/fsfw/tmtcservices/TmStoreHelper.cpp @@ -58,3 +58,5 @@ void TmStoreHelper::setTimeStamper(TimeStamperIF& timeStamper_) { } void TmStoreHelper::setApid(uint16_t apid) { creator.setApid(apid); } + +PusTmCreator& TmStoreHelper::getCreatorRef() { return creator; } diff --git a/src/fsfw/tmtcservices/TmStoreHelper.h b/src/fsfw/tmtcservices/TmStoreHelper.h index 1707e5bb9..ebaff707b 100644 --- a/src/fsfw/tmtcservices/TmStoreHelper.h +++ b/src/fsfw/tmtcservices/TmStoreHelper.h @@ -15,6 +15,7 @@ class TmStoreHelper { ReturnValue_t preparePacket(uint8_t service, uint8_t subservice, uint16_t counter); + PusTmCreator& getCreatorRef(); void setTimeStamper(TimeStamperIF& timeStamper); [[nodiscard]] const store_address_t& getCurrentAddr() const; void setSourceDataRaw(const uint8_t* data, size_t len); diff --git a/src/fsfw/tmtcservices/TmTcBridge.cpp b/src/fsfw/tmtcservices/TmTcBridge.cpp index 8ea671195..6af03600f 100644 --- a/src/fsfw/tmtcservices/TmTcBridge.cpp +++ b/src/fsfw/tmtcservices/TmTcBridge.cpp @@ -166,7 +166,7 @@ ReturnValue_t TmTcBridge::handleTmQueue() { } ReturnValue_t TmTcBridge::storeDownlinkData(TmTcMessage* message) { - store_address_t storeId = 0; + store_address_t storeId; if (tmFifo == nullptr) { return HasReturnvaluesIF::RETURN_FAILED; } diff --git a/unittests/action/TestActionHelper.cpp b/unittests/action/TestActionHelper.cpp index 6461a195c..dd252d881 100644 --- a/unittests/action/TestActionHelper.cpp +++ b/unittests/action/TestActionHelper.cpp @@ -80,7 +80,7 @@ TEST_CASE("Action Helper", "[ActionHelper]") { } SECTION("Handle failed") { - store_address_t toLongParamAddress = StorageManagerIF::INVALID_ADDRESS; + store_address_t toLongParamAddress = store_address_t::invalid(); std::array toLongData = {5, 4, 3, 2, 1}; REQUIRE(ipcStore->addData(&toLongParamAddress, toLongData.data(), 5) == retval::CATCH_OK); ActionMessage::setCommand(&actionMessage, testActionId, toLongParamAddress); @@ -98,7 +98,7 @@ TEST_CASE("Action Helper", "[ActionHelper]") { } SECTION("Missing IPC Data") { - ActionMessage::setCommand(&actionMessage, testActionId, StorageManagerIF::INVALID_ADDRESS); + ActionMessage::setCommand(&actionMessage, testActionId, store_address_t::invalid()); CHECK(not testDhMock.executeActionCalled); REQUIRE(actionHelper.handleActionMessage(&actionMessage) == retval::CATCH_OK); CommandMessage testMessage; diff --git a/unittests/datapoollocal/LocalPoolManagerTest.cpp b/unittests/datapoollocal/LocalPoolManagerTest.cpp index 58a6065ee..6d9f162cc 100644 --- a/unittests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittests/datapoollocal/LocalPoolManagerTest.cpp @@ -392,13 +392,13 @@ TEST_CASE("LocalPoolManagerTest", "[LocManTest]") { CHECK(gpidToCheck == lpool::uint8VarGpid); HousekeepingMessage::setUpdateSnapshotSetCommand(&hkCmd, lpool::testSid, - storeId::INVALID_STORE_ADDRESS); + store_address_t::invalid()); CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); CHECK(poolOwner->changedDataSetCallbackWasCalled(sidToCheck, storeId) == true); CHECK(sidToCheck == lpool::testSid); HousekeepingMessage::setUpdateSnapshotVariableCommand(&hkCmd, lpool::uint8VarGpid, - storeId::INVALID_STORE_ADDRESS); + store_address_t::invalid()); CHECK(poolOwner->poolManager.handleHousekeepingMessage(&hkCmd) == retval::CATCH_OK); CHECK(poolOwner->changedVariableCallbackWasCalled(gpidToCheck, storeId) == true); CHECK(gpidToCheck == lpool::uint8VarGpid); diff --git a/unittests/datapoollocal/LocalPoolOwnerBase.cpp b/unittests/datapoollocal/LocalPoolOwnerBase.cpp index 6f0548931..2743e7f71 100644 --- a/unittests/datapoollocal/LocalPoolOwnerBase.cpp +++ b/unittests/datapoollocal/LocalPoolOwnerBase.cpp @@ -90,7 +90,7 @@ bool LocalPoolOwnerBase::changedDataSetCallbackWasCalled(sid_t &sid, store_addre sid = changedDatasetSid; storeId = storeIdForChangedSet; this->changedDatasetSid.raw = sid_t::INVALID_SID; - this->storeIdForChangedSet = storeId::INVALID_STORE_ADDRESS; + this->storeIdForChangedSet = store_address_t::invalid(); return condition; } @@ -108,7 +108,7 @@ bool LocalPoolOwnerBase::changedVariableCallbackWasCalled(gp_id_t &gpid, store_a gpid = changedPoolVariableGpid; storeId = storeIdForChangedVariable; this->changedPoolVariableGpid.raw = gp_id_t::INVALID_GPID; - this->storeIdForChangedVariable = storeId::INVALID_STORE_ADDRESS; + this->storeIdForChangedVariable = store_address_t::invalid(); return condition; } diff --git a/unittests/storagemanager/TestNewAccessor.cpp b/unittests/storagemanager/TestNewAccessor.cpp index 2cc2e4695..996239a67 100644 --- a/unittests/storagemanager/TestNewAccessor.cpp +++ b/unittests/storagemanager/TestNewAccessor.cpp @@ -162,7 +162,7 @@ TEST_CASE("New Accessor", "[NewAccessor]") { REQUIRE(result == retval::CATCH_OK); { StorageAccessor accessor(testStoreId); - StorageAccessor accessor2(0); + StorageAccessor accessor2(store_address_t::invalid()); accessor2 = std::move(accessor); REQUIRE(accessor.data() == nullptr); std::array data; diff --git a/unittests/tmtcservices/testStoreHelper.cpp b/unittests/tmtcservices/testStoreHelper.cpp index 8c92a2283..4740ef873 100644 --- a/unittests/tmtcservices/testStoreHelper.cpp +++ b/unittests/tmtcservices/testStoreHelper.cpp @@ -9,4 +9,21 @@ TEST_CASE("TM Store Helper", "[tm-store-helper]") { LocalPool::LocalPoolConfig cfg = {{10, 32}, {5, 64}}; LocalPool pool(objects::NO_OBJECT, cfg); auto storeHelper = TmStoreHelper(2, pool, timeStamper); + + SECTION("State") { + REQUIRE(storeHelper.getCurrentAddr() == store_address_t::invalid()); + REQUIRE(storeHelper.preparePacket(17, 1, 1) == HasReturnvaluesIF::RETURN_OK); + auto& creator = storeHelper.getCreatorRef(); + REQUIRE(creator.getApid() == 2); + REQUIRE(creator.getService() == 17); + REQUIRE(creator.getSubService() == 1); + REQUIRE(creator.getSequenceCount() == 0); + REQUIRE(creator.getMessageTypeCounter() == 1); + } + + SECTION("Basic") { + REQUIRE(storeHelper.preparePacket(17, 1, 1) == HasReturnvaluesIF::RETURN_OK); + REQUIRE(storeHelper.addPacketToStore() == HasReturnvaluesIF::RETURN_OK); + REQUIRE(storeHelper.getCurrentAddr() != store_address_t::invalid()); + } } \ No newline at end of file