Merge pull request 'Local Pool Update Remove Add Data Ignore Fault Argument' (#701) from eive/fsfw:mueller/local-pool-update-remove-ignore-fault-arg into development

Reviewed-on: fsfw/fsfw#701
This commit is contained in:
Steffen Gaisser 2022-11-14 14:49:34 +01:00
commit 69d338f9bb
10 changed files with 84 additions and 89 deletions

View File

@ -57,6 +57,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- `DeviceHandlerBase`: New signature of `handleDeviceTm` which expects - `DeviceHandlerBase`: New signature of `handleDeviceTm` which expects
a `const SerializeIF&` and additional helper variant which expects `const uint8_t*` a `const SerializeIF&` and additional helper variant which expects `const uint8_t*`
PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/671 PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/671
- Move some generic `StorageManagerIF` implementations from `LocalPool` to
interface itself so it can be re-used more easily. Also add new
abstract function `bool hasDataAtId(store_address_t storeId) const`.
PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/685
- Improvements for `AcceptsTelemetryIF` and `AcceptsTelecommandsIF`: - Improvements for `AcceptsTelemetryIF` and `AcceptsTelecommandsIF`:
- Make functions `const` where it makes sense - Make functions `const` where it makes sense
- Add `const char* getName const` abstract function - Add `const char* getName const` abstract function

View File

@ -19,7 +19,7 @@ ConstStorageAccessor::~ConstStorageAccessor() {
} }
} }
ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other) ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other) noexcept
: constDataPointer(other.constDataPointer), : constDataPointer(other.constDataPointer),
storeId(other.storeId), storeId(other.storeId),
size_(other.size_), size_(other.size_),
@ -30,7 +30,7 @@ ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other)
other.store = nullptr; other.store = nullptr;
} }
ConstStorageAccessor& ConstStorageAccessor::operator=(ConstStorageAccessor&& other) { ConstStorageAccessor& ConstStorageAccessor::operator=(ConstStorageAccessor&& other) noexcept {
constDataPointer = other.constDataPointer; constDataPointer = other.constDataPointer;
storeId = other.storeId; storeId = other.storeId;
store = other.store; store = other.store;
@ -84,7 +84,7 @@ void ConstStorageAccessor::print() const {
arrayprinter::print(constDataPointer, size_); arrayprinter::print(constDataPointer, size_);
} }
void ConstStorageAccessor::assignStore(StorageManagerIF* store) { void ConstStorageAccessor::assignStore(StorageManagerIF* store_) {
internalState = AccessState::ASSIGNED; internalState = AccessState::ASSIGNED;
this->store = store; store = store_;
} }

View File

@ -23,6 +23,7 @@ class ConstStorageAccessor {
//! StorageManager classes have exclusive access to private variables. //! StorageManager classes have exclusive access to private variables.
friend class PoolManager; friend class PoolManager;
friend class LocalPool; friend class LocalPool;
friend class StorageManagerIF;
public: public:
/** /**
@ -30,7 +31,7 @@ class ConstStorageAccessor {
* entry to access. * entry to access.
* @param storeId * @param storeId
*/ */
ConstStorageAccessor(store_address_t storeId); explicit ConstStorageAccessor(store_address_t storeId);
ConstStorageAccessor(store_address_t storeId, StorageManagerIF* store); ConstStorageAccessor(store_address_t storeId, StorageManagerIF* store);
/** /**
@ -43,7 +44,7 @@ class ConstStorageAccessor {
* @brief Returns a pointer to the read-only data * @brief Returns a pointer to the read-only data
* @return * @return
*/ */
const uint8_t* data() const; [[nodiscard]] const uint8_t* data() const;
/** /**
* @brief Copies the read-only data to the supplied pointer * @brief Copies the read-only data to the supplied pointer
@ -61,13 +62,13 @@ class ConstStorageAccessor {
* Get the size of the data * Get the size of the data
* @return * @return
*/ */
size_t size() const; [[nodiscard]] size_t size() const;
/** /**
* Get the storage ID. * Get the storage ID.
* @return * @return
*/ */
store_address_t getId() const; [[nodiscard]] store_address_t getId() const;
void print() const; void print() const;
@ -79,8 +80,8 @@ class ConstStorageAccessor {
* @param * @param
* @return * @return
*/ */
ConstStorageAccessor& operator=(ConstStorageAccessor&&); ConstStorageAccessor& operator=(ConstStorageAccessor&&) noexcept;
ConstStorageAccessor(ConstStorageAccessor&&); ConstStorageAccessor(ConstStorageAccessor&&) noexcept;
//! The copy ctor and copy assignemnt should be deleted implicitely //! The copy ctor and copy assignemnt should be deleted implicitely
//! according to https://foonathan.net/2019/02/special-member-functions/ //! according to https://foonathan.net/2019/02/special-member-functions/

View File

@ -29,11 +29,10 @@ LocalPool::LocalPool(object_id_t setObjectId, const LocalPoolConfig& poolConfig,
} }
} }
LocalPool::~LocalPool(void) {} LocalPool::~LocalPool() = default;
ReturnValue_t LocalPool::addData(store_address_t* storageId, const uint8_t* data, size_t size, ReturnValue_t LocalPool::addData(store_address_t* storageId, const uint8_t* data, size_t size) {
bool ignoreFault) { ReturnValue_t status = reserveSpace(size, storageId);
ReturnValue_t status = reserveSpace(size, storageId, ignoreFault);
if (status == returnvalue::OK) { if (status == returnvalue::OK) {
write(*storageId, data, size); write(*storageId, data, size);
} }
@ -48,25 +47,9 @@ ReturnValue_t LocalPool::getData(store_address_t packetId, const uint8_t** packe
return status; return status;
} }
ReturnValue_t LocalPool::getData(store_address_t storeId, ConstStorageAccessor& storeAccessor) {
uint8_t* tempData = nullptr;
ReturnValue_t status = modifyData(storeId, &tempData, &storeAccessor.size_);
storeAccessor.assignStore(this);
storeAccessor.constDataPointer = tempData;
return status;
}
ConstAccessorPair LocalPool::getData(store_address_t storeId) {
uint8_t* tempData = nullptr;
ConstStorageAccessor constAccessor(storeId, this);
ReturnValue_t status = modifyData(storeId, &tempData, &constAccessor.size_);
constAccessor.constDataPointer = tempData;
return ConstAccessorPair(status, std::move(constAccessor));
}
ReturnValue_t LocalPool::getFreeElement(store_address_t* storageId, const size_t size, ReturnValue_t LocalPool::getFreeElement(store_address_t* storageId, const size_t size,
uint8_t** pData, bool ignoreFault) { uint8_t** pData) {
ReturnValue_t status = reserveSpace(size, storageId, ignoreFault); ReturnValue_t status = reserveSpace(size, storageId);
if (status == returnvalue::OK) { if (status == returnvalue::OK) {
*pData = &store[storageId->poolIndex][getRawPosition(*storageId)]; *pData = &store[storageId->poolIndex][getRawPosition(*storageId)];
} else { } else {
@ -75,20 +58,6 @@ ReturnValue_t LocalPool::getFreeElement(store_address_t* storageId, const size_t
return status; return status;
} }
AccessorPair LocalPool::modifyData(store_address_t storeId) {
StorageAccessor accessor(storeId, this);
ReturnValue_t status = modifyData(storeId, &accessor.dataPointer, &accessor.size_);
accessor.assignConstPointer();
return AccessorPair(status, std::move(accessor));
}
ReturnValue_t LocalPool::modifyData(store_address_t storeId, StorageAccessor& storeAccessor) {
storeAccessor.assignStore(this);
ReturnValue_t status = modifyData(storeId, &storeAccessor.dataPointer, &storeAccessor.size_);
storeAccessor.assignConstPointer();
return status;
}
ReturnValue_t LocalPool::modifyData(store_address_t storeId, uint8_t** packetPtr, size_t* size) { ReturnValue_t LocalPool::modifyData(store_address_t storeId, uint8_t** packetPtr, size_t* size) {
ReturnValue_t status = returnvalue::FAILED; ReturnValue_t status = returnvalue::FAILED;
if (storeId.poolIndex >= NUMBER_OF_SUBPOOLS) { if (storeId.poolIndex >= NUMBER_OF_SUBPOOLS) {
@ -197,8 +166,7 @@ void LocalPool::clearStore() {
} }
} }
ReturnValue_t LocalPool::reserveSpace(const size_t size, store_address_t* storeId, ReturnValue_t LocalPool::reserveSpace(size_t size, store_address_t* storeId) {
bool ignoreFault) {
ReturnValue_t status = getSubPoolIndex(size, &storeId->poolIndex); ReturnValue_t status = getSubPoolIndex(size, &storeId->poolIndex);
if (status != returnvalue::OK) { if (status != returnvalue::OK) {
#if FSFW_CPP_OSTREAM_ENABLED == 1 #if FSFW_CPP_OSTREAM_ENABLED == 1

View File

@ -86,22 +86,16 @@ class LocalPool : public SystemObject, public StorageManagerIF {
/** /**
* Documentation: See StorageManagerIF.h * Documentation: See StorageManagerIF.h
*/ */
ReturnValue_t addData(store_address_t* storeId, const uint8_t* data, size_t size, ReturnValue_t addData(store_address_t* storeId, const uint8_t* data, size_t size) override;
bool ignoreFault = false) override;
ReturnValue_t getFreeElement(store_address_t* storeId, const size_t size, uint8_t** pData, ReturnValue_t getFreeElement(store_address_t* storeId, size_t size, uint8_t** pData) override;
bool ignoreFault = false) override;
ConstAccessorPair getData(store_address_t storeId) override;
ReturnValue_t getData(store_address_t storeId, ConstStorageAccessor& constAccessor) override;
ReturnValue_t getData(store_address_t storeId, const uint8_t** packet_ptr, size_t* size) override; ReturnValue_t getData(store_address_t storeId, const uint8_t** packet_ptr, size_t* size) override;
AccessorPair modifyData(store_address_t storeId) override;
ReturnValue_t modifyData(store_address_t storeId, StorageAccessor& storeAccessor) override;
ReturnValue_t modifyData(store_address_t storeId, uint8_t** packet_ptr, size_t* size) override; ReturnValue_t modifyData(store_address_t storeId, uint8_t** packet_ptr, size_t* size) override;
virtual ReturnValue_t deleteData(store_address_t storeId) override; ReturnValue_t deleteData(store_address_t storeId) override;
virtual ReturnValue_t deleteData(uint8_t* ptr, size_t size, ReturnValue_t deleteData(uint8_t* ptr, size_t size, store_address_t* storeId) override;
store_address_t* storeId = nullptr) override;
/** /**
* Get the total size of allocated memory for pool data. * Get the total size of allocated memory for pool data.
@ -131,8 +125,14 @@ class LocalPool : public SystemObject, public StorageManagerIF {
* Get number sub pools. Each pool has pages with a specific bucket size. * Get number sub pools. Each pool has pages with a specific bucket size.
* @return * @return
*/ */
max_subpools_t getNumberOfSubPools() const override; [[nodiscard]] max_subpools_t getNumberOfSubPools() const override;
bool hasDataAtId(store_address_t storeId) const override; [[nodiscard]] bool hasDataAtId(store_address_t storeId) const override;
// Using functions provided by StorageManagerIF requires either a fully qualified path
// like for example localPool.StorageManagerIF::getFreeElement(...) or re-exporting
// the fully qualified path with the using directive.
using StorageManagerIF::getData;
using StorageManagerIF::modifyData;
protected: protected:
/** /**
@ -142,7 +142,7 @@ class LocalPool : public SystemObject, public StorageManagerIF {
* @return - returnvalue::OK on success, * @return - returnvalue::OK on success,
* - the return codes of #getPoolIndex or #findEmpty otherwise. * - the return codes of #getPoolIndex or #findEmpty otherwise.
*/ */
virtual ReturnValue_t reserveSpace(const size_t size, store_address_t* address, bool ignoreFault); virtual ReturnValue_t reserveSpace(size_t size, store_address_t* address);
private: private:
/** /**
@ -186,6 +186,8 @@ class LocalPool : public SystemObject, public StorageManagerIF {
std::vector<std::vector<size_type>> sizeLists = std::vector<std::vector<size_type>> sizeLists =
std::vector<std::vector<size_type>>(NUMBER_OF_SUBPOOLS); std::vector<std::vector<size_type>>(NUMBER_OF_SUBPOOLS);
bool ignoreFault = false;
//! A variable to determine whether higher n pools are used if //! A variable to determine whether higher n pools are used if
//! the store is full. //! the store is full.
bool spillsToHigherPools = false; bool spillsToHigherPools = false;

View File

@ -9,10 +9,9 @@ PoolManager::PoolManager(object_id_t setObjectId, const LocalPoolConfig& localPo
PoolManager::~PoolManager() { MutexFactory::instance()->deleteMutex(mutex); } PoolManager::~PoolManager() { MutexFactory::instance()->deleteMutex(mutex); }
ReturnValue_t PoolManager::reserveSpace(const size_t size, store_address_t* address, ReturnValue_t PoolManager::reserveSpace(const size_t size, store_address_t* address) {
bool ignoreFault) {
MutexGuard mutexHelper(mutex, MutexIF::TimeoutType::WAITING, mutexTimeoutMs); MutexGuard mutexHelper(mutex, MutexIF::TimeoutType::WAITING, mutexTimeoutMs);
ReturnValue_t status = LocalPool::reserveSpace(size, address, ignoreFault); ReturnValue_t status = LocalPool::reserveSpace(size, address);
return status; return status;
} }

View File

@ -27,7 +27,7 @@ class PoolManager : public LocalPool {
* @brief In the PoolManager's destructor all allocated memory * @brief In the PoolManager's destructor all allocated memory
* is freed. * is freed.
*/ */
virtual ~PoolManager(); ~PoolManager() override;
/** /**
* Set the default mutex timeout for internal calls. * Set the default mutex timeout for internal calls.
@ -40,8 +40,7 @@ class PoolManager : public LocalPool {
* which wraps LocalPool calls with a mutex protection. * which wraps LocalPool calls with a mutex protection.
*/ */
ReturnValue_t deleteData(store_address_t) override; ReturnValue_t deleteData(store_address_t) override;
ReturnValue_t deleteData(uint8_t* buffer, size_t size, ReturnValue_t deleteData(uint8_t* buffer, size_t size, store_address_t* storeId) override;
store_address_t* storeId = nullptr) override;
/** /**
* The developer is allowed to lock the mutex in case the lock needs * The developer is allowed to lock the mutex in case the lock needs
@ -58,8 +57,7 @@ class PoolManager : public LocalPool {
//! Default mutex timeout value to prevent permanent blocking. //! Default mutex timeout value to prevent permanent blocking.
uint32_t mutexTimeoutMs = 20; uint32_t mutexTimeoutMs = 20;
ReturnValue_t reserveSpace(const size_t size, store_address_t* address, ReturnValue_t reserveSpace(size_t size, store_address_t* address) override;
bool ignoreFault) override;
/** /**
* @brief The mutex is created in the constructor and makes * @brief The mutex is created in the constructor and makes

View File

@ -10,7 +10,7 @@ StorageAccessor::StorageAccessor(store_address_t storeId) : ConstStorageAccessor
StorageAccessor::StorageAccessor(store_address_t storeId, StorageManagerIF* store) StorageAccessor::StorageAccessor(store_address_t storeId, StorageManagerIF* store)
: ConstStorageAccessor(storeId, store) {} : ConstStorageAccessor(storeId, store) {}
StorageAccessor& StorageAccessor::operator=(StorageAccessor&& other) { StorageAccessor& StorageAccessor::operator=(StorageAccessor&& other) noexcept {
// Call the parent move assignment and also assign own member. // Call the parent move assignment and also assign own member.
dataPointer = other.dataPointer; dataPointer = other.dataPointer;
ConstStorageAccessor::operator=(std::move(other)); ConstStorageAccessor::operator=(std::move(other));
@ -18,7 +18,7 @@ StorageAccessor& StorageAccessor::operator=(StorageAccessor&& other) {
} }
// Call the parent move ctor and also transfer own member. // Call the parent move ctor and also transfer own member.
StorageAccessor::StorageAccessor(StorageAccessor&& other) StorageAccessor::StorageAccessor(StorageAccessor&& other) noexcept
: ConstStorageAccessor(std::move(other)), dataPointer(other.dataPointer) {} : ConstStorageAccessor(std::move(other)), dataPointer(other.dataPointer) {}
ReturnValue_t StorageAccessor::getDataCopy(uint8_t* pointer, size_t maxSize) { ReturnValue_t StorageAccessor::getDataCopy(uint8_t* pointer, size_t maxSize) {

View File

@ -12,9 +12,10 @@ class StorageAccessor : public ConstStorageAccessor {
//! StorageManager classes have exclusive access to private variables. //! StorageManager classes have exclusive access to private variables.
friend class PoolManager; friend class PoolManager;
friend class LocalPool; friend class LocalPool;
friend class StorageManagerIF;
public: public:
StorageAccessor(store_address_t storeId); explicit StorageAccessor(store_address_t storeId);
StorageAccessor(store_address_t storeId, StorageManagerIF* store); StorageAccessor(store_address_t storeId, StorageManagerIF* store);
/** /**
@ -25,8 +26,8 @@ class StorageAccessor : public ConstStorageAccessor {
* @param * @param
* @return * @return
*/ */
StorageAccessor& operator=(StorageAccessor&&); StorageAccessor& operator=(StorageAccessor&&) noexcept;
StorageAccessor(StorageAccessor&&); StorageAccessor(StorageAccessor&&) noexcept;
ReturnValue_t write(uint8_t* data, size_t size, uint16_t offset = 0); ReturnValue_t write(uint8_t* data, size_t size, uint16_t offset = 0);
uint8_t* data(); uint8_t* data();

View File

@ -55,7 +55,7 @@ class StorageManagerIF {
/** /**
* @brief This is the empty virtual destructor as required for C++ interfaces. * @brief This is the empty virtual destructor as required for C++ interfaces.
*/ */
~StorageManagerIF() = default; virtual ~StorageManagerIF() = default;
/** /**
* @brief With addData, a free storage position is allocated and data * @brief With addData, a free storage position is allocated and data
* stored there. * stored there.
@ -66,8 +66,8 @@ class StorageManagerIF {
* @return Returns @returnvalue::OK if data was added. * @return Returns @returnvalue::OK if data was added.
* @returnvalue::FAILED if data could not be added, storageId is unchanged then. * @returnvalue::FAILED if data could not be added, storageId is unchanged then.
*/ */
virtual ReturnValue_t addData(store_address_t* storageId, const uint8_t* data, size_t size, virtual ReturnValue_t addData(store_address_t* storageId, const uint8_t* data, size_t size) = 0;
bool ignoreFault = false) = 0;
/** /**
* @brief With deleteData, the storageManager frees the memory region * @brief With deleteData, the storageManager frees the memory region
* identified by packet_id. * identified by packet_id.
@ -86,9 +86,10 @@ class StorageManagerIF {
* @return @li returnvalue::OK on success. * @return @li returnvalue::OK on success.
* @li failure code if deletion did not work * @li failure code if deletion did not work
*/ */
virtual ReturnValue_t deleteData(uint8_t* buffer, size_t size, virtual ReturnValue_t deleteData(uint8_t* buffer, size_t size, store_address_t* storeId) = 0;
store_address_t* storeId = nullptr) = 0; virtual ReturnValue_t deleteData(uint8_t* buffer, size_t size) {
return deleteData(buffer, size, nullptr);
}
/** /**
* @brief Access the data by supplying a store ID. * @brief Access the data by supplying a store ID.
* @details * @details
@ -97,7 +98,13 @@ class StorageManagerIF {
* @param storeId * @param storeId
* @return Pair of return value and a ConstStorageAccessor instance * @return Pair of return value and a ConstStorageAccessor instance
*/ */
virtual ConstAccessorPair getData(store_address_t storeId) = 0; virtual ConstAccessorPair getData(store_address_t storeId) {
uint8_t* tempData = nullptr;
ConstStorageAccessor constAccessor(storeId, this);
ReturnValue_t status = modifyData(storeId, &tempData, &constAccessor.size_);
constAccessor.constDataPointer = tempData;
return {status, std::move(constAccessor)};
}
/** /**
* @brief Access the data by supplying a store ID and a helper * @brief Access the data by supplying a store ID and a helper
@ -106,7 +113,13 @@ class StorageManagerIF {
* @param constAccessor Wrapper function to access store data. * @param constAccessor Wrapper function to access store data.
* @return * @return
*/ */
virtual ReturnValue_t getData(store_address_t storeId, ConstStorageAccessor& constAccessor) = 0; virtual ReturnValue_t getData(store_address_t storeId, ConstStorageAccessor& accessor) {
uint8_t* tempData = nullptr;
ReturnValue_t status = modifyData(storeId, &tempData, &accessor.size_);
accessor.assignStore(this);
accessor.constDataPointer = tempData;
return status;
}
/** /**
* @brief getData returns an address to data and the size of the data * @brief getData returns an address to data and the size of the data
@ -127,7 +140,12 @@ class StorageManagerIF {
* @param storeId * @param storeId
* @return Pair of return value and StorageAccessor helper * @return Pair of return value and StorageAccessor helper
*/ */
virtual AccessorPair modifyData(store_address_t storeId) = 0; virtual AccessorPair modifyData(store_address_t storeId) {
StorageAccessor accessor(storeId, this);
ReturnValue_t status = modifyData(storeId, &accessor.dataPointer, &accessor.size_);
accessor.assignConstPointer();
return {status, std::move(accessor)};
}
/** /**
* Modify data by supplying a store ID and a StorageAccessor helper instance. * Modify data by supplying a store ID and a StorageAccessor helper instance.
@ -135,7 +153,12 @@ class StorageManagerIF {
* @param accessor Helper class to access the modifiable data. * @param accessor Helper class to access the modifiable data.
* @return * @return
*/ */
virtual ReturnValue_t modifyData(store_address_t storeId, StorageAccessor& accessor) = 0; virtual ReturnValue_t modifyData(store_address_t storeId, StorageAccessor& accessor) {
accessor.assignStore(this);
ReturnValue_t status = modifyData(storeId, &accessor.dataPointer, &accessor.size_);
accessor.assignConstPointer();
return status;
}
/** /**
* Get pointer and size of modifiable data by supplying the storeId * Get pointer and size of modifiable data by supplying the storeId
@ -154,12 +177,11 @@ class StorageManagerIF {
* written to p_data! * written to p_data!
* @param storageId A pointer to the storageId to retrieve. * @param storageId A pointer to the storageId to retrieve.
* @param size The size of the space to be reserved. * @param size The size of the space to be reserved.
* @param p_data A pointer to the element data is returned here. * @param dataPtr A pointer to the element data is returned here.
* @return Returns @li returnvalue::OK if data was added. * @return Returns @returnvalue::OK if data was added.
* @returnvalue::FAILED if data could not be added, storageId is unchanged then. * @returnvalue::FAILED if data could not be added, storageId is unchanged then.
*/ */
virtual ReturnValue_t getFreeElement(store_address_t* storageId, size_t size, uint8_t** p_data, virtual ReturnValue_t getFreeElement(store_address_t* storageId, size_t size, uint8_t** dataPtr) = 0;
bool ignoreFault = false) = 0;
[[nodiscard]] virtual bool hasDataAtId(store_address_t storeId) const = 0; [[nodiscard]] virtual bool hasDataAtId(store_address_t storeId) const = 0;