From 5069f8b02bb547c01657f7d5e3592e429b890371 Mon Sep 17 00:00:00 2001
From: Robin Mueller <muellerr@irs.uni-stuttgart.de>
Date: Mon, 16 Dec 2024 17:38:05 +0100
Subject: [PATCH] some fixes

---
 src/fsfw/controller/ExtendedControllerBase.h     |  7 ++++---
 src/fsfw/datapool/LocalPoolObjectBase.cpp        |  8 ++++++++
 src/fsfw/datapool/SharedSetBase.cpp              | 11 +++++++++--
 src/fsfw/devicehandlers/FreshDeviceHandlerBase.h |  8 +++++---
 4 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/fsfw/controller/ExtendedControllerBase.h b/src/fsfw/controller/ExtendedControllerBase.h
index 47824ea9..9780b67b 100644
--- a/src/fsfw/controller/ExtendedControllerBase.h
+++ b/src/fsfw/controller/ExtendedControllerBase.h
@@ -34,11 +34,12 @@ class ExtendedControllerBase : public ControllerBase,
   ActionHelper actionHelper;
 
   // Periodic HK methods, default method assumes that no shared pool is required.
-  virtual datapool::SharedPool* getOptionalSharedPool() override;
+  datapool::SharedPool* getOptionalSharedPool() override = 0;
 
   // Periodic HK abstract methods.
-  ReturnValue_t serializeHkDataset(dp::sid_t structureId, uint8_t* buf, size_t maxSize) = 0;
-  ReturnValue_t specifyHkDatasets(std::vector<hk::SetSpecification>& setList) = 0;
+  ReturnValue_t serializeHkDataset(dp::sid_t structureId, uint8_t* buf,
+                                   size_t maxSize) override = 0;
+  ReturnValue_t specifyHkDatasets(std::vector<hk::SetSpecification>& setList) override = 0;
 
   /**
    * Implemented by child class. Handle all command messages which are
diff --git a/src/fsfw/datapool/LocalPoolObjectBase.cpp b/src/fsfw/datapool/LocalPoolObjectBase.cpp
index 99d4196e..054dcc20 100644
--- a/src/fsfw/datapool/LocalPoolObjectBase.cpp
+++ b/src/fsfw/datapool/LocalPoolObjectBase.cpp
@@ -48,6 +48,14 @@ PoolObjectBase::PoolObjectBase(object_id_t poolOwner, id_t poolId, DataSetIF* da
     return;
   }
   sharedPool = hkOwner->getOptionalSharedPool();
+  if (sharedPool == nullptr) {
+#if FSFW_CPP_OSTREAM_ENABLED == 1
+    sif::error << "PoolObjectBase: HK owner 0x" << std::hex << poolOwner << std::dec
+               << "does not have a shared pool " << std::endl;
+#else
+    sif::printError("PoolObjectBase: HK owner 0x%08x does not have a shared pool\n", poolOwner);
+#endif
+  }
 
   if (dataSet != nullptr) {
     dataSet->registerVariable(this);
diff --git a/src/fsfw/datapool/SharedSetBase.cpp b/src/fsfw/datapool/SharedSetBase.cpp
index cda80a05..5900861c 100644
--- a/src/fsfw/datapool/SharedSetBase.cpp
+++ b/src/fsfw/datapool/SharedSetBase.cpp
@@ -143,10 +143,17 @@ void SharedSetBase::setAllVariablesReadOnly() {
 void SharedSetBase::printSet() { return; }
 
 ReturnValue_t SharedSetBase::read(MutexIF::TimeoutType timeoutType, uint32_t timeoutMs) {
-  return base.read(timeoutType, timeoutMs);
+  lockDataPool(timeoutType, timeoutMs);
+  ReturnValue_t result = base.read(timeoutType, timeoutMs);
+  unlockDataPool();
+  return result;
 }
+
 ReturnValue_t SharedSetBase::commit(MutexIF::TimeoutType timeoutType, uint32_t timeoutMs) {
-  return base.commit(timeoutType, timeoutMs);
+  lockDataPool(timeoutType, timeoutMs);
+  ReturnValue_t result = base.commit(timeoutType, timeoutMs);
+  unlockDataPool();
+  return result;
 }
 uint16_t SharedSetBase::getFillCount() const { return base.getFillCount(); }
 
diff --git a/src/fsfw/devicehandlers/FreshDeviceHandlerBase.h b/src/fsfw/devicehandlers/FreshDeviceHandlerBase.h
index ad4973c2..d37138a6 100644
--- a/src/fsfw/devicehandlers/FreshDeviceHandlerBase.h
+++ b/src/fsfw/devicehandlers/FreshDeviceHandlerBase.h
@@ -148,9 +148,6 @@ class FreshDeviceHandlerBase : public SystemObject,
   // System Object overrides.
   ReturnValue_t initialize() override;
 
-  // Default implementation assumes that no optional shared pool is required.
-  datapool::SharedPool* getOptionalSharedPool() override;
-
   /**
    * This function is implemented to serialize a housekeeping packet when a HK message to
    * generate the packet is received, or periodic generation is necessary. The user should serialize
@@ -164,6 +161,11 @@ class FreshDeviceHandlerBase : public SystemObject,
    */
   ReturnValue_t specifyHkDatasets(std::vector<hk::SetSpecification>& setList) override = 0;
 
+  /*
+   *  If this device handler has an optional pool, return it. Otherwise, nullptr can be returned.
+   */
+  datapool::SharedPool* getOptionalSharedPool() override = 0;
+
   /**
    * Implemented by child class. Handle all command messages which are
    * not health, mode, action or housekeeping messages.