Skip to content

Commit e567348

Browse files
wqx6wqx6
committed
review changes
1 parent 99cfacb commit e567348

File tree

5 files changed

+67
-96
lines changed

5 files changed

+67
-96
lines changed

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

+11-6
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ class Delegate
4646
virtual void OnActivateDatasetComplete(CHIP_ERROR error) = 0;
4747
};
4848

49+
enum class DatasetType : uint8_t
50+
{
51+
kActive,
52+
kPending,
53+
};
54+
4955
virtual CHIP_ERROR Init() = 0;
5056

5157
virtual CHIP_ERROR GetPanChangeSupported(bool & panChangeSupported) = 0;
@@ -58,16 +64,15 @@ class Delegate
5864

5965
virtual CHIP_ERROR GetInterfaceEnabled(bool & interfaceEnabled) = 0;
6066

61-
virtual CHIP_ERROR GetActiveDataset(chip::Thread::OperationalDataset & activeDataset) = 0;
62-
63-
virtual CHIP_ERROR GetPendingDataset(chip::Thread::OperationalDataset & pendingDataset) = 0;
67+
virtual CHIP_ERROR GetDataset(Thread::OperationalDataset & dataset, DatasetType type) = 0;
6468

65-
virtual CHIP_ERROR SetActiveDataset(const chip::Thread::OperationalDataset & activeDataset,
66-
ActivateDatasetCallback * callback) = 0;
69+
virtual CHIP_ERROR SetActiveDataset(const Thread::OperationalDataset & activeDataset, ActivateDatasetCallback * callback) = 0;
6770

71+
// The Thread BR should be able to revert the active dataset set by SetActiveDataset(), and restore
72+
// the previous status of Thread BR.
6873
virtual CHIP_ERROR RevertActiveDataset() = 0;
6974

70-
virtual CHIP_ERROR SetPendingDataset(const chip::Thread::OperationalDataset & pendingDataset) = 0;
75+
virtual CHIP_ERROR SetPendingDataset(const Thread::OperationalDataset & pendingDataset) = 0;
7176
};
7277

7378
} // namespace ThreadBorderRouterManagement

‎src/app/clusters/thread-border-router-management-server/thread-br-mgmt-server.cpp

+33-60
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "app/CommandHandler.h"
2727
#include "app/CommandHandlerInterface.h"
2828
#include "app/InteractionModelEngine.h"
29+
#include "app/MessageDef/StatusIB.h"
2930
#include "app/clusters/general-commissioning-server/general-commissioning-server.h"
3031
#include "app/data-model/Nullable.h"
3132
#include "app/server/Server.h"
@@ -46,25 +47,25 @@ using Protocols::InteractionModel::Status;
4647

4748
static bool CheckOverCASESession(CommandHandlerInterface::HandlerContext & ctx)
4849
{
49-
chip::Messaging::ExchangeContext * exchangeCtx = ctx.mCommandHandler.GetExchangeContext();
50+
Messaging::ExchangeContext * exchangeCtx = ctx.mCommandHandler.GetExchangeContext();
5051
if (!exchangeCtx || !exchangeCtx->HasSessionHandle() || !exchangeCtx->GetSessionHandle()->IsSecureSession() ||
5152
exchangeCtx->GetSessionHandle()->AsSecureSession()->GetSecureSessionType() != Transport::SecureSession::Type::kCASE)
5253
{
5354
ChipLogError(Zcl, "This command MUST be over a valid CASE session");
54-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::UnsupportedAccess);
55+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::UnsupportedAccess);
5556
return false;
5657
}
5758
return true;
5859
}
5960

6061
static bool CheckFailSafeArmed(CommandHandlerInterface::HandlerContext & ctx)
6162
{
62-
auto & failSafeContext = chip::Server::GetInstance().GetFailSafeContext();
63+
auto & failSafeContext = Server::GetInstance().GetFailSafeContext();
6364
if (failSafeContext.IsFailSafeArmed(ctx.mCommandHandler.GetAccessingFabricIndex()))
6465
{
6566
return true;
6667
}
67-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::FailsafeRequired);
68+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::FailsafeRequired);
6869
return false;
6970
}
7071

@@ -73,64 +74,42 @@ static bool CheckDelegate(CommandHandlerInterface::HandlerContext & ctx, Delegat
7374
if (!delegate)
7475
{
7576
ChipLogError(Zcl, "Thread Border Router Management server not initialized");
76-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::InvalidInState);
77+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::InvalidInState);
7778
}
7879
return delegate;
7980
}
8081

81-
void ServerInstance::HandleGetActiveDatasetRequest(HandlerContext & ctx,
82-
const Commands::GetActiveDatasetRequest::DecodableType & req)
83-
{
84-
VerifyOrReturn(CheckOverCASESession(ctx));
85-
VerifyOrReturn(CheckDelegate(ctx, mDelegate));
86-
87-
Commands::DatasetResponse::Type response;
88-
Thread::OperationalDataset activeDataset;
89-
CHIP_ERROR err = mDelegate->GetActiveDataset(activeDataset);
90-
if (err == CHIP_ERROR_NOT_FOUND)
91-
{
92-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::NotFound);
93-
}
94-
else if (err != CHIP_NO_ERROR)
95-
{
96-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::Failure);
97-
}
98-
ReturnOnFailure(err);
99-
response.dataset = activeDataset.AsByteSpan();
100-
ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response);
101-
}
102-
103-
void ServerInstance::HandleGetPendingDatasetRequest(HandlerContext & ctx,
104-
const Commands::GetPendingDatasetRequest::DecodableType & req)
82+
void ServerInstance::HandleGetDatasetRequest(HandlerContext & ctx, Delegate::DatasetType type)
10583
{
10684
VerifyOrReturn(CheckOverCASESession(ctx));
10785
VerifyOrReturn(CheckDelegate(ctx, mDelegate));
10886

10987
Commands::DatasetResponse::Type response;
110-
Thread::OperationalDataset pendingDataset;
111-
CHIP_ERROR err = mDelegate->GetPendingDataset(pendingDataset);
112-
if (err == CHIP_ERROR_NOT_FOUND)
88+
Thread::OperationalDataset dataset;
89+
CHIP_ERROR err = mDelegate->GetDataset(dataset, type);
90+
if (err != CHIP_NO_ERROR)
11391
{
114-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::NotFound);
115-
}
116-
else if (err != CHIP_NO_ERROR)
117-
{
118-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::Failure);
92+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, StatusIB(err).mStatus);
93+
return;
11994
}
120-
ReturnOnFailure(err);
121-
response.dataset = pendingDataset.AsByteSpan();
95+
response.dataset = dataset.AsByteSpan();
12296
ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response);
12397
}
12498

12599
void ServerInstance::HandleSetActiveDatasetRequest(HandlerContext & ctx,
126100
const Commands::SetActiveDatasetRequest::DecodableType & req)
127101
{
102+
// The SetActiveDatasetRequest command SHALL be FailSafeArmed. Upon receiving this command, the Thread BR will set its
103+
// active dataset. If the dataset is set successfully, OnActivateDatasetComplete will be called with CHIP_NO_ERROR, prompting
104+
// the Thread BR to respond with a success status and disarm the FailSafe timer. If an error occurs while setting the active
105+
// dataset, the Thread BR should respond with a failure status. In this case, when the FailSafe timer expires, the active
106+
// dataset set by this command will be reverted. If the FailSafe timer expires before the Thread BR responds, the Thread BR will
107+
// respond with a timeout status and the active dataset should also be reverted.
128108
VerifyOrReturn(CheckFailSafeArmed(ctx));
129109
VerifyOrReturn(CheckDelegate(ctx, mDelegate));
130110
if (mAsyncCommandHandle.Get() != nullptr)
131111
{
132112
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::Busy);
133-
ctx.SetCommandHandled();
134113
return;
135114
}
136115

@@ -145,7 +124,7 @@ void ServerInstance::HandleSetActiveDatasetRequest(HandlerContext & ctx,
145124
return;
146125
}
147126

148-
if (mDelegate->GetActiveDataset(currentActiveDataset) == CHIP_NO_ERROR &&
127+
if (mDelegate->GetDataset(currentActiveDataset, Delegate::DatasetType::kActive) == CHIP_NO_ERROR &&
149128
currentActiveDataset.GetActiveTimestamp(currentActiveDatasetTimestamp) == CHIP_NO_ERROR)
150129
{
151130
// If this command is invoked when the ActiveDatasetTimestamp attribute is not null, the command SHALL
@@ -172,7 +151,7 @@ void ServerInstance::HandleSetPendingDatasetRequest(HandlerContext & ctx,
172151
bool panChangeSupported;
173152
if (mDelegate->GetPanChangeSupported(panChangeSupported) != CHIP_NO_ERROR || !panChangeSupported)
174153
{
175-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::UnsupportedCommand);
154+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::UnsupportedCommand);
176155
return;
177156
}
178157
Thread::OperationalDataset pendingDataset;
@@ -183,14 +162,8 @@ void ServerInstance::HandleSetPendingDatasetRequest(HandlerContext & ctx,
183162
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::InvalidCommand);
184163
return;
185164
}
186-
if (mDelegate->SetPendingDataset(pendingDataset) != CHIP_NO_ERROR)
187-
{
188-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::Failure);
189-
}
190-
else
191-
{
192-
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::Success);
193-
}
165+
CHIP_ERROR err = mDelegate->SetPendingDataset(pendingDataset);
166+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, StatusIB(err).mStatus);
194167
}
195168

196169
void ServerInstance::InvokeCommand(HandlerContext & ctx)
@@ -235,15 +208,15 @@ CHIP_ERROR ServerInstance::ReadBorderRouterName(AttributeValueEncoder & aEncoder
235208
char borderRouterName[kBorderRouterNameMaxLength];
236209
MutableCharSpan borderRouterNameSpan(borderRouterName);
237210
ReturnErrorOnFailure(mDelegate->GetBorderRouterName(borderRouterNameSpan));
238-
return aEncoder.Encode(chip::CharSpan(borderRouterName, borderRouterNameSpan.size()));
211+
return aEncoder.Encode(borderRouterNameSpan);
239212
}
240213

241214
CHIP_ERROR ServerInstance::ReadBorderAgentID(AttributeValueEncoder & aEncoder)
242215
{
243216
uint8_t borderAgentId[kBorderAgentIdLength];
244217
MutableByteSpan borderAgentIdSpan(borderAgentId);
245218
ReturnErrorOnFailure(mDelegate->GetBorderAgentId(borderAgentIdSpan));
246-
return aEncoder.Encode(chip::ByteSpan(borderAgentId));
219+
return aEncoder.Encode(borderAgentIdSpan);
247220
}
248221

249222
CHIP_ERROR ServerInstance::ReadThreadVersion(AttributeValueEncoder & aEncoder)
@@ -264,12 +237,12 @@ CHIP_ERROR ServerInstance::ReadActiveDatasetTimestamp(AttributeValueEncoder & aE
264237
{
265238
Thread::OperationalDataset activeDataset;
266239
uint64_t activeDatasetTimestamp;
267-
if (mDelegate->GetActiveDataset(activeDataset) == CHIP_NO_ERROR &&
240+
if (mDelegate->GetDataset(activeDataset, Delegate::DatasetType::kActive) == CHIP_NO_ERROR &&
268241
activeDataset.GetActiveTimestamp(activeDatasetTimestamp) == CHIP_NO_ERROR)
269242
{
270243
return aEncoder.Encode(DataModel::MakeNullable(activeDatasetTimestamp));
271244
}
272-
return aEncoder.Encode(DataModel::Nullable<uint64_t>());
245+
return aEncoder.EncodeNull();
273246
}
274247

275248
CHIP_ERROR ServerInstance::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
@@ -330,29 +303,29 @@ void ServerInstance::OnActivateDatasetComplete(CHIP_ERROR error)
330303
if (error == CHIP_NO_ERROR)
331304
{
332305
// The successful completion of the activation process SHALL disarm the fail-safe timer.
333-
auto & failSafeContext = chip::Server::GetInstance().GetFailSafeContext();
306+
auto & failSafeContext = Server::GetInstance().GetFailSafeContext();
334307
failSafeContext.DisarmFailSafe();
335308
CommitSavedBreadcrumb();
336309
}
337310
else
338311
{
339312
ChipLogError(Zcl, "Failed on activating the active dataset for Thread BR: %" CHIP_ERROR_FORMAT, error.Format());
340313
}
341-
commandHandle->AddStatus(mPath, error == CHIP_NO_ERROR ? Status::Success : Status::Failure);
314+
commandHandle->AddStatus(mPath, StatusIB(error).mStatus);
342315
}
343316

344317
void ServerInstance::OnFailSafeTimerExpired()
345318
{
319+
if (mDelegate)
320+
{
321+
mDelegate->RevertActiveDataset();
322+
}
346323
auto commandHandleRef = std::move(mAsyncCommandHandle);
347324
auto commandHandle = commandHandleRef.Get();
348325
if (commandHandle == nullptr)
349326
{
350327
return;
351328
}
352-
if (mDelegate)
353-
{
354-
mDelegate->RevertActiveDataset();
355-
}
356329
commandHandle->AddStatus(mPath, Status::Timeout);
357330
}
358331

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

+9-2
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,17 @@ class ServerInstance : public CommandHandlerInterface, public AttributeAccessInt
5656

5757
private:
5858
// Command Handlers
59-
void HandleGetActiveDatasetRequest(HandlerContext & ctx, const Commands::GetActiveDatasetRequest::DecodableType & req);
60-
void HandleGetPendingDatasetRequest(HandlerContext & ctx, const Commands::GetPendingDatasetRequest::DecodableType & req);
59+
void HandleGetActiveDatasetRequest(HandlerContext & ctx, const Commands::GetActiveDatasetRequest::DecodableType & req)
60+
{
61+
HandleGetDatasetRequest(ctx, Delegate::DatasetType::kActive);
62+
}
63+
void HandleGetPendingDatasetRequest(HandlerContext & ctx, const Commands::GetPendingDatasetRequest::DecodableType & req)
64+
{
65+
HandleGetDatasetRequest(ctx, Delegate::DatasetType::kPending);
66+
}
6167
void HandleSetActiveDatasetRequest(HandlerContext & ctx, const Commands::SetActiveDatasetRequest::DecodableType & req);
6268
void HandleSetPendingDatasetRequest(HandlerContext & ctx, const Commands::SetPendingDatasetRequest::DecodableType & req);
69+
void HandleGetDatasetRequest(HandlerContext & ctx, Delegate::DatasetType type);
6370

6471
// Attribute Read handler
6572
CHIP_ERROR ReadFeatureMap(AttributeValueEncoder & aEncoder);

‎src/platform/ESP32/ESP32ThreadBorderRouterDelegate.cpp

+11-18
Original file line numberDiff line numberDiff line change
@@ -92,34 +92,27 @@ CHIP_ERROR ESP32ThreadBorderRouterDelegate::GetInterfaceEnabled(bool & interface
9292
return CHIP_NO_ERROR;
9393
}
9494

95-
CHIP_ERROR ESP32ThreadBorderRouterDelegate::GetActiveDataset(chip::Thread::OperationalDataset & activeDataset)
95+
CHIP_ERROR ESP32ThreadBorderRouterDelegate::GetDataset(Thread::OperationalDataset & dataset, DatasetType type)
9696
{
9797
ScopedThreadLock threadLock;
98+
otError otErr = OT_ERROR_NONE;
9899
otOperationalDatasetTlvs datasetTlvs;
99-
if (GetThreadEnabled())
100+
if (type == DatasetType::kActive)
100101
{
101-
otError otErr = otDatasetGetActiveTlvs(esp_openthread_get_instance(), &datasetTlvs);
102-
if (otErr == OT_ERROR_NONE)
103-
{
104-
return activeDataset.Init(ByteSpan(datasetTlvs.mTlvs, datasetTlvs.mLength));
105-
}
102+
otErr = otDatasetGetActiveTlvs(esp_openthread_get_instance(), &datasetTlvs);
103+
}
104+
else
105+
{
106+
otErr = otDatasetGetPendingTlvs(esp_openthread_get_instance(), &datasetTlvs);
106107
}
107-
return CHIP_ERROR_NOT_FOUND;
108-
}
109-
110-
CHIP_ERROR ESP32ThreadBorderRouterDelegate::GetPendingDataset(chip::Thread::OperationalDataset & pendingDataset)
111-
{
112-
ScopedThreadLock threadLock;
113-
otOperationalDatasetTlvs datasetTlvs;
114-
otError otErr = otDatasetGetPendingTlvs(esp_openthread_get_instance(), &datasetTlvs);
115108
if (otErr == OT_ERROR_NONE)
116109
{
117-
return pendingDataset.Init(ByteSpan(datasetTlvs.mTlvs, datasetTlvs.mLength));
110+
return dataset.Init(ByteSpan(datasetTlvs.mTlvs, datasetTlvs.mLength));
118111
}
119112
return CHIP_ERROR_NOT_FOUND;
120113
}
121114

122-
CHIP_ERROR ESP32ThreadBorderRouterDelegate::SetActiveDataset(const chip::Thread::OperationalDataset & activeDataset,
115+
CHIP_ERROR ESP32ThreadBorderRouterDelegate::SetActiveDataset(const Thread::OperationalDataset & activeDataset,
123116
ActivateDatasetCallback * callback)
124117
{
125118
VerifyOrReturnError(callback, CHIP_ERROR_INVALID_ARGUMENT);
@@ -202,7 +195,7 @@ CHIP_ERROR ESP32ThreadBorderRouterDelegate::RevertActiveDataset()
202195
return CHIP_NO_ERROR;
203196
}
204197

205-
CHIP_ERROR ESP32ThreadBorderRouterDelegate::SetPendingDataset(const chip::Thread::OperationalDataset & pendingDataset)
198+
CHIP_ERROR ESP32ThreadBorderRouterDelegate::SetPendingDataset(const Thread::OperationalDataset & pendingDataset)
206199
{
207200
ScopedThreadLock threadLock;
208201
otOperationalDatasetTlvs datasetTlvs;

‎src/platform/ESP32/ESP32ThreadBorderRouterDelegate.h

+3-10
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,6 @@
2525
#include <lib/support/Span.h>
2626
#include <openthread/netdiag.h>
2727

28-
using chip::MutableByteSpan;
29-
using chip::MutableCharSpan;
30-
using chip::Span;
31-
3228
namespace chip {
3329
namespace app {
3430
namespace Clusters {
@@ -70,16 +66,13 @@ class ESP32ThreadBorderRouterDelegate : public Delegate
7066

7167
CHIP_ERROR GetInterfaceEnabled(bool & interfaceEnabled) override;
7268

73-
CHIP_ERROR GetActiveDataset(chip::Thread::OperationalDataset & activeDataset) override;
74-
75-
CHIP_ERROR GetPendingDataset(chip::Thread::OperationalDataset & pendingDataset) override;
69+
CHIP_ERROR GetDataset(Thread::OperationalDataset & dataset, DatasetType type) override;
7670

77-
CHIP_ERROR SetActiveDataset(const chip::Thread::OperationalDataset & activeDataset,
78-
ActivateDatasetCallback * callback) override;
71+
CHIP_ERROR SetActiveDataset(const Thread::OperationalDataset & activeDataset, ActivateDatasetCallback * callback) override;
7972

8073
CHIP_ERROR RevertActiveDataset() override;
8174

82-
CHIP_ERROR SetPendingDataset(const chip::Thread::OperationalDataset & pendingDataset) override;
75+
CHIP_ERROR SetPendingDataset(const Thread::OperationalDataset & pendingDataset) override;
8376

8477
static void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg);
8578

0 commit comments

Comments
 (0)