From a89ba37fb4441b3b8302dc6ee488676b575e6963 Mon Sep 17 00:00:00 2001
From: Kamil Kasperczyk <kamil.kasperczyk@nordicsemi.no>
Date: Wed, 19 Feb 2025 12:53:56 +0100
Subject: [PATCH] samples: matter: Improved bridge storage manager error
handling
Added checking the enum value returned by persistent storage.
Changed PSErrorCode enum to enum class to prevent such
unintuitive comparison in the future.
Signed-off-by: Kamil Kasperczyk <kamil.kasperczyk@nordicsemi.no>
---
.../src/bridge/bridge_storage_manager.cpp | 55 ++++++++++++-------
.../persistent_storage_common.h | 2 +-
.../matter/lock/src/access/access_storage.cpp | 2 +-
3 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/samples/matter/common/src/bridge/bridge_storage_manager.cpp b/samples/matter/common/src/bridge/bridge_storage_manager.cpp
index ac4a721cd121..1e9ffbc63afd 100644
--- a/samples/matter/common/src/bridge/bridge_storage_manager.cpp
+++ b/samples/matter/common/src/bridge/bridge_storage_manager.cpp
@@ -17,10 +17,9 @@ namespace
template <class T> bool LoadDataToObject(Nrf::PersistentStorageNode *node, T &data)
{
size_t readSize = 0;
+ const Nrf::PSErrorCode status = Nrf::GetPersistentStorage().NonSecureLoad(node, &data, sizeof(T), readSize);
- bool result = Nrf::GetPersistentStorage().NonSecureLoad(node, &data, sizeof(T), readSize);
-
- return result;
+ return status == Nrf::PSErrorCode::Success ? sizeof(T) == readSize : false;
}
Nrf::PersistentStorageNode CreateIndexNode(uint8_t bridgedDeviceIndex, Nrf::PersistentStorageNode *parent)
@@ -47,8 +46,8 @@ template <> bool BridgeStorageManager::LoadBridgedDevice(BridgedDeviceV1 &device
const uint8_t mandatoryItemsSize =
sizeof(device.mEndpointId) + sizeof(device.mDeviceType) + sizeof(device.mNodeLabelLength);
- if (!Nrf::GetPersistentStorage().NonSecureLoad(&id, buffer, sizeof(BridgedDeviceV1) + kMaxUserDataSize,
- readSize)) {
+ if (Nrf::GetPersistentStorage().NonSecureLoad(&id, buffer, sizeof(BridgedDeviceV1) + kMaxUserDataSize,
+ readSize) != PSErrorCode::Success) {
return false;
}
@@ -114,8 +113,8 @@ template <> bool BridgeStorageManager::LoadBridgedDevice(BridgedDeviceV2 &device
const uint8_t mandatoryItemsSize =
sizeof(device.mEndpointId) + sizeof(device.mDeviceType) + sizeof(device.mUniqueIDLength);
- if (!Nrf::GetPersistentStorage().NonSecureLoad(&id, buffer, sizeof(BridgedDeviceV2) + kMaxUserDataSize,
- readSize)) {
+ if (Nrf::GetPersistentStorage().NonSecureLoad(&id, buffer, sizeof(BridgedDeviceV2) + kMaxUserDataSize,
+ readSize) != PSErrorCode::Success) {
return false;
}
@@ -189,10 +188,10 @@ template <> bool BridgeStorageManager::LoadBridgedDevice(BridgedDeviceV2 &device
bool BridgeStorageManager::Init()
{
- bool result = Nrf::GetPersistentStorage().NonSecureInit();
+ const PSErrorCode status = Nrf::GetPersistentStorage().NonSecureInit();
- if (!result) {
- return result;
+ if (status != PSErrorCode::Success) {
+ return false;
}
/* Perform data migration from previous data structure versions if needed. */
@@ -361,7 +360,10 @@ bool BridgeStorageManager::MigrateData()
bool BridgeStorageManager::StoreBridgedDevicesCount(uint8_t count)
{
- return Nrf::GetPersistentStorage().NonSecureStore(&mBridgedDevicesCount, &count, sizeof(count));
+ const PSErrorCode status =
+ Nrf::GetPersistentStorage().NonSecureStore(&mBridgedDevicesCount, &count, sizeof(count));
+
+ return status == PSErrorCode::Success;
}
bool BridgeStorageManager::LoadBridgedDevicesCount(uint8_t &count)
@@ -375,7 +377,9 @@ bool BridgeStorageManager::StoreBridgedDevicesIndexes(uint8_t *indexes, uint8_t
return false;
}
- return Nrf::GetPersistentStorage().NonSecureStore(&mBridgedDevicesIndexes, indexes, count);
+ const PSErrorCode status = Nrf::GetPersistentStorage().NonSecureStore(&mBridgedDevicesIndexes, indexes, count);
+
+ return status == PSErrorCode::Success;
}
bool BridgeStorageManager::LoadBridgedDevicesIndexes(uint8_t *indexes, uint8_t maxCount, size_t &count)
@@ -384,7 +388,10 @@ bool BridgeStorageManager::LoadBridgedDevicesIndexes(uint8_t *indexes, uint8_t m
return false;
}
- return Nrf::GetPersistentStorage().NonSecureLoad(&mBridgedDevicesIndexes, indexes, maxCount, count);
+ const PSErrorCode status =
+ Nrf::GetPersistentStorage().NonSecureLoad(&mBridgedDevicesIndexes, indexes, maxCount, count);
+
+ return status == PSErrorCode::Success;
}
#ifdef CONFIG_BRIDGE_MIGRATE_PRE_2_7_0
@@ -398,23 +405,26 @@ bool BridgeStorageManager::LoadBridgedDeviceEndpointId(uint16_t &endpointId, uin
bool BridgeStorageManager::RemoveBridgedDeviceEndpointId(uint8_t bridgedDeviceIndex)
{
Nrf::PersistentStorageNode id = CreateIndexNode(bridgedDeviceIndex, &mBridgedDeviceEndpointId);
+ const PSErrorCode status = Nrf::GetPersistentStorage().NonSecureRemove(&id);
- return Nrf::GetPersistentStorage().NonSecureRemove(&id);
+ return status == PSErrorCode::Success;
}
bool BridgeStorageManager::LoadBridgedDeviceNodeLabel(char *label, size_t labelMaxLength, size_t &labelLength,
uint8_t bridgedDeviceIndex)
{
Nrf::PersistentStorageNode id = CreateIndexNode(bridgedDeviceIndex, &mBridgedDeviceNodeLabel);
+ const PSErrorCode status = Nrf::GetPersistentStorage().NonSecureLoad(&id, label, labelMaxLength, labelLength);
- return Nrf::GetPersistentStorage().NonSecureLoad(&id, label, labelMaxLength, labelLength);
+ return status == PSErrorCode::Success;
}
bool BridgeStorageManager::RemoveBridgedDeviceNodeLabel(uint8_t bridgedDeviceIndex)
{
Nrf::PersistentStorageNode id = CreateIndexNode(bridgedDeviceIndex, &mBridgedDeviceNodeLabel);
+ const PSErrorCode status = Nrf::GetPersistentStorage().NonSecureRemove(&id);
- return Nrf::GetPersistentStorage().NonSecureRemove(&id);
+ return status == PSErrorCode::Success;
}
bool BridgeStorageManager::LoadBridgedDeviceType(uint16_t &deviceType, uint8_t bridgedDeviceIndex)
@@ -427,8 +437,9 @@ bool BridgeStorageManager::LoadBridgedDeviceType(uint16_t &deviceType, uint8_t b
bool BridgeStorageManager::RemoveBridgedDeviceType(uint8_t bridgedDeviceIndex)
{
Nrf::PersistentStorageNode id = CreateIndexNode(bridgedDeviceIndex, &mBridgedDeviceType);
+ const PSErrorCode status = Nrf::GetPersistentStorage().NonSecureRemove(&id);
- return Nrf::GetPersistentStorage().NonSecureRemove(&id);
+ return status == PSErrorCode::Success;
}
#endif
@@ -464,14 +475,17 @@ bool BridgeStorageManager::StoreBridgedDevice(BridgedDevice &device, uint8_t ind
counter += device.mUserDataSize;
}
- return Nrf::GetPersistentStorage().NonSecureStore(&id, buffer, counter);
+ const PSErrorCode status = Nrf::GetPersistentStorage().NonSecureStore(&id, buffer, counter);
+
+ return status == PSErrorCode::Success;
}
bool BridgeStorageManager::RemoveBridgedDevice(uint8_t index)
{
Nrf::PersistentStorageNode id = CreateIndexNode(index, &mBridgedDevice);
+ const PSErrorCode status = Nrf::GetPersistentStorage().NonSecureRemove(&id);
- return Nrf::GetPersistentStorage().NonSecureRemove(&id);
+ return status == PSErrorCode::Success;
}
#ifdef CONFIG_BRIDGE_MIGRATE_PRE_2_7_0
@@ -486,8 +500,9 @@ bool BridgeStorageManager::LoadBtAddress(bt_addr_le_t &addr, uint8_t bridgedDevi
bool BridgeStorageManager::RemoveBtAddress(uint8_t bridgedDeviceIndex)
{
Nrf::PersistentStorageNode id = CreateIndexNode(bridgedDeviceIndex, &mBtAddress);
+ const PSErrorCode status = Nrf::GetPersistentStorage().NonSecureRemove(&id);
- return Nrf::GetPersistentStorage().NonSecureRemove(&id);
+ return status == PSErrorCode::Success;
}
#endif
#endif
diff --git a/samples/matter/common/src/persistent_storage/persistent_storage_common.h b/samples/matter/common/src/persistent_storage/persistent_storage_common.h
index eb25096711db..7406217f76b1 100644
--- a/samples/matter/common/src/persistent_storage/persistent_storage_common.h
+++ b/samples/matter/common/src/persistent_storage/persistent_storage_common.h
@@ -10,7 +10,7 @@
namespace Nrf
{
-enum PSErrorCode : uint8_t { Failure, Success, NotSupported };
+enum class PSErrorCode : uint8_t { Failure, Success, NotSupported };
/**
* @brief Class representing single tree node and containing information about its key.
diff --git a/samples/matter/lock/src/access/access_storage.cpp b/samples/matter/lock/src/access/access_storage.cpp
index 2bb2fdc3305e..658a20ff5537 100644
--- a/samples/matter/lock/src/access/access_storage.cpp
+++ b/samples/matter/lock/src/access/access_storage.cpp
@@ -62,7 +62,7 @@ bool GetStorageFreeSpace(size_t &freeBytes)
bool AccessStorage::Init()
{
- return (Nrf::Success == Nrf::GetPersistentStorage().PSInit());
+ return (Nrf::PSErrorCode::Success == Nrf::GetPersistentStorage().PSInit());
}
bool AccessStorage::PrepareKeyName(Type storageType, uint16_t index, uint16_t subindex)