Skip to content

Commit 4566110

Browse files
bzbarsky-applepull[bot]
bzbarsky-apple
authored and
pull[bot]
committed
Use cluster-objects response struct in DoorLockServer::getUserCommandHandler. (#25092)
Get rids of some manual TLV use and use of emberAfCurrentEndpoint().
1 parent 9e03528 commit 4566110

File tree

12 files changed

+190
-116
lines changed

12 files changed

+190
-116
lines changed

‎src/app/clusters/door-lock-server/door-lock-server.cpp

+38-62
Original file line numberDiff line numberDiff line change
@@ -436,82 +436,58 @@ void DoorLockServer::getUserCommandHandler(chip::app::CommandHandler * commandOb
436436
return;
437437
}
438438

439-
CHIP_ERROR err = CHIP_NO_ERROR;
440439
EmberAfPluginDoorLockUserInfo user;
441-
VerifyOrExit(emberAfPluginDoorLockGetUser(commandPath.mEndpointId, userIndex, user), err = CHIP_ERROR_INTERNAL);
440+
if (!emberAfPluginDoorLockGetUser(commandPath.mEndpointId, userIndex, user))
442441
{
443-
chip::app::ConcreteCommandPath path = { emberAfCurrentEndpoint(), ::Id, Commands::GetUserResponse::Id };
444-
chip::TLV::TLVWriter * writer;
445-
SuccessOrExit(err = commandObj->PrepareCommand(path));
446-
VerifyOrExit((writer = commandObj->GetCommandDataIBTLVWriter()) != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
447-
SuccessOrExit(
448-
err = writer->Put(chip::TLV::ContextTag(to_underlying(Commands::GetUserResponse::Fields::kUserIndex)), userIndex));
442+
emberAfDoorLockClusterPrintln("[GetUser] Could not get user info [userIndex=%d]", userIndex);
443+
commandObj->AddStatus(commandPath, Status::Failure);
444+
return;
445+
}
449446

450-
using ResponseFields = Commands::GetUserResponse::Fields;
447+
Commands::GetUserResponse::Type response;
448+
response.userIndex = userIndex;
451449

452-
// appclusters, 5.2.4.36: we should not add user-specific field if the user status is set to Available
453-
if (UserStatusEnum::kAvailable != user.userStatus)
454-
{
455-
emberAfDoorLockClusterPrintln("Found user in storage: "
456-
"[userIndex=%d,userName=\"%.*s\",userStatus=%u,userType=%u"
457-
",credentialRule=%u,createdBy=%u,modifiedBy=%u]",
458-
userIndex, static_cast<int>(user.userName.size()), user.userName.data(),
459-
to_underlying(user.userStatus), to_underlying(user.userType),
460-
to_underlying(user.credentialRule), user.createdBy, user.lastModifiedBy);
450+
// appclusters, 5.2.4.36: we should not set user-specific fields to non-null if the user status is set to Available
451+
if (UserStatusEnum::kAvailable != user.userStatus)
452+
{
453+
emberAfDoorLockClusterPrintln("Found user in storage: "
454+
"[userIndex=%d,userName=\"%.*s\",userStatus=%u,userType=%u"
455+
",credentialRule=%u,createdBy=%u,modifiedBy=%u]",
456+
userIndex, static_cast<int>(user.userName.size()), user.userName.data(),
457+
to_underlying(user.userStatus), to_underlying(user.userType),
458+
to_underlying(user.credentialRule), user.createdBy, user.lastModifiedBy);
461459

462-
SuccessOrExit(err = writer->PutString(TLV::ContextTag(to_underlying(ResponseFields::kUserName)), user.userName));
463-
if (0xFFFFFFFFU != user.userUniqueId)
464-
{
465-
SuccessOrExit(err = writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kUserUniqueID)), user.userUniqueId));
466-
}
467-
SuccessOrExit(err = writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kUserStatus)), user.userStatus));
468-
SuccessOrExit(err = writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kUserType)), user.userType));
469-
SuccessOrExit(err = writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kCredentialRule)), user.credentialRule));
470-
if (!user.credentials.empty())
471-
{
472-
TLV::TLVType credentialsContainer;
473-
SuccessOrExit(err = writer->StartContainer(TLV::ContextTag(to_underlying(ResponseFields::kCredentials)),
474-
TLV::kTLVType_Array, credentialsContainer));
475-
for (size_t i = 0; i < user.credentials.size(); ++i)
476-
{
477-
SuccessOrExit(err = user.credentials.data()[i].Encode(*writer, TLV::AnonymousTag()));
478-
}
479-
SuccessOrExit(err = writer->EndContainer(credentialsContainer));
480-
}
481-
// Append fabric IDs only if the user was created/modified by matter
482-
if (user.creationSource == DlAssetSource::kMatterIM)
483-
{
484-
SuccessOrExit(err =
485-
writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kCreatorFabricIndex)), user.createdBy));
486-
}
487-
if (user.modificationSource == DlAssetSource::kMatterIM)
488-
{
489-
SuccessOrExit(err = writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kLastModifiedFabricIndex)),
490-
user.lastModifiedBy));
491-
}
460+
response.userName.SetNonNull(user.userName);
461+
if (0xFFFFFFFFU != user.userUniqueId)
462+
{
463+
response.userUniqueID.SetNonNull(user.userUniqueId);
492464
}
493-
else
465+
response.userStatus.SetNonNull(user.userStatus);
466+
response.userType.SetNonNull(user.userType);
467+
response.credentialRule.SetNonNull(user.credentialRule);
468+
response.credentials.SetNonNull(user.credentials);
469+
// Set fabric indices only if the user was created/modified by matter.
470+
if (user.creationSource == DlAssetSource::kMatterIM)
494471
{
495-
emberAfDoorLockClusterPrintln("[GetUser] User not found [userIndex=%d]", userIndex);
472+
response.creatorFabricIndex.SetNonNull(user.createdBy);
496473
}
497-
498-
// appclusters, 5.2.4.36.1: We need to add next occupied user after userIndex if any.
499-
uint16_t nextAvailableUserIndex = 0;
500-
if (findOccupiedUserSlot(commandPath.mEndpointId, static_cast<uint16_t>(userIndex + 1), nextAvailableUserIndex))
474+
if (user.modificationSource == DlAssetSource::kMatterIM)
501475
{
502-
SuccessOrExit(err =
503-
writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kNextUserIndex)), nextAvailableUserIndex));
476+
response.lastModifiedFabricIndex.SetNonNull(user.lastModifiedBy);
504477
}
505-
SuccessOrExit(err = commandObj->FinishCommand());
478+
}
479+
else
480+
{
481+
emberAfDoorLockClusterPrintln("[GetUser] User not found [userIndex=%d]", userIndex);
506482
}
507483

508-
exit:
509-
if (CHIP_NO_ERROR != err)
484+
// appclusters, 5.2.4.36.1: We need to add next occupied user after userIndex if any.
485+
uint16_t nextAvailableUserIndex = 0;
486+
if (findOccupiedUserSlot(commandPath.mEndpointId, static_cast<uint16_t>(userIndex + 1), nextAvailableUserIndex))
510487
{
511-
ChipLogError(Zcl, "[GetUser] Command processing failed [endpointId=%d,userIndex=%d,err=\"%s\"]", commandPath.mEndpointId,
512-
userIndex, err.AsString());
513-
commandObj->AddStatus(commandPath, Status::Failure);
488+
response.nextUserIndex.SetNonNull(nextAvailableUserIndex);
514489
}
490+
commandObj->AddResponse(commandPath, response);
515491
}
516492

517493
void DoorLockServer::clearUserCommandHandler(chip::app::CommandHandler * commandObj,

‎src/app/data-model/List.h

+8
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,14 @@ struct List : public Span<T>
5050
//
5151
using Span<T>::Span;
5252

53+
// Inherited copy constructors are _not_ imported by the using statement
54+
// above, though, so we need to implement that ourselves. This is templated
55+
// on the span's type to allow us to init a List<const Foo> from Span<Foo>.
56+
// Span's constructor handles the checks on the types for us.
57+
template <class U>
58+
constexpr List(const Span<U> & other) : Span<T>(other)
59+
{}
60+
5361
template <size_t N>
5462
constexpr List & operator=(T (&databuf)[N])
5563
{

‎src/app/tests/suites/DL_UsersAndCredentials.yaml

+11-11
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ tests:
124124
- name: "CredentialRule"
125125
value: 0
126126
- name: "Credentials"
127-
value: null
127+
value: []
128128
- name: "CreatorFabricIndex"
129129
value: 1
130130
- name: "LastModifiedFabricIndex"
@@ -195,7 +195,7 @@ tests:
195195
- name: "CredentialRule"
196196
value: 0
197197
- name: "Credentials"
198-
value: null
198+
value: []
199199
- name: "CreatorFabricIndex"
200200
value: 1
201201
- name: "LastModifiedFabricIndex"
@@ -244,7 +244,7 @@ tests:
244244
- name: "CredentialRule"
245245
value: 0
246246
- name: "Credentials"
247-
value: null
247+
value: []
248248
- name: "CreatorFabricIndex"
249249
value: 1
250250
- name: "LastModifiedFabricIndex"
@@ -293,7 +293,7 @@ tests:
293293
- name: "CredentialRule"
294294
value: 0
295295
- name: "Credentials"
296-
value: null
296+
value: []
297297
- name: "CreatorFabricIndex"
298298
value: 1
299299
- name: "LastModifiedFabricIndex"
@@ -342,7 +342,7 @@ tests:
342342
- name: "CredentialRule"
343343
value: 0
344344
- name: "Credentials"
345-
value: null
345+
value: []
346346
- name: "CreatorFabricIndex"
347347
value: 1
348348
- name: "LastModifiedFabricIndex"
@@ -391,7 +391,7 @@ tests:
391391
- name: "CredentialRule"
392392
value: 2
393393
- name: "Credentials"
394-
value: null
394+
value: []
395395
- name: "CreatorFabricIndex"
396396
value: 1
397397
- name: "LastModifiedFabricIndex"
@@ -440,7 +440,7 @@ tests:
440440
- name: "CredentialRule"
441441
value: 1
442442
- name: "Credentials"
443-
value: null
443+
value: []
444444
- name: "CreatorFabricIndex"
445445
value: 1
446446
- name: "LastModifiedFabricIndex"
@@ -489,7 +489,7 @@ tests:
489489
- name: "CredentialRule"
490490
value: 2
491491
- name: "Credentials"
492-
value: null
492+
value: []
493493
- name: "CreatorFabricIndex"
494494
value: 1
495495
- name: "LastModifiedFabricIndex"
@@ -640,7 +640,7 @@ tests:
640640
- name: "CredentialRule"
641641
value: 0
642642
- name: "Credentials"
643-
value: null
643+
value: []
644644
- name: "CreatorFabricIndex"
645645
value: 1
646646
- name: "LastModifiedFabricIndex"
@@ -689,7 +689,7 @@ tests:
689689
- name: "CredentialRule"
690690
value: 0
691691
- name: "Credentials"
692-
value: null
692+
value: []
693693
- name: "CreatorFabricIndex"
694694
value: 1
695695
- name: "LastModifiedFabricIndex"
@@ -820,7 +820,7 @@ tests:
820820
- name: "CredentialRule"
821821
value: 0
822822
- name: "Credentials"
823-
value: null
823+
value: []
824824
- name: "CreatorFabricIndex"
825825
value: 1
826826
- name: "LastModifiedFabricIndex"

‎src/app/tests/suites/certification/Test_TC_DRLK_2_11.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ tests:
7777
- name: "CredentialRule"
7878
value: 0
7979
- name: "Credentials"
80-
value: null
80+
value: []
8181
- name: "CreatorFabricIndex"
8282
value: 1
8383
- name: "LastModifiedFabricIndex"

‎src/app/tests/suites/certification/Test_TC_DRLK_2_2.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ tests:
7272
- name: "CredentialRule"
7373
value: 0
7474
- name: "Credentials"
75-
value: null
75+
value: []
7676
- name: "CreatorFabricIndex"
7777
value: 1
7878
- name: "LastModifiedFabricIndex"

‎src/app/tests/suites/certification/Test_TC_DRLK_2_3.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ tests:
7373
- name: "CredentialRule"
7474
value: 0
7575
- name: "Credentials"
76-
value: null
76+
value: []
7777
- name: "CreatorFabricIndex"
7878
value: 1
7979
- name: "LastModifiedFabricIndex"

‎src/app/tests/suites/certification/Test_TC_DRLK_2_4.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ tests:
7474
- name: "CredentialRule"
7575
value: 0
7676
- name: "Credentials"
77-
value: null
77+
value: []
7878
- name: "CreatorFabricIndex"
7979
value: 1
8080
- name: "LastModifiedFabricIndex"

‎src/app/tests/suites/certification/Test_TC_DRLK_2_5.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ tests:
7575
- name: "CredentialRule"
7676
value: 0
7777
- name: "Credentials"
78-
value: null
78+
value: []
7979
- name: "CreatorFabricIndex"
8080
value: 1
8181
- name: "LastModifiedFabricIndex"

‎src/app/tests/suites/certification/Test_TC_DRLK_2_7.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ tests:
7575
- name: "CredentialRule"
7676
value: 0
7777
- name: "Credentials"
78-
value: null
78+
value: []
7979
- name: "CreatorFabricIndex"
8080
value: 1
8181
- name: "LastModifiedFabricIndex"

‎src/app/tests/suites/certification/Test_TC_DRLK_2_9.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ tests:
7575
- name: "CredentialRule"
7676
value: 0
7777
- name: "Credentials"
78-
value: null
78+
value: []
7979
- name: "CreatorFabricIndex"
8080
value: 1
8181
- name: "LastModifiedFabricIndex"

0 commit comments

Comments
 (0)