fix some tests

src/fsfw/datapool/PoolDataSetBase.cpp

@@ -82,19 +82,19 @@ uint16_t PoolDataSetBase::getFillCount() const { return fillCount; }
 ReturnValue_t PoolDataSetBase::readVariable(uint16_t count) {
   ReturnValue_t result = returnvalue::OK;
   if (registeredVariables[count] == nullptr) {
-    /* Configuration error. */
+    // Configuration error.
     return returnvalue::FAILED;
   }
 
-  /* These checks are often performed by the respective variable implementation too, but I guess
+  // These checks are often performed by the respective variable implementation too, but I guess
-  a double check does not hurt. */
+  // a double check does not hurt.
   if (registeredVariables[count]->getReadWriteMode() != PoolVariableIF::VAR_WRITE and
       registeredVariables[count]->getDataPoolId() != PoolVariableIF::NO_PARAMETER) {
     if (protectEveryReadCommitCall) {
       result =
           registeredVariables[count]->read(timeoutTypeForSingleVars, mutexTimeoutForSingleVars);
     } else {
-      /* The readWithoutLock function is protected, so we use the attorney here */
+      // The readWithoutLock function is protected, so we use the attorney here
       result = ReadCommitIFAttorney::readWithoutLock(registeredVariables[count]);
     }
 

src/fsfw/datapool/PoolVariableIF.h

@@ -23,6 +23,7 @@ class PoolVariableIF : public SerializeIF, public ReadCommitIF {
   static constexpr uint8_t INTERFACE_ID = CLASS_ID::POOL_VARIABLE_IF;
   static constexpr ReturnValue_t INVALID_READ_WRITE_MODE = MAKE_RETURN_CODE(0xA0);
   static constexpr ReturnValue_t INVALID_POOL_ENTRY = MAKE_RETURN_CODE(0xA1);
+  static constexpr ReturnValue_t INVALID_SHARED_POOL = MAKE_RETURN_CODE(0xA2);
 
   static constexpr bool VALID = 1;
   static constexpr bool INVALID = 0;

src/fsfw/datapoollocal/LocalPoolDataSetBase.h

@@ -42,7 +42,6 @@ class PeriodicHousekeepingHelper;
  * @ingroup data_pool
  */
 class LocalPoolDataSetBase : public SerializeIF, public PoolDataSetIF {
-  friend class LocalPoolDataSetAttorney;
   friend class PeriodicHousekeepingHelper;
 
  public:

src/fsfw/datapoollocal/LocalPoolObjectBase.cpp

@@ -49,6 +49,7 @@ LocalPoolObjectBase::LocalPoolObjectBase(object_id_t poolOwner, lp_id_t poolId,
 #endif
     return;
   }
+  sharedPool = hkOwner->getOptionalSharedPool();
 
   if (dataSet != nullptr) {
     dataSet->registerVariable(this);

src/fsfw/datapoollocal/LocalPoolVariable.tpp

@@ -39,6 +39,9 @@ inline ReturnValue_t LocalPoolVariable<T>::read(MutexIF::TimeoutType timeoutType
 
 template <typename T>
 inline ReturnValue_t LocalPoolVariable<T>::readWithoutLock() {
+  if (sharedPool == nullptr) {
+    return PoolVariableIF::INVALID_SHARED_POOL;
+  }
   if (readWriteMode == pool_rwm_t::VAR_WRITE) {
     return PoolVariableIF::INVALID_READ_WRITE_MODE;
   }

src/fsfw/datapoollocal/PeriodicHkGenerationHelper.cpp

@@ -143,7 +143,7 @@ ReturnValue_t PeriodicHkGenerationHelper::generateHousekeepingPacket(sid_t sid,
   if (!optSetSpec.has_value()) {
     return DATASET_NOT_FOUND;
   }
-  auto setSpec = optSetSpec.value();
+  auto& setSpec = optSetSpec.value().get();
   uint8_t* dataPtr = nullptr;
   const size_t maxSize = setSpec.size;
   ReturnValue_t result = ipcStore->getFreeElement(&storeId, maxSize, &dataPtr);
@@ -185,9 +185,6 @@ ReturnValue_t PeriodicHkGenerationHelper::generateHousekeepingPacket(sid_t sid,
   return hkQueue->sendMessage(destination, &hkMessage);
 }
 
-ReturnValue_t PeriodicHkGenerationHelper::serializeHkPacketIntoStore(
-    HousekeepingPacketDownlink& hkPacket, store_address_t& storeId, size_t* serializedSize) {}
-
 void PeriodicHkGenerationHelper::performPeriodicHkGeneration(periodicHk::SetSpecification& setSpec,
                                                              timeval& now) {
   if (not setSpec.periodicCollectionEnabled) {
@@ -225,12 +222,12 @@ ReturnValue_t PeriodicHkGenerationHelper::togglePeriodicGeneration(sid_t sid, bo
                         DATASET_NOT_FOUND);
     return DATASET_NOT_FOUND;
   }
-  optSetSpec.value().periodicCollectionEnabled = enable;
+  optSetSpec.value().get().periodicCollectionEnabled = enable;
   return returnvalue::OK;
 }
 
-std::optional<periodicHk::SetSpecification> PeriodicHkGenerationHelper::getSetSpecification(
+std::optional<std::reference_wrapper<periodicHk::SetSpecification>>
-    sid_t structureId) {
+PeriodicHkGenerationHelper::getSetSpecification(sid_t structureId) {
   for (auto& receiver : setList) {
     if (receiver.dataId.sid == structureId) {
       return receiver;
@@ -263,7 +260,7 @@ ReturnValue_t PeriodicHkGenerationHelper::generateSetStructurePacket(sid_t sid)
                         DATASET_NOT_FOUND);
     return DATASET_NOT_FOUND;
   }
-  auto setSpec = *optSetSpec;
+  auto& setSpec = optSetSpec.value().get();
 
   uint8_t* storePtr = nullptr;
   store_address_t storeId;
@@ -314,7 +311,7 @@ ReturnValue_t PeriodicHkGenerationHelper::enablePeriodicPacket(
                         DATASET_NOT_FOUND);
     return DATASET_NOT_FOUND;
   }
-  auto setSpec = optSetSpec.value();
+  auto& setSpec = optSetSpec.value().get();
   setSpec.periodicCollectionEnabled = true;
   if (frequencyMs) {
     setSpec.collectionFrequency = frequencyMs.value();
@@ -330,7 +327,7 @@ ReturnValue_t PeriodicHkGenerationHelper::disablePeriodicPacket(sid_t structureI
                         DATASET_NOT_FOUND);
     return DATASET_NOT_FOUND;
   }
-  auto setSpec = optSetSpec.value();
+  auto& setSpec = optSetSpec.value().get();
   setSpec.periodicCollectionEnabled = false;
   return returnvalue::OK;
 }

src/fsfw/datapoollocal/PeriodicHkGenerationHelper.h

@@ -213,7 +213,8 @@ class PeriodicHkGenerationHelper : public PeriodicHkGenerationProviderIF {
   ReturnValue_t setCollectionInterval(sid_t structureId, dur_millis_t newCollectionIntervalMs);
 
  protected:
-  std::optional<periodicHk::SetSpecification> getSetSpecification(sid_t structureId);
+  std::optional<std::reference_wrapper<periodicHk::SetSpecification>> getSetSpecification(
+      sid_t structureId);
 
   ReturnValue_t subscribeForPeriodicPacket(subdp::ParamsBase& params);
   ReturnValue_t subscribeForUpdatePacket(subdp::ParamsBase& params);

src/fsfw/datapoollocal/SharedPool.h

@@ -35,7 +35,7 @@ class SharedPool {
   object_id_t ownerId;
 
   // Core data structure for the actual pool data
-  DataPool localPoolMap;
+  DataPool localPoolMap{};
   // Every housekeeping data manager has a mutex to protect access
   // to it's data pool.
   MutexIF* mutex = nullptr;

unittests/datapoollocal/testDataSet.cpp

@@ -30,12 +30,12 @@ TEST_CASE("DataSetTest", "[DataSetTest]") {
     CHECK(localSet.getCreatorObjectId() == objects::TEST_LOCAL_POOL_OWNER_BASE);
     size_t maxSize = localSet.getLocalPoolIdsSerializedSize(true);
     uint8_t localPoolIdBuff[maxSize];
-    /* Skip size field */
+    // Skip size field
     auto* lpIds = reinterpret_cast<lp_id_t*>(localPoolIdBuff + 1);
     size_t serSize = 0;
     auto* localPoolIdBuffPtr = reinterpret_cast<uint8_t*>(localPoolIdBuff);
 
-    /* Test local pool ID serialization */
+    // Test local pool ID serialization
     CHECK(localSet.serializeLocalPoolIds(&localPoolIdBuffPtr, &serSize, maxSize,
                                          SerializeIF::Endianness::MACHINE) == returnvalue::OK);
     CHECK(serSize == maxSize);
@@ -43,7 +43,7 @@ TEST_CASE("DataSetTest", "[DataSetTest]") {
     CHECK(lpIds[0] == localSet.localPoolVarUint8.getDataPoolId());
     CHECK(lpIds[1] == localSet.localPoolVarFloat.getDataPoolId());
     CHECK(lpIds[2] == localSet.localPoolUint16Vec.getDataPoolId());
-    /* Now serialize without fill count */
+    // Now serialize without fill count
     lpIds = reinterpret_cast<lp_id_t*>(localPoolIdBuff);
     localPoolIdBuffPtr = localPoolIdBuff;
     serSize = 0;
@@ -55,29 +55,24 @@ TEST_CASE("DataSetTest", "[DataSetTest]") {
     CHECK(lpIds[2] == localSet.localPoolUint16Vec.getDataPoolId());
 
     {
-      /* Test read operation. Values should be all zeros */
+      // Test read operation. Values should be all zeros
       PoolReadGuard readHelper(&localSet);
       REQUIRE(readHelper.getReadResult() == returnvalue::OK);
-      // CHECK(not localSet.isValid());
       CHECK(localSet.localPoolVarUint8.value == 0);
-      // CHECK(not localSet.localPoolVarUint8.isValid());
       CHECK(localSet.localPoolVarFloat.value == Catch::Approx(0.0));
-      // CHECK(not localSet.localPoolVarUint8.isValid());
       CHECK(localSet.localPoolUint16Vec.value[0] == 0);
       CHECK(localSet.localPoolUint16Vec.value[1] == 0);
       CHECK(localSet.localPoolUint16Vec.value[2] == 0);
-      // CHECK(not localSet.localPoolVarUint8.isValid());
 
-      /* Now set new values, commit should be done by read helper automatically */
+      // Now set new values, commit should be done by read helper automatically
       localSet.localPoolVarUint8 = 232;
       localSet.localPoolVarFloat = -2324.322;
       localSet.localPoolUint16Vec.value[0] = 232;
       localSet.localPoolUint16Vec.value[1] = 23923;
       localSet.localPoolUint16Vec.value[2] = 1;
-      // localSet.setValidity(true, true);
     }
 
-    /* Zero out some values for next test */
+    // Zero out some values for next test
     localSet.localPoolVarUint8 = 0;
     localSet.localPoolVarFloat = 0;
 
@@ -87,25 +82,21 @@ TEST_CASE("DataSetTest", "[DataSetTest]") {
     CHECK(localSet.localPoolVarFloat.getReadWriteMode() == pool_rwm_t::VAR_READ);
 
     {
-      /* Now we read again and check whether our zeroed values were overwritten with
+      // Now we read again and check whether our zeroed values were overwritten with
-      the values in the pool */
+      // the values in the pool
       PoolReadGuard readHelper(&localSet);
       REQUIRE(readHelper.getReadResult() == returnvalue::OK);
-      // CHECK(localSet.isValid());
       CHECK(localSet.localPoolVarUint8.value == 232);
-      // CHECK(localSet.localPoolVarUint8.isValid());
       CHECK(localSet.localPoolVarFloat.value == Catch::Approx(-2324.322));
-      // CHECK(localSet.localPoolVarFloat.isValid());
       CHECK(localSet.localPoolUint16Vec.value[0] == 232);
       CHECK(localSet.localPoolUint16Vec.value[1] == 23923);
       CHECK(localSet.localPoolUint16Vec.value[2] == 1);
-      // CHECK(localSet.localPoolUint16Vec.isValid());
 
       // Now we serialize these values into a buffer without the validity buffer
       maxSize = localSet.getSerializedSize();
       CHECK(maxSize == sizeof(uint8_t) + sizeof(uint16_t) * 3 + sizeof(float));
       serSize = 0;
-      /* Already reserve additional space for validity buffer, will be needed later */
+      // Already reserve additional space for validity buffer, will be needed later
       uint8_t buffer[maxSize + 1];
       uint8_t* buffPtr = buffer;
       CHECK(localSet.serialize(&buffPtr, &serSize, maxSize, SerializeIF::Endianness::MACHINE) ==
@@ -123,79 +114,18 @@ TEST_CASE("DataSetTest", "[DataSetTest]") {
       CHECK(rawUint16Vec[2] == 1);
 
       size_t sizeToDeserialize = maxSize;
-      /* Now we zeros out the raw entries and deserialize back into the dataset */
+      // Now we zeros out the raw entries and deserialize back into the dataset
       std::memset(buffer, 0, sizeof(buffer));
       const uint8_t* constBuffPtr = buffer;
       CHECK(localSet.deSerialize(&constBuffPtr, &sizeToDeserialize,
                                  SerializeIF::Endianness::MACHINE) == returnvalue::OK);
-      /* Check whether deserialization was successfull */
+      // Check whether deserialization was successfull
       CHECK(localSet.localPoolVarUint8.value == 0);
       CHECK(localSet.localPoolVarFloat.value == Catch::Approx(0.0));
       CHECK(localSet.localPoolVarUint8.value == 0);
       CHECK(localSet.localPoolUint16Vec.value[0] == 0);
       CHECK(localSet.localPoolUint16Vec.value[1] == 0);
       CHECK(localSet.localPoolUint16Vec.value[2] == 0);
-      /* Validity should be unchanged */
-      // CHECK(localSet.localPoolVarUint8.isValid());
-      // CHECK(localSet.localPoolVarFloat.isValid());
-      // CHECK(localSet.localPoolUint16Vec.isValid());
-
-      /* Now we do the same process but with the validity buffer */
-      localSet.localPoolVarUint8 = 232;
-      localSet.localPoolVarFloat = -2324.322;
-      localSet.localPoolUint16Vec.value[0] = 232;
-      localSet.localPoolUint16Vec.value[1] = 23923;
-      localSet.localPoolUint16Vec.value[2] = 1;
-      // localSet.localPoolVarUint8.setValid(true);
-      // localSet.localPoolVarFloat.setValid(false);
-      // localSet.localPoolUint16Vec.setValid(true);
-      // localSet.setValidityBufferGeneration(true);
-      maxSize = localSet.getSerializedSize();
-      CHECK(maxSize == sizeof(uint8_t) + sizeof(uint16_t) * 3 + sizeof(float) + 1);
-      serSize = 0;
-      buffPtr = buffer;
-      CHECK(localSet.serialize(&buffPtr, &serSize, maxSize, SerializeIF::Endianness::MACHINE) ==
-            returnvalue::OK);
-      CHECK(rawUint8 == 232);
-      std::memcpy(&rawFloat, buffer + sizeof(uint8_t), sizeof(float));
-      CHECK(rawFloat == Catch::Approx(-2324.322));
-
-      std::memcpy(&rawUint16Vec, buffer + sizeof(uint8_t) + sizeof(float), 3 * sizeof(uint16_t));
-      CHECK(rawUint16Vec[0] == 232);
-      CHECK(rawUint16Vec[1] == 23923);
-      CHECK(rawUint16Vec[2] == 1);
-      /* We can do it like this because the buffer only has one byte for
-      less than 8 variables */
-      uint8_t* validityByte = buffer + sizeof(buffer) - 1;
-      bool bitSet = false;
-      bitutil::get(validityByte, 0, bitSet);
-
-      CHECK(bitSet == true);
-      bitutil::get(validityByte, 1, bitSet);
-      CHECK(bitSet == false);
-      bitutil::get(validityByte, 2, bitSet);
-      CHECK(bitSet == true);
-
-      /* Now we manipulate the validity buffer for the deserialization */
-      bitutil::clear(validityByte, 0);
-      bitutil::set(validityByte, 1);
-      bitutil::clear(validityByte, 2);
-      /* Zero out everything except validity buffer */
-      std::memset(buffer, 0, sizeof(buffer) - 1);
-      sizeToDeserialize = maxSize;
-      constBuffPtr = buffer;
-      CHECK(localSet.deSerialize(&constBuffPtr, &sizeToDeserialize,
-                                 SerializeIF::Endianness::MACHINE) == returnvalue::OK);
-      /* Check whether deserialization was successfull */
-      CHECK(localSet.localPoolVarUint8.value == 0);
-      CHECK(localSet.localPoolVarFloat.value == Catch::Approx(0.0));
-      CHECK(localSet.localPoolVarUint8.value == 0);
-      CHECK(localSet.localPoolUint16Vec.value[0] == 0);
-      CHECK(localSet.localPoolUint16Vec.value[1] == 0);
-      CHECK(localSet.localPoolUint16Vec.value[2] == 0);
-      // CHECK(not localSet.localPoolVarUint8.isValid());
-      // CHECK(localSet.localPoolVarFloat.isValid());
-      // CHECK(not localSet.localPoolUint16Vec.isValid());
     }
 
     /* Common fault test cases */
@@ -207,53 +137,6 @@ TEST_CASE("DataSetTest", "[DataSetTest]") {
             static_cast<int>(DataSetIF::POOL_VAR_NULL));
   }
 
-  SECTION("MorePoolVariables") {
-    LocalDataSet set(poolOwner.sharedPool, 2, 10);
-
-    /* Register same variables again to get more than 8 registered variables */
-    for (uint8_t idx = 0; idx < 8; idx++) {
-      REQUIRE(set.registerVariable(&localSet.localPoolVarUint8) == returnvalue::OK);
-    }
-    REQUIRE(set.registerVariable(&localSet.localPoolVarUint8) == returnvalue::OK);
-    REQUIRE(set.registerVariable(&localSet.localPoolUint16Vec) == returnvalue::OK);
-
-    // set.setValidityBufferGeneration(true);
-    {
-      PoolReadGuard readHelper(&localSet);
-      localSet.localPoolVarUint8.value = 42;
-      // localSet.localPoolVarUint8.setValid(true);
-      // localSet.localPoolUint16Vec.setValid(false);
-    }
-
-    size_t maxSize = set.getSerializedSize();
-    CHECK(maxSize == 9 + sizeof(uint16_t) * 3 + 2);
-    size_t serSize = 0;
-    /* Already reserve additional space for validity buffer, will be needed later */
-    uint8_t buffer[maxSize + 1];
-    uint8_t* buffPtr = buffer;
-    CHECK(set.serialize(&buffPtr, &serSize, maxSize, SerializeIF::Endianness::MACHINE) == OK);
-    std::array<uint8_t, 2> validityBuffer{};
-    std::memcpy(validityBuffer.data(), buffer + 9 + sizeof(uint16_t) * 3, 2);
-    /* The first 9 variables should be valid */
-    CHECK(validityBuffer[0] == 0xff);
-    bool bitSet = false;
-    bitutil::get(validityBuffer.data() + 1, 0, bitSet);
-    CHECK(bitSet == true);
-    bitutil::get(validityBuffer.data() + 1, 1, bitSet);
-    CHECK(bitSet == false);
-
-    /* Now we invert the validity */
-    validityBuffer[0] = 0;
-    validityBuffer[1] = 0b0100'0000;
-    std::memcpy(buffer + 9 + sizeof(uint16_t) * 3, validityBuffer.data(), 2);
-    const uint8_t* constBuffPtr = buffer;
-    size_t sizeToDeSerialize = serSize;
-    CHECK(set.deSerialize(&constBuffPtr, &sizeToDeSerialize, SerializeIF::Endianness::MACHINE) ==
-          returnvalue::OK);
-    // CHECK(localSet.localPoolVarUint8.isValid() == false);
-    // CHECK(localSet.localPoolUint16Vec.isValid() == true);
-  }
-
   SECTION("SharedDataSet") {
     object_id_t sharedSetId = objects::SHARED_SET_ID;
     SharedLocalDataSet sharedSet(sharedSetId, poolOwner.sharedPool, lpool::testSetId, 5);
@@ -266,15 +149,13 @@ TEST_CASE("DataSetTest", "[DataSetTest]") {
     CHECK(sharedSet.unlockDataset() == returnvalue::OK);
 
     {
-      // PoolReadGuard rg(&sharedSet);
+      PoolReadGuard rg(&sharedSet);
-      // CHECK(rg.getReadResult() == returnvalue::OK);
+      CHECK(rg.getReadResult() == returnvalue::OK);
       localSet.localPoolVarUint8.value = 5;
       localSet.localPoolUint16Vec.value[0] = 1;
       localSet.localPoolUint16Vec.value[1] = 2;
       localSet.localPoolUint16Vec.value[2] = 3;
       CHECK(sharedSet.commit() == returnvalue::OK);
     }
-
-    // sharedSet.setReadCommitProtectionBehaviour(true);
   }
 }