Update GenericThreadBorderRouterDelegate and add the function description for thread-br-delegate

Author: wqx6

src/app/clusters/thread-border-router-management-server/thread-br-delegate.h

@@ -69,10 +69,14 @@ class Delegate
 
     virtual CHIP_ERROR GetDataset(Thread::OperationalDataset & dataset, DatasetType type) = 0;
 
+    // The Delegate implement should back up the old active dataset at the begining of function if no backup already exists.
     virtual CHIP_ERROR SetActiveDataset(const Thread::OperationalDataset & activeDataset, ActivateDatasetCallback * callback) = 0;
 
-    // The Thread BR should be able to revert the active dataset set by SetActiveDataset(), and restore
-    // the previous status of Thread BR.
+    // The Delegate implement should remove the backup active dataset in this function.
+    virtual CHIP_ERROR CommitActiveDataset() = 0;
+
+    // The Delegate implement should restore the backup active dataset in this function.
+    // Once RevertActiveDataset is called the ActivateDatasetCallback of previous SetActiveDataset should never be called.
     virtual CHIP_ERROR RevertActiveDataset() = 0;
 
     virtual CHIP_ERROR SetPendingDataset(const Thread::OperationalDataset & pendingDataset) = 0;

src/platform/OpenThread/GenericThreadBorderRouterDelegate.cpp

@@ -121,35 +121,16 @@ CHIP_ERROR GenericThreadBorderRouterDelegate::GetDataset(Thread::OperationalData
 CHIP_ERROR GenericThreadBorderRouterDelegate::SetActiveDataset(const Thread::OperationalDataset & activeDataset,
                                                                ActivateDatasetCallback * callback)
 {
-    otInstance * otInst = DeviceLayer::ThreadStackMgrImpl().OTInstance();
-    VerifyOrReturnError(otInst, CHIP_ERROR_INCORRECT_STATE);
-
-    VerifyOrReturnError(callback, CHIP_ERROR_INVALID_ARGUMENT);
-    otOperationalDatasetTlvs datasetTlvs;
-    memcpy(datasetTlvs.mTlvs, activeDataset.AsByteSpan().data(), activeDataset.AsByteSpan().size());
-    datasetTlvs.mLength = activeDataset.AsByteSpan().size();
-
-    ScopedThreadLock threadLock;
-    // Save the previous thread state and dataset for reverting
-    bool threadIsEnabled = GetThreadEnabled();
-    ReturnErrorOnFailure(DeviceLayer::PersistedStorage::KeyValueStoreMgr().Put(kFailsafeThreadEnabledKey, threadIsEnabled));
-    if (threadIsEnabled)
+    CHIP_ERROR err = BackupActiveDataset();
+    if (err == CHIP_NO_ERROR)
     {
-        otOperationalDatasetTlvs stagingDataset;
-        ReturnErrorCodeIf(otDatasetGetActiveTlvs(otInst, &stagingDataset) != OT_ERROR_NONE, CHIP_ERROR_INTERNAL);
-        if (activeDataset.AsByteSpan().data_equal(ByteSpan(stagingDataset.mTlvs, stagingDataset.mLength)))
-        {
-            callback->OnActivateDatasetComplete(CHIP_NO_ERROR);
-            return CHIP_NO_ERROR;
-        }
-        ReturnErrorOnFailure(DeviceLayer::PersistedStorage::KeyValueStoreMgr().Put(kFailsafeThreadDatasetTlvsKey,
-                                                                                   stagingDataset.mTlvs, stagingDataset.mLength));
+        err = DeviceLayer::ThreadStackMgrImpl().AttachToThreadNetwork(activeDataset, nullptr);
     }
-    SetThreadEnabled(false);
-    ReturnErrorCodeIf(otDatasetSetActiveTlvs(otInst, &datasetTlvs) != OT_ERROR_NONE, CHIP_ERROR_INTERNAL);
-    SetThreadEnabled(true);
-    mCallback = callback;
-    return CHIP_NO_ERROR;
+    if (err == CHIP_NO_ERROR)
+    {
+        mCallback = callback;
+    }
+    return err;
 }
 
 void GenericThreadBorderRouterDelegate::OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg)
@@ -163,45 +144,53 @@ void GenericThreadBorderRouterDelegate::OnPlatformEventHandler(const DeviceLayer
             delegate->mCallback->OnActivateDatasetComplete(CHIP_NO_ERROR);
             // Delete Failsafe Keys after activating dataset is completed
             DeviceLayer::PersistedStorage::KeyValueStoreMgr().Delete(kFailsafeThreadDatasetTlvsKey);
-            DeviceLayer::PersistedStorage::KeyValueStoreMgr().Delete(kFailsafeThreadEnabledKey);
             delegate->mCallback = nullptr;
         }
     }
 }
 
-CHIP_ERROR GenericThreadBorderRouterDelegate::RevertActiveDataset()
+CHIP_ERROR GenericThreadBorderRouterDelegate::BackupActiveDataset()
 {
-    otInstance * otInst = DeviceLayer::ThreadStackMgrImpl().OTInstance();
-    VerifyOrReturnError(otInst, CHIP_ERROR_INCORRECT_STATE);
-
-    bool threadIsEnabled = false;
-    CHIP_ERROR err       = DeviceLayer::PersistedStorage::KeyValueStoreMgr().Get(kFailsafeThreadEnabledKey, &threadIsEnabled);
-    if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND)
+    // If active dataset is already backed up, return with no error
+    CHIP_ERROR err = DeviceLayer::PersistedStorage::KeyValueStoreMgr().Get(kFailsafeThreadDatasetTlvsKey, nullptr, 0);
+    if (err == CHIP_NO_ERROR || err == CHIP_ERROR_BUFFER_TOO_SMALL)
     {
         return CHIP_NO_ERROR;
     }
-    ReturnErrorOnFailure(err);
+    GetDataset(mStagingDataset, DatasetType::kActive);
+    ByteSpan dataset = mStagingDataset.AsByteSpan();
+    return DeviceLayer::PersistedStorage::KeyValueStoreMgr().Put(kFailsafeThreadDatasetTlvsKey, dataset.data(), dataset.size());
+}
 
-    ScopedThreadLock threadLock;
-    if (threadIsEnabled)
+CHIP_ERROR GenericThreadBorderRouterDelegate::CommitActiveDataset()
+{
+    // Delete Failsafe Key when committing.
+    DeviceLayer::PersistedStorage::KeyValueStoreMgr().Delete(kFailsafeThreadDatasetTlvsKey);
+    return CHIP_NO_ERROR;
+}
+
+CHIP_ERROR GenericThreadBorderRouterDelegate::RevertActiveDataset()
+{
+    // The FailSafe Timer is triggered and the previous command request should be handled, so reset the callback.
+    mCallback = nullptr;
+    uint8_t datasetBytes[Thread::kSizeOperationalDataset];
+    size_t datasetLength;
+    CHIP_ERROR err = DeviceLayer::PersistedStorage::KeyValueStoreMgr().Get(kFailsafeThreadDatasetTlvsKey, datasetBytes,
+                                                                           sizeof(datasetBytes), &datasetLength);
+    // If no backup could be found, it means the active datset has not been modified since the fail-safe was armed,
+    // so return with no error.
+    ReturnErrorCodeIf(err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND, CHIP_NO_ERROR);
+    if (err == CHIP_NO_ERROR)
     {
-        otOperationalDatasetTlvs stagingDataset;
-        size_t datasetTlvslen = 0;
-        err = DeviceLayer::PersistedStorage::KeyValueStoreMgr().Get(kFailsafeThreadDatasetTlvsKey, stagingDataset.mTlvs,
-                                                                    sizeof(stagingDataset.mTlvs), &datasetTlvslen);
-        if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND)
-        {
-            return CHIP_NO_ERROR;
-        }
-        ReturnErrorOnFailure(err);
-        stagingDataset.mLength = datasetTlvslen;
-        ReturnErrorOnFailure(SetThreadEnabled(false));
-        ReturnErrorCodeIf(otDatasetSetActiveTlvs(otInst, &stagingDataset) != OT_ERROR_NONE, CHIP_ERROR_INTERNAL);
+        err = mStagingDataset.Init(ByteSpan(datasetBytes, datasetLength));
+    }
+    if (err == CHIP_NO_ERROR)
+    {
+        err = DeviceLayer::ThreadStackMgrImpl().AttachToThreadNetwork(mStagingDataset, nullptr);
     }
-    ReturnErrorOnFailure(SetThreadEnabled(threadIsEnabled));
-    // Delete Failsafe Keys after reverting.
+
+    // Always delete the backup, regardless if it can be successfully restored.
     DeviceLayer::PersistedStorage::KeyValueStoreMgr().Delete(kFailsafeThreadDatasetTlvsKey);
-    DeviceLayer::PersistedStorage::KeyValueStoreMgr().Delete(kFailsafeThreadEnabledKey);
     return CHIP_NO_ERROR;
 }
 
@@ -218,33 +207,6 @@ CHIP_ERROR GenericThreadBorderRouterDelegate::SetPendingDataset(const Thread::Op
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR GenericThreadBorderRouterDelegate::SetThreadEnabled(bool enabled)
-{
-    otInstance * otInst = DeviceLayer::ThreadStackMgrImpl().OTInstance();
-    VerifyOrReturnError(otInst, CHIP_ERROR_INCORRECT_STATE);
-    bool isEnabled    = (otThreadGetDeviceRole(otInst) != OT_DEVICE_ROLE_DISABLED);
-    bool isIp6Enabled = otIp6IsEnabled(otInst);
-    if (enabled && !isIp6Enabled)
-    {
-        ReturnErrorCodeIf(otIp6SetEnabled(otInst, enabled) != OT_ERROR_NONE, CHIP_ERROR_INTERNAL);
-    }
-    if (enabled != isEnabled)
-    {
-        ReturnErrorCodeIf(otThreadSetEnabled(otInst, enabled) != OT_ERROR_NONE, CHIP_ERROR_INTERNAL);
-    }
-    if (!enabled && isIp6Enabled)
-    {
-        ReturnErrorCodeIf(otIp6SetEnabled(otInst, enabled) != OT_ERROR_NONE, CHIP_ERROR_INTERNAL);
-    }
-    return CHIP_NO_ERROR;
-}
-
-bool GenericThreadBorderRouterDelegate::GetThreadEnabled()
-{
-    otInstance * otInst = DeviceLayer::ThreadStackMgrImpl().OTInstance();
-    return otInst && otIp6IsEnabled(otInst) && (otThreadGetDeviceRole(otInst) != OT_DEVICE_ROLE_DISABLED);
-}
-
 } // namespace ThreadBorderRouterManagement
 } // namespace Clusters
 } // namespace app

src/platform/OpenThread/GenericThreadBorderRouterDelegate.h

@@ -34,7 +34,6 @@ class GenericThreadBorderRouterDelegate : public Delegate
 public:
     static constexpr char kThreadBorderRourterName[]      = "Espressif-ThreadBR";
     static constexpr char kFailsafeThreadDatasetTlvsKey[] = "g/fs/td";
-    static constexpr char kFailsafeThreadEnabledKey[]     = "g/fs/te";
 
     GenericThreadBorderRouterDelegate()  = default;
     ~GenericThreadBorderRouterDelegate() = default;
@@ -70,14 +69,16 @@ class GenericThreadBorderRouterDelegate : public Delegate
 
     CHIP_ERROR RevertActiveDataset() override;
 
+    CHIP_ERROR CommitActiveDataset() override;
+
     CHIP_ERROR SetPendingDataset(const Thread::OperationalDataset & pendingDataset) override;
 
     static void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg);
 
 private:
-    CHIP_ERROR SetThreadEnabled(bool enabled);
-    bool GetThreadEnabled();
+    CHIP_ERROR BackupActiveDataset();
     ActivateDatasetCallback * mCallback = nullptr;
+    Thread::OperationalDataset mStagingDataset = {};
 };
 } // namespace ThreadBorderRouterManagement
 } // namespace Clusters