From 3310601b93b8070f9bc41c28b8d8c7b83346cfba Mon Sep 17 00:00:00 2001 From: Jiacheng Guo Date: Fri, 4 Mar 2022 22:37:05 +0800 Subject: [PATCH] [bindings] Save binding table in persistent storage (#15449) * [bindings] Save binding table in persistent storage * [bindings] Use TLV format for saving in storage * fix review comments --- .../src/binding-handler.cpp | 16 +- .../efr32/src/binding-handler.cpp | 18 +- examples/tv-casting-app/linux/main.cpp | 4 +- src/app/clusters/bindings/BindingManager.cpp | 68 ++++-- src/app/clusters/bindings/BindingManager.h | 13 +- src/app/clusters/bindings/bindings.cpp | 3 +- src/app/server/Server.h | 2 + src/app/tests/TestBindingTable.cpp | 96 +++++++- src/app/tests/TestPendingNotificationMap.cpp | 5 +- src/app/util/binding-table.cpp | 226 ++++++++++++++++-- src/app/util/binding-table.h | 40 +++- src/app/util/types_stub.h | 4 +- src/lib/support/DefaultStorageKeyAllocator.h | 3 + .../support/TestPersistentStorageDelegate.h | 18 ++ 14 files changed, 452 insertions(+), 64 deletions(-) diff --git a/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp b/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp index ab1b917a1c9738..03c96e8c66be09 100644 --- a/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp @@ -24,6 +24,7 @@ #include "app/server/Server.h" #include "controller/InvokeInteraction.h" #include "lib/core/CHIPError.h" +#include "platform/CHIPDeviceLayer.h" #if defined(ENABLE_CHIP_SHELL) #include "lib/shell/Engine.h" @@ -102,10 +103,21 @@ static void BoundDeviceChangedHandler(const EmberBindingTableEntry & binding, ch } } -CHIP_ERROR InitBindingHandlers() +static void InitBindingHandlerInternal(intptr_t arg) { - chip::BindingManager::GetInstance().SetAppServer(&chip::Server::GetInstance()); + auto & server = chip::Server::GetInstance(); + chip::BindingManager::GetInstance().Init( + { &server.GetFabricTable(), server.GetCASESessionManager(), &server.GetPersistentStorage() }); chip::BindingManager::GetInstance().RegisterBoundDeviceChangedHandler(BoundDeviceChangedHandler); +} + +CHIP_ERROR InitBindingHandlers() +{ + // The initialization of binding manager will try establishing connection with unicast peers + // so it requires the Server instance to be correctly initialized. Post the init function to + // the event queue so that everything is ready when initialization is conducted. + // TODO: Fix initialization order issue in Matter server. + chip::DeviceLayer::PlatformMgr().ScheduleWork(InitBindingHandlerInternal); #if defined(ENABLE_CHIP_SHELL) RegisterSwitchCommands(); #endif diff --git a/examples/light-switch-app/efr32/src/binding-handler.cpp b/examples/light-switch-app/efr32/src/binding-handler.cpp index 76aa97ff523db2..8373c7c0b1ef36 100644 --- a/examples/light-switch-app/efr32/src/binding-handler.cpp +++ b/examples/light-switch-app/efr32/src/binding-handler.cpp @@ -22,6 +22,7 @@ #include "app/clusters/bindings/BindingManager.h" #include "app/server/Server.h" #include "controller/InvokeInteraction.h" +#include "platform/CHIPDeviceLayer.h" #if defined(ENABLE_CHIP_SHELL) #include "lib/shell/Engine.h" @@ -132,6 +133,15 @@ static void RegisterSwitchCommands() return; } #endif // ENABLE_CHIP_SHELL + +void InitBindingHandlerInternal(intptr_t arg) +{ + auto & server = chip::Server::GetInstance(); + chip::BindingManager::GetInstance().Init( + { &server.GetFabricTable(), server.GetCASESessionManager(), &server.GetPersistentStorage() }); + chip::BindingManager::GetInstance().RegisterBoundDeviceChangedHandler(LightSwitchChangedHandler); +} + } // namespace void SwitchToggleOnOff(intptr_t context) @@ -169,12 +179,12 @@ void SwitchOnOffOff(intptr_t context) CHIP_ERROR InitBindingHandler() { - BindingManager::GetInstance().SetAppServer(&Server::GetInstance()); - BindingManager::GetInstance().RegisterBoundDeviceChangedHandler(LightSwitchChangedHandler); - + // The initialization of binding manager will try establishing connection with unicast peers + // so it requires the Server instance to be correctly initialized. Post the init function to + // the event queue so that everything is ready when initialization is conducted. + chip::DeviceLayer::PlatformMgr().ScheduleWork(InitBindingHandlerInternal); #if defined(ENABLE_CHIP_SHELL) RegisterSwitchCommands(); #endif - return CHIP_NO_ERROR; } diff --git a/examples/tv-casting-app/linux/main.cpp b/examples/tv-casting-app/linux/main.cpp index 444dab93ac375b..8039a144ea2370 100644 --- a/examples/tv-casting-app/linux/main.cpp +++ b/examples/tv-casting-app/linux/main.cpp @@ -132,7 +132,9 @@ static void OnBindingAdded(const EmberBindingTableEntry & binding) CHIP_ERROR InitBindingHandlers() { - chip::BindingManager::GetInstance().SetAppServer(&chip::Server::GetInstance()); + auto & server = chip::Server::GetInstance(); + chip::BindingManager::GetInstance().Init( + { &server.GetFabricTable(), server.GetCASESessionManager(), &server.GetPersistentStorage() }); ReturnErrorOnFailure(chip::BindingManager::GetInstance().RegisterBindingAddedHandler(OnBindingAdded)); return CHIP_NO_ERROR; } diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp index 35967d50ab1016..d6ee8d019b4c17 100644 --- a/src/app/clusters/bindings/BindingManager.cpp +++ b/src/app/clusters/bindings/BindingManager.cpp @@ -18,6 +18,7 @@ #include #include #include +#include namespace { @@ -25,12 +26,13 @@ class BindingFabricTableDelegate : public chip::FabricTableDelegate { void OnFabricDeletedFromStorage(chip::CompressedFabricId compressedFabricId, chip::FabricIndex fabricIndex) { - auto iter = chip::BindingTable::GetInstance().begin(); - while (iter != chip::BindingTable::GetInstance().end()) + chip::BindingTable & bindingTable = chip::BindingTable::GetInstance(); + auto iter = bindingTable.begin(); + while (iter != bindingTable.end()) { if (iter->fabricIndex == fabricIndex) { - iter = chip::BindingTable::GetInstance().RemoveAt(iter); + bindingTable.RemoveAt(iter); } else { @@ -53,9 +55,9 @@ BindingFabricTableDelegate gFabricTableDelegate; namespace { -chip::PeerId PeerIdForNode(chip::FabricTable & fabricTable, chip::FabricIndex fabric, chip::NodeId node) +chip::PeerId PeerIdForNode(chip::FabricTable * fabricTable, chip::FabricIndex fabric, chip::NodeId node) { - chip::FabricInfo * fabricInfo = fabricTable.FindFabricWithIndex(fabric); + chip::FabricInfo * fabricInfo = fabricTable->FindFabricWithIndex(fabric); if (fabricInfo == nullptr) { return chip::PeerId(); @@ -80,20 +82,42 @@ CHIP_ERROR BindingManager::UnicastBindingRemoved(uint8_t bindingEntryId) return CHIP_NO_ERROR; } -void BindingManager::SetAppServer(Server * appServer) +CHIP_ERROR BindingManager::Init(const BindingManagerInitParams & params) { - mAppServer = appServer; - mAppServer->GetFabricTable().AddFabricDelegate(&gFabricTableDelegate); + VerifyOrReturnError(params.mCASESessionManager != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(params.mFabricTable != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(params.mStorage != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + mInitParams = params; + params.mFabricTable->AddFabricDelegate(&gFabricTableDelegate); + BindingTable::GetInstance().SetPersistentStorage(params.mStorage); + CHIP_ERROR error = BindingTable::GetInstance().LoadFromStorage(); + if (error != CHIP_NO_ERROR) + { + // This can happen during first boot of the device. + ChipLogProgress(AppServer, "Cannot load binding table: %" CHIP_ERROR_FORMAT, error.Format()); + } + else + { + for (const EmberBindingTableEntry & entry : BindingTable::GetInstance()) + { + if (entry.type == EMBER_UNICAST_BINDING) + { + // The CASE connection can also fail if the unicast peer is offline. + // There is recovery mechanism to retry connection on-demand so ignore error. + (void) UnicastBindingCreated(entry.fabricIndex, entry.nodeId); + } + } + } + return CHIP_NO_ERROR; } CHIP_ERROR BindingManager::EstablishConnection(FabricIndex fabric, NodeId node) { - VerifyOrReturnError(mAppServer != nullptr, CHIP_ERROR_INCORRECT_STATE); - - PeerId peer = PeerIdForNode(mAppServer->GetFabricTable(), fabric, node); + VerifyOrReturnError(mInitParams.mCASESessionManager != nullptr, CHIP_ERROR_INCORRECT_STATE); + PeerId peer = PeerIdForNode(mInitParams.mFabricTable, fabric, node); VerifyOrReturnError(peer.GetNodeId() != kUndefinedNodeId, CHIP_ERROR_NOT_FOUND); CHIP_ERROR error = - mAppServer->GetCASESessionManager()->FindOrEstablishSession(peer, &mOnConnectedCallback, &mOnConnectionFailureCallback); + mInitParams.mCASESessionManager->FindOrEstablishSession(peer, &mOnConnectedCallback, &mOnConnectionFailureCallback); if (error == CHIP_ERROR_NO_MEMORY) { // Release the least recently used entry @@ -104,11 +128,11 @@ CHIP_ERROR BindingManager::EstablishConnection(FabricIndex fabric, NodeId node) if (mPendingNotificationMap.FindLRUConnectPeer(&fabricToRemove, &nodeToRemove) == CHIP_NO_ERROR) { mPendingNotificationMap.RemoveAllEntriesForNode(fabricToRemove, nodeToRemove); - PeerId lruPeer = PeerIdForNode(mAppServer->GetFabricTable(), fabricToRemove, nodeToRemove); - mAppServer->GetCASESessionManager()->ReleaseSession(lruPeer); + PeerId lruPeer = PeerIdForNode(mInitParams.mFabricTable, fabricToRemove, nodeToRemove); + mInitParams.mCASESessionManager->ReleaseSession(lruPeer); // Now retry - error = mAppServer->GetCASESessionManager()->FindOrEstablishSession(peer, &mOnConnectedCallback, - &mOnConnectionFailureCallback); + error = + mInitParams.mCASESessionManager->FindOrEstablishSession(peer, &mOnConnectedCallback, &mOnConnectionFailureCallback); } } return error; @@ -131,7 +155,7 @@ void BindingManager::HandleDeviceConnected(OperationalDeviceProxy * device) { EmberBindingTableEntry entry = BindingTable::GetInstance().GetAt(pendingNotification.mBindingEntryId); - PeerId peer = PeerIdForNode(mAppServer->GetFabricTable(), entry.fabricIndex, entry.nodeId); + PeerId peer = PeerIdForNode(mInitParams.mFabricTable, entry.fabricIndex, entry.nodeId); if (device->GetPeerId() == peer) { fabricToRemove = entry.fabricIndex; @@ -152,13 +176,13 @@ void BindingManager::HandleDeviceConnectionFailure(PeerId peerId, CHIP_ERROR err { // Simply release the entry, the connection will be re-established as needed. ChipLogError(AppServer, "Failed to establish connection to node 0x" ChipLogFormatX64, ChipLogValueX64(peerId.GetNodeId())); - mAppServer->GetCASESessionManager()->ReleaseSession(peerId); + mInitParams.mCASESessionManager->ReleaseSession(peerId); } void BindingManager::FabricRemoved(CompressedFabricId compressedFabricId, FabricIndex fabricIndex) { mPendingNotificationMap.RemoveAllEntriesForFabric(fabricIndex); - mAppServer->GetCASESessionManager()->ReleaseSessionsForFabric(compressedFabricId); + mInitParams.mCASESessionManager->ReleaseSessionsForFabric(compressedFabricId); } CHIP_ERROR BindingManager::NotifyBindingAdded(const EmberBindingTableEntry & binding) @@ -172,7 +196,7 @@ CHIP_ERROR BindingManager::NotifyBindingAdded(const EmberBindingTableEntry & bin CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, ClusterId cluster, void * context) { - VerifyOrReturnError(mAppServer != nullptr, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mInitParams.mFabricTable != nullptr, CHIP_ERROR_INCORRECT_STATE); for (auto iter = BindingTable::GetInstance().begin(); iter != BindingTable::GetInstance().end(); ++iter) { @@ -180,10 +204,10 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste { if (iter->type == EMBER_UNICAST_BINDING) { - FabricInfo * fabricInfo = mAppServer->GetFabricTable().FindFabricWithIndex(iter->fabricIndex); + FabricInfo * fabricInfo = mInitParams.mFabricTable->FindFabricWithIndex(iter->fabricIndex); VerifyOrReturnError(fabricInfo != nullptr, CHIP_ERROR_NOT_FOUND); PeerId peer = fabricInfo->GetPeerIdForNode(iter->nodeId); - OperationalDeviceProxy * peerDevice = mAppServer->GetCASESessionManager()->FindExistingSession(peer); + OperationalDeviceProxy * peerDevice = mInitParams.mCASESessionManager->FindExistingSession(peer); if (peerDevice != nullptr && peerDevice->IsConnected() && mBoundDeviceChangedHandler) { // We already have an active connection diff --git a/src/app/clusters/bindings/BindingManager.h b/src/app/clusters/bindings/BindingManager.h index d337c57eb7fa59..0c392d9257ea9a 100644 --- a/src/app/clusters/bindings/BindingManager.h +++ b/src/app/clusters/bindings/BindingManager.h @@ -21,6 +21,8 @@ #include #include #include +#include +#include namespace chip { @@ -38,6 +40,13 @@ namespace chip { */ using BoundDeviceChangedHandler = void (*)(const EmberBindingTableEntry & binding, DeviceProxy * peer_device, void * context); +struct BindingManagerInitParams +{ + FabricTable * mFabricTable = nullptr; + CASESessionManager * mCASESessionManager = nullptr; + PersistentStorageDelegate * mStorage = nullptr; +}; + using BindingAddedHandler = void (*)(const EmberBindingTableEntry & binding); /** @@ -72,7 +81,7 @@ class BindingManager return CHIP_ERROR_INCORRECT_STATE; } - void SetAppServer(Server * appServer); + CHIP_ERROR Init(const BindingManagerInitParams & params); /* * Notifies the BindingManager that a new unicast binding is created. @@ -124,8 +133,8 @@ class BindingManager PendingNotificationMap mPendingNotificationMap; BoundDeviceChangedHandler mBoundDeviceChangedHandler; + BindingManagerInitParams mInitParams; BindingAddedHandler mBindingAddedHandler; - Server * mAppServer = nullptr; Callback::Callback mOnConnectedCallback; Callback::Callback mOnConnectionFailureCallback; diff --git a/src/app/clusters/bindings/bindings.cpp b/src/app/clusters/bindings/bindings.cpp index 234667982636ef..b94a513ace3ebb 100644 --- a/src/app/clusters/bindings/bindings.cpp +++ b/src/app/clusters/bindings/bindings.cpp @@ -101,6 +101,7 @@ void AddBindingEntry(const TargetStructType & entry, EndpointId localEndpoint) CHIP_ERROR err = BindingManager::GetInstance().UnicastBindingCreated(entry.fabricIndex, entry.node.Value()); if (err != CHIP_NO_ERROR) { + // Unicast connection failure can happen if peer is offline. We'll retry connection on-demand. ChipLogProgress( Zcl, "Binding: Failed to create session for unicast binding to device " ChipLogFormatX64 ": %" CHIP_ERROR_FORMAT, ChipLogValueX64(entry.node.Value()), err.Format()); @@ -187,7 +188,7 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath { BindingManager::GetInstance().UnicastBindingRemoved(bindingTableIter.GetIndex()); } - bindingTableIter = BindingTable::GetInstance().RemoveAt(bindingTableIter); + ReturnErrorOnFailure(BindingTable::GetInstance().RemoveAt(bindingTableIter)); } else { diff --git a/src/app/server/Server.h b/src/app/server/Server.h index c03af254406d3e..98c69d84bea4d7 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -92,6 +92,8 @@ class Server CommissioningWindowManager & GetCommissioningWindowManager() { return mCommissioningWindowManager; } + PersistentStorageDelegate & GetPersistentStorage() { return mDeviceStorage; } + /** * This function send the ShutDown event before stopping * the event loop. diff --git a/src/app/tests/TestBindingTable.cpp b/src/app/tests/TestBindingTable.cpp index 5a72d4e1213a97..389d5820dc92d1 100644 --- a/src/app/tests/TestBindingTable.cpp +++ b/src/app/tests/TestBindingTable.cpp @@ -16,6 +16,8 @@ */ #include +#include +#include #include #include @@ -27,6 +29,8 @@ namespace { void TestEmptyBindingTable(nlTestSuite * aSuite, void * aContext) { BindingTable table; + chip::TestPersistentStorageDelegate testStorage; + table.SetPersistentStorage(&testStorage); NL_TEST_ASSERT(aSuite, table.Size() == 0); NL_TEST_ASSERT(aSuite, table.begin() == table.end()); } @@ -34,7 +38,8 @@ void TestEmptyBindingTable(nlTestSuite * aSuite, void * aContext) void TestAdd(nlTestSuite * aSuite, void * aContext) { BindingTable table; - + chip::TestPersistentStorageDelegate testStorage; + table.SetPersistentStorage(&testStorage); EmberBindingTableEntry unusedEntry; unusedEntry.type = EMBER_UNUSED_BINDING; NL_TEST_ASSERT(aSuite, table.Add(unusedEntry) == CHIP_ERROR_INVALID_ARGUMENT); @@ -59,9 +64,12 @@ void TestAdd(nlTestSuite * aSuite, void * aContext) void TestRemoveThenAdd(nlTestSuite * aSuite, void * aContext) { BindingTable table; + chip::TestPersistentStorageDelegate testStorage; + table.SetPersistentStorage(&testStorage); NL_TEST_ASSERT(aSuite, table.Add(EmberBindingTableEntry::ForNode(0, 0, 0, 0, NullOptional)) == CHIP_NO_ERROR); auto iter = table.begin(); - NL_TEST_ASSERT(aSuite, table.RemoveAt(iter) == table.end()); + NL_TEST_ASSERT(aSuite, table.RemoveAt(iter) == CHIP_NO_ERROR); + NL_TEST_ASSERT(aSuite, iter == table.end()); NL_TEST_ASSERT(aSuite, table.Size() == 0); NL_TEST_ASSERT(aSuite, table.begin() == table.end()); for (uint8_t i = 0; i < EMBER_BINDING_TABLE_SIZE; i++) @@ -70,7 +78,7 @@ void TestRemoveThenAdd(nlTestSuite * aSuite, void * aContext) } iter = table.begin(); ++iter; - iter = table.RemoveAt(iter); + NL_TEST_ASSERT(aSuite, table.RemoveAt(iter) == CHIP_NO_ERROR); NL_TEST_ASSERT(aSuite, table.Size() == EMBER_BINDING_TABLE_SIZE - 1); NL_TEST_ASSERT(aSuite, iter->nodeId == 2); NL_TEST_ASSERT(aSuite, iter.GetIndex() == 2); @@ -90,7 +98,7 @@ void TestRemoveThenAdd(nlTestSuite * aSuite, void * aContext) ++iter; NL_TEST_ASSERT(aSuite, iter == table.end()); iter = table.begin(); - iter = table.RemoveAt(iter); + NL_TEST_ASSERT(aSuite, table.RemoveAt(iter) == CHIP_NO_ERROR); NL_TEST_ASSERT(aSuite, table.Size() == EMBER_BINDING_TABLE_SIZE - 1); NL_TEST_ASSERT(aSuite, iter == table.begin()); NL_TEST_ASSERT(aSuite, iter.GetIndex() == 2); @@ -98,6 +106,85 @@ void TestRemoveThenAdd(nlTestSuite * aSuite, void * aContext) NL_TEST_ASSERT(aSuite, table.GetAt(0).type == EMBER_UNUSED_BINDING); } +void VerifyTableSame(nlTestSuite * aSuite, BindingTable & table, const std::vector & expected) +{ + NL_TEST_ASSERT(aSuite, table.Size() == expected.size()); + auto iter1 = table.begin(); + auto iter2 = expected.begin(); + while (iter2 != expected.end()) + { + NL_TEST_ASSERT(aSuite, iter1 != table.end()); + NL_TEST_ASSERT(aSuite, *iter1 == *iter2); + ++iter1; + ++iter2; + } + NL_TEST_ASSERT(aSuite, iter1 == table.end()); +} + +void VerifyRestored(nlTestSuite * aSuite, chip::TestPersistentStorageDelegate & storage, + const std::vector & expected) +{ + BindingTable restoredTable; + restoredTable.SetPersistentStorage(&storage); + NL_TEST_ASSERT(aSuite, restoredTable.LoadFromStorage() == CHIP_NO_ERROR); + VerifyTableSame(aSuite, restoredTable, expected); +} + +void TestPersistentStorage(nlTestSuite * aSuite, void * aContext) +{ + chip::TestPersistentStorageDelegate testStorage; + BindingTable table; + chip::DefaultStorageKeyAllocator key; + chip::Optional cluster = chip::MakeOptional(static_cast(6)); + std::vector expected = { + EmberBindingTableEntry::ForNode(0, 0, 0, 0, NullOptional), + EmberBindingTableEntry::ForNode(1, 1, 0, 0, cluster), + EmberBindingTableEntry::ForGroup(2, 2, 0, NullOptional), + EmberBindingTableEntry::ForGroup(3, 3, 0, cluster), + }; + table.SetPersistentStorage(&testStorage); + NL_TEST_ASSERT(aSuite, table.Add(expected[0]) == CHIP_NO_ERROR); + NL_TEST_ASSERT(aSuite, table.Add(expected[1]) == CHIP_NO_ERROR); + NL_TEST_ASSERT(aSuite, table.Add(expected[2]) == CHIP_NO_ERROR); + NL_TEST_ASSERT(aSuite, table.Add(expected[3]) == CHIP_NO_ERROR); + VerifyRestored(aSuite, testStorage, expected); + + // Verify storage untouched if add fails + testStorage.AddErrorKey(key.BindingTableEntry(4)); + NL_TEST_ASSERT(aSuite, table.Add(EmberBindingTableEntry::ForNode(4, 4, 0, 0, NullOptional)) != CHIP_NO_ERROR); + VerifyRestored(aSuite, testStorage, expected); + testStorage.ClearErrorKey(); + + // Verify storage untouched if removing head fails + testStorage.AddErrorKey(key.BindingTable()); + auto iter = table.begin(); + NL_TEST_ASSERT(aSuite, table.RemoveAt(iter) != CHIP_NO_ERROR); + VerifyTableSame(aSuite, table, expected); + testStorage.ClearErrorKey(); + VerifyRestored(aSuite, testStorage, expected); + + // Verify storage untouched if removing other nodes fails + testStorage.AddErrorKey(key.BindingTableEntry(0)); + iter = table.begin(); + ++iter; + NL_TEST_ASSERT(aSuite, table.RemoveAt(iter) != CHIP_NO_ERROR); + VerifyTableSame(aSuite, table, expected); + testStorage.ClearErrorKey(); + VerifyRestored(aSuite, testStorage, expected); + + // Verify removing head + iter = table.begin(); + NL_TEST_ASSERT(aSuite, table.RemoveAt(iter) == CHIP_NO_ERROR); + VerifyTableSame(aSuite, table, { expected[1], expected[2], expected[3] }); + VerifyRestored(aSuite, testStorage, { expected[1], expected[2], expected[3] }); + + // Verify removing other nodes + ++iter; + NL_TEST_ASSERT(aSuite, table.RemoveAt(iter) == CHIP_NO_ERROR); + VerifyTableSame(aSuite, table, { expected[1], expected[3] }); + VerifyRestored(aSuite, testStorage, { expected[1], expected[3] }); +} + } // namespace int TestBindingTable() @@ -106,6 +193,7 @@ int TestBindingTable() NL_TEST_DEF("TestEmptyBindingTable", TestEmptyBindingTable), NL_TEST_DEF("TestAdd", TestAdd), NL_TEST_DEF("TestRemoveThenAdd", TestRemoveThenAdd), + NL_TEST_DEF("TestPersistentStorage", TestPersistentStorage), NL_TEST_SENTINEL(), }; diff --git a/src/app/tests/TestPendingNotificationMap.cpp b/src/app/tests/TestPendingNotificationMap.cpp index 7a5b730502e622..a622b2da701ce8 100644 --- a/src/app/tests/TestPendingNotificationMap.cpp +++ b/src/app/tests/TestPendingNotificationMap.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -36,7 +37,7 @@ void ClearBindingTable(BindingTable & table) auto iter = table.begin(); while (iter != table.end()) { - iter = table.RemoveAt(iter); + table.RemoveAt(iter); } } @@ -140,6 +141,8 @@ int TestPeindingNotificationMap() nullptr, nullptr, }; + chip::TestPersistentStorageDelegate storage; + BindingTable::GetInstance().SetPersistentStorage(&storage); nlTestRunner(&theSuite, nullptr); return (nlTestRunnerStats(&theSuite)); } diff --git a/src/app/util/binding-table.cpp b/src/app/util/binding-table.cpp index 1b3bf4a2000142..ea15e94e68b497 100644 --- a/src/app/util/binding-table.cpp +++ b/src/app/util/binding-table.cpp @@ -27,7 +27,7 @@ BindingTable BindingTable::sInstance; BindingTable::BindingTable() { - memset(mNextIndex, EMBER_BINDING_TABLE_SIZE, sizeof(mNextIndex)); + memset(mNextIndex, kNextNullIndex, sizeof(mNextIndex)); } CHIP_ERROR BindingTable::Add(const EmberBindingTableEntry & entry) @@ -42,18 +42,45 @@ CHIP_ERROR BindingTable::Add(const EmberBindingTableEntry & entry) return CHIP_ERROR_NO_MEMORY; } mBindingTable[newIndex] = entry; - if (mTail == EMBER_BINDING_TABLE_SIZE) + CHIP_ERROR error = SaveEntryToStorage(newIndex, kNextNullIndex); + if (error == CHIP_NO_ERROR) { - mTail = newIndex; - mHead = newIndex; + if (mTail == kNextNullIndex) + { + error = SaveListInfo(newIndex); + } + else + { + error = SaveEntryToStorage(mTail, newIndex); + } + if (error != CHIP_NO_ERROR) + { + mStorage->SyncDeleteKeyValue(mKeyAllocator.BindingTableEntry(newIndex)); + } + } + if (error != CHIP_NO_ERROR) + { + // Roll back + mBindingTable[newIndex].type = EMBER_UNUSED_BINDING; + return error; } else { - mNextIndex[mTail] = newIndex; - mTail = newIndex; + if (mTail == kNextNullIndex) + { + mTail = newIndex; + mHead = newIndex; + } + else + { + mNextIndex[mTail] = newIndex; + mNextIndex[newIndex] = kNextNullIndex; + mTail = newIndex; + } + + mSize++; + return CHIP_NO_ERROR; } - mSize++; - return CHIP_NO_ERROR; } const EmberBindingTableEntry & BindingTable::GetAt(uint8_t index) @@ -61,11 +88,154 @@ const EmberBindingTableEntry & BindingTable::GetAt(uint8_t index) return mBindingTable[index]; } -BindingTable::Iterator BindingTable::RemoveAt(Iterator iter) +CHIP_ERROR BindingTable::SaveEntryToStorage(uint8_t index, uint8_t nextIndex) +{ + EmberBindingTableEntry & entry = mBindingTable[index]; + uint8_t buffer[kEntryStorageSize] = { 0 }; + TLV::TLVWriter writer; + writer.Init(buffer); + TLV::TLVType container; + ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::TLVType::kTLVType_Structure, container)); + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(kTagFabricIndex), entry.fabricIndex)); + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(kTagLocalEndpoint), entry.local)); + if (entry.clusterId.HasValue()) + { + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(kTagCluster), entry.clusterId.Value())); + } + if (entry.type == EMBER_UNICAST_BINDING) + { + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(kTagRemoteEndpoint), entry.remote)); + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(kTagNodeId), entry.nodeId)); + } + else + { + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(kTagGroupId), entry.groupId)); + } + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(kTagNextEntry), nextIndex)); + ReturnErrorOnFailure(writer.EndContainer(container)); + ReturnErrorOnFailure(writer.Finalize()); + return mStorage->SyncSetKeyValue(mKeyAllocator.BindingTableEntry(index), buffer, + static_cast(writer.GetLengthWritten())); +} + +CHIP_ERROR BindingTable::SaveListInfo(uint8_t head) { - if (iter.mTable != this || iter.mIndex == EMBER_BINDING_TABLE_SIZE) + uint8_t buffer[kListInfoStorageSize] = { 0 }; + TLV::TLVWriter writer; + writer.Init(buffer); + TLV::TLVType container; + ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::TLVType::kTLVType_Structure, container)); + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(kTagStorageVersion), kStorageVersion)); + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(kTagHead), head)); + ReturnErrorOnFailure(writer.EndContainer(container)); + ReturnErrorOnFailure(writer.Finalize()); + return mStorage->SyncSetKeyValue(mKeyAllocator.BindingTable(), buffer, static_cast(writer.GetLengthWritten())); +} + +CHIP_ERROR BindingTable::LoadFromStorage() +{ + VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE); + uint8_t buffer[kListInfoStorageSize] = { 0 }; + uint16_t size = sizeof(buffer); + CHIP_ERROR error; + + ReturnErrorOnFailure(mStorage->SyncGetKeyValue(mKeyAllocator.BindingTable(), buffer, size)); + TLV::TLVReader reader; + reader.Init(buffer, size); + + ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::AnonymousTag())); + + TLV::TLVType container; + ReturnErrorOnFailure(reader.EnterContainer(container)); + + ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagStorageVersion))); + uint32_t version; + ReturnErrorOnFailure(reader.Get(version)); + VerifyOrReturnError(version == kStorageVersion, CHIP_ERROR_VERSION_MISMATCH); + ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagHead))); + uint8_t index; + ReturnErrorOnFailure(reader.Get(index)); + mHead = index; + while (index != kNextNullIndex) { - return iter; + uint8_t nextIndex; + error = LoadEntryFromStorage(index, nextIndex); + if (error != CHIP_NO_ERROR) + { + mHead = kNextNullIndex; + mTail = kNextNullIndex; + return error; + } + mTail = index; + index = nextIndex; + mSize++; + } + error = reader.ExitContainer(container); + if (error != CHIP_NO_ERROR) + { + mHead = kNextNullIndex; + mTail = kNextNullIndex; + } + return error; +} + +CHIP_ERROR BindingTable::LoadEntryFromStorage(uint8_t index, uint8_t & nextIndex) +{ + uint8_t buffer[kEntryStorageSize] = { 0 }; + uint16_t size = sizeof(buffer); + EmberBindingTableEntry entry; + + ReturnErrorOnFailure(mStorage->SyncGetKeyValue(mKeyAllocator.BindingTableEntry(index), buffer, size)); + TLV::TLVReader reader; + reader.Init(buffer, size); + + ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::AnonymousTag())); + + TLV::TLVType container; + ReturnErrorOnFailure(reader.EnterContainer(container)); + ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagFabricIndex))); + ReturnErrorOnFailure(reader.Get(entry.fabricIndex)); + ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagLocalEndpoint))); + ReturnErrorOnFailure(reader.Get(entry.local)); + ReturnErrorOnFailure(reader.Next()); + if (reader.GetTag() == TLV::ContextTag(kTagCluster)) + { + uint16_t clusterId; + ReturnErrorOnFailure(reader.Get(clusterId)); + entry.clusterId.SetValue(clusterId); + ReturnErrorOnFailure(reader.Next()); + } + else + { + entry.clusterId = NullOptional; + } + if (reader.GetTag() == TLV::ContextTag(kTagRemoteEndpoint)) + { + entry.type = EMBER_UNICAST_BINDING; + ReturnErrorOnFailure(reader.Get(entry.remote)); + ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagNodeId))); + ReturnErrorOnFailure(reader.Get(entry.nodeId)); + } + else + { + entry.type = EMBER_MULTICAST_BINDING; + ReturnErrorCodeIf(reader.GetTag() != TLV::ContextTag(kTagGroupId), CHIP_ERROR_INVALID_TLV_TAG); + ReturnErrorOnFailure(reader.Get(entry.groupId)); + } + ReturnErrorOnFailure(reader.Next(TLV::ContextTag(kTagNextEntry))); + ReturnErrorOnFailure(reader.Get(nextIndex)); + ReturnErrorOnFailure(reader.ExitContainer(container)); + mBindingTable[index] = entry; + mNextIndex[index] = nextIndex; + return CHIP_NO_ERROR; +} + +CHIP_ERROR BindingTable::RemoveAt(Iterator & iter) +{ + CHIP_ERROR error; + if (iter.mTable != this || iter.mIndex == kNextNullIndex) + { + return CHIP_ERROR_INVALID_ARGUMENT; } if (iter.mIndex == mTail) { @@ -74,24 +244,40 @@ BindingTable::Iterator BindingTable::RemoveAt(Iterator iter) uint8_t next = mNextIndex[iter.mIndex]; if (iter.mIndex != mHead) { - mNextIndex[iter.mPrevIndex] = next; + error = SaveEntryToStorage(iter.mPrevIndex, next); + if (error == CHIP_NO_ERROR) + { + mNextIndex[iter.mPrevIndex] = next; + } } else { - mHead = next; + error = SaveListInfo(next); + if (error == CHIP_NO_ERROR) + { + mHead = next; + } + } + if (error == CHIP_NO_ERROR) + { + // The remove is considered "submitted" once the change on prev node takes effect + if (mStorage->SyncDeleteKeyValue(mKeyAllocator.BindingTableEntry(iter.mIndex)) != CHIP_NO_ERROR) + { + ChipLogError(AppServer, "Failed to remove binding table entry %u from storage", iter.mIndex); + } + mBindingTable[iter.mIndex].type = EMBER_UNUSED_BINDING; + mNextIndex[iter.mIndex] = kNextNullIndex; + mSize--; } - mBindingTable[iter.mIndex].type = EMBER_UNUSED_BINDING; - mNextIndex[iter.mIndex] = EMBER_BINDING_TABLE_SIZE; - mSize--; iter.mIndex = next; - return iter; + return error; } BindingTable::Iterator BindingTable::begin() { Iterator iter; iter.mTable = this; - iter.mPrevIndex = EMBER_BINDING_TABLE_SIZE; + iter.mPrevIndex = kNextNullIndex; iter.mIndex = mHead; return iter; } @@ -100,7 +286,7 @@ BindingTable::Iterator BindingTable::end() { Iterator iter; iter.mTable = this; - iter.mIndex = EMBER_BINDING_TABLE_SIZE; + iter.mIndex = kNextNullIndex; return iter; } @@ -118,7 +304,7 @@ uint8_t BindingTable::GetNextAvaiableIndex() BindingTable::Iterator BindingTable::Iterator::operator++() { - if (mIndex != EMBER_BINDING_TABLE_SIZE) + if (mIndex != kNextNullIndex) { mPrevIndex = mIndex; mIndex = mTable->mNextIndex[mIndex]; diff --git a/src/app/util/binding-table.h b/src/app/util/binding-table.h index 948711d51b86da..1ae270fd109035 100644 --- a/src/app/util/binding-table.h +++ b/src/app/util/binding-table.h @@ -22,6 +22,9 @@ #pragma once #include +#include +#include +#include namespace chip { @@ -63,9 +66,8 @@ class BindingTable const EmberBindingTableEntry & GetAt(uint8_t index); - // The RemoveAt function shares the same sematics as the std::list::remove. - // It returns the next iterator after removal and the old iterator is no loger valid. - Iterator RemoveAt(Iterator iter); + // The iter will be moved to the next item in the table after calling RemoveAt. + CHIP_ERROR RemoveAt(Iterator & iter); // Returns the number of active entries in the binding table. // *NOTE* The function does not return the capacity of the binding table. @@ -75,19 +77,47 @@ class BindingTable Iterator end(); + void SetPersistentStorage(PersistentStorageDelegate * storage) { mStorage = storage; } + + CHIP_ERROR LoadFromStorage(); + static BindingTable & GetInstance() { return sInstance; } private: static BindingTable sInstance; + static constexpr uint32_t kStorageVersion = 1; + static constexpr uint8_t kEntryStorageSize = TLV::EstimateStructOverhead( + sizeof(FabricIndex), sizeof(EndpointId), sizeof(ClusterId), sizeof(EndpointId), sizeof(NodeId), sizeof(uint8_t)); + static constexpr uint8_t kListInfoStorageSize = TLV::EstimateStructOverhead(sizeof(kStorageVersion), sizeof(uint8_t)); + + static constexpr uint8_t kTagStorageVersion = 1; + static constexpr uint8_t kTagHead = 2; + static constexpr uint8_t kTagFabricIndex = 1; + static constexpr uint8_t kTagLocalEndpoint = 2; + static constexpr uint8_t kTagCluster = 3; + static constexpr uint8_t kTagRemoteEndpoint = 4; + static constexpr uint8_t kTagNodeId = 5; + static constexpr uint8_t kTagGroupId = 6; + static constexpr uint8_t kTagNextEntry = 7; + static constexpr uint8_t kNextNullIndex = 255; + uint8_t GetNextAvaiableIndex(); + CHIP_ERROR SaveEntryToStorage(uint8_t index, uint8_t nextIndex); + CHIP_ERROR SaveListInfo(uint8_t head); + + CHIP_ERROR LoadEntryFromStorage(uint8_t index, uint8_t & nextIndex); + EmberBindingTableEntry mBindingTable[EMBER_BINDING_TABLE_SIZE]; uint8_t mNextIndex[EMBER_BINDING_TABLE_SIZE]; - uint8_t mHead = EMBER_BINDING_TABLE_SIZE; - uint8_t mTail = EMBER_BINDING_TABLE_SIZE; + uint8_t mHead = kNextNullIndex; + uint8_t mTail = kNextNullIndex; uint8_t mSize = 0; + + PersistentStorageDelegate * mStorage; + DefaultStorageKeyAllocator mKeyAllocator; }; } // namespace chip diff --git a/src/app/util/types_stub.h b/src/app/util/types_stub.h index ca381d3f0fbfd5..afd75dbb31d89d 100644 --- a/src/app/util/types_stub.h +++ b/src/app/util/types_stub.h @@ -565,12 +565,12 @@ struct EmberBindingTableEntry return false; } - if (type == EMBER_UNICAST_BINDING && nodeId != other.nodeId) + if (type == EMBER_UNICAST_BINDING && (nodeId != other.nodeId || remote != other.remote)) { return false; } - return fabricIndex == other.fabricIndex && local == other.local && clusterId == other.clusterId && remote == other.remote; + return fabricIndex == other.fabricIndex && local == other.local && clusterId == other.clusterId; } }; diff --git a/src/lib/support/DefaultStorageKeyAllocator.h b/src/lib/support/DefaultStorageKeyAllocator.h index 2297b69dc0e55b..0ba0f4ebe2b7bf 100644 --- a/src/lib/support/DefaultStorageKeyAllocator.h +++ b/src/lib/support/DefaultStorageKeyAllocator.h @@ -69,6 +69,9 @@ class DefaultStorageKeyAllocator return Format("a/%" PRIx16 "/%" PRIx32 "/%" PRIx32, aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId); } + const char * BindingTable() { return Format("bt"); } + const char * BindingTableEntry(uint8_t index) { return Format("bt/%x", index); } + private: static const size_t kKeyLengthMax = 32; diff --git a/src/lib/support/TestPersistentStorageDelegate.h b/src/lib/support/TestPersistentStorageDelegate.h index 899501b3dbd0e5..85ae1b8b772141 100644 --- a/src/lib/support/TestPersistentStorageDelegate.h +++ b/src/lib/support/TestPersistentStorageDelegate.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -36,6 +37,10 @@ class TestPersistentStorageDelegate : public PersistentStorageDelegate, public F CHIP_ERROR SyncGetKeyValue(const char * key, void * buffer, uint16_t & size) override { + if (mErrorKeys.find(std::string(key)) != mErrorKeys.end()) + { + return CHIP_ERROR_PERSISTED_STORAGE_FAILED; + } bool contains = mStorage.find(key) != mStorage.end(); VerifyOrReturnError(contains, CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND); @@ -50,6 +55,10 @@ class TestPersistentStorageDelegate : public PersistentStorageDelegate, public F CHIP_ERROR SyncSetKeyValue(const char * key, const void * value, uint16_t size) override { + if (mErrorKeys.find(std::string(key)) != mErrorKeys.end()) + { + return CHIP_ERROR_PERSISTED_STORAGE_FAILED; + } const uint8_t * bytes = static_cast(value); mStorage[key] = std::vector(bytes, bytes + size); return CHIP_NO_ERROR; @@ -57,6 +66,10 @@ class TestPersistentStorageDelegate : public PersistentStorageDelegate, public F CHIP_ERROR SyncDeleteKeyValue(const char * key) override { + if (mErrorKeys.find(std::string(key)) != mErrorKeys.end()) + { + return CHIP_ERROR_PERSISTED_STORAGE_FAILED; + } bool contains = mStorage.find(key) != mStorage.end(); VerifyOrReturnError(contains, CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND); mStorage.erase(key); @@ -75,8 +88,13 @@ class TestPersistentStorageDelegate : public PersistentStorageDelegate, public F CHIP_ERROR SyncDelete(FabricIndex fabricIndex, const char * key) override { return SyncDeleteKeyValue(key); }; + void AddErrorKey(const std::string & key) { mErrorKeys.insert(key); } + + void ClearErrorKey() { mErrorKeys.clear(); } + protected: std::map> mStorage; + std::set mErrorKeys; }; } // namespace chip