Skip to content

Commit

Permalink
Use cluster-objects response struct in DoorLockServer::getUserCommand…
Browse files Browse the repository at this point in the history
…Handler. (#25092)

Get rids of some manual TLV use and use of emberAfCurrentEndpoint().
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Nov 23, 2023
1 parent 94e5172 commit 4573966
Show file tree
Hide file tree
Showing 12 changed files with 190 additions and 116 deletions.
100 changes: 38 additions & 62 deletions src/app/clusters/door-lock-server/door-lock-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -436,82 +436,58 @@ void DoorLockServer::getUserCommandHandler(chip::app::CommandHandler * commandOb
return;
}

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

using ResponseFields = Commands::GetUserResponse::Fields;
Commands::GetUserResponse::Type response;
response.userIndex = userIndex;

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

SuccessOrExit(err = writer->PutString(TLV::ContextTag(to_underlying(ResponseFields::kUserName)), user.userName));
if (0xFFFFFFFFU != user.userUniqueId)
{
SuccessOrExit(err = writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kUserUniqueID)), user.userUniqueId));
}
SuccessOrExit(err = writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kUserStatus)), user.userStatus));
SuccessOrExit(err = writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kUserType)), user.userType));
SuccessOrExit(err = writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kCredentialRule)), user.credentialRule));
if (!user.credentials.empty())
{
TLV::TLVType credentialsContainer;
SuccessOrExit(err = writer->StartContainer(TLV::ContextTag(to_underlying(ResponseFields::kCredentials)),
TLV::kTLVType_Array, credentialsContainer));
for (size_t i = 0; i < user.credentials.size(); ++i)
{
SuccessOrExit(err = user.credentials.data()[i].Encode(*writer, TLV::AnonymousTag()));
}
SuccessOrExit(err = writer->EndContainer(credentialsContainer));
}
// Append fabric IDs only if the user was created/modified by matter
if (user.creationSource == DlAssetSource::kMatterIM)
{
SuccessOrExit(err =
writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kCreatorFabricIndex)), user.createdBy));
}
if (user.modificationSource == DlAssetSource::kMatterIM)
{
SuccessOrExit(err = writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kLastModifiedFabricIndex)),
user.lastModifiedBy));
}
response.userName.SetNonNull(user.userName);
if (0xFFFFFFFFU != user.userUniqueId)
{
response.userUniqueID.SetNonNull(user.userUniqueId);
}
else
response.userStatus.SetNonNull(user.userStatus);
response.userType.SetNonNull(user.userType);
response.credentialRule.SetNonNull(user.credentialRule);
response.credentials.SetNonNull(user.credentials);
// Set fabric indices only if the user was created/modified by matter.
if (user.creationSource == DlAssetSource::kMatterIM)
{
emberAfDoorLockClusterPrintln("[GetUser] User not found [userIndex=%d]", userIndex);
response.creatorFabricIndex.SetNonNull(user.createdBy);
}

// appclusters, 5.2.4.36.1: We need to add next occupied user after userIndex if any.
uint16_t nextAvailableUserIndex = 0;
if (findOccupiedUserSlot(commandPath.mEndpointId, static_cast<uint16_t>(userIndex + 1), nextAvailableUserIndex))
if (user.modificationSource == DlAssetSource::kMatterIM)
{
SuccessOrExit(err =
writer->Put(TLV::ContextTag(to_underlying(ResponseFields::kNextUserIndex)), nextAvailableUserIndex));
response.lastModifiedFabricIndex.SetNonNull(user.lastModifiedBy);
}
SuccessOrExit(err = commandObj->FinishCommand());
}
else
{
emberAfDoorLockClusterPrintln("[GetUser] User not found [userIndex=%d]", userIndex);
}

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

void DoorLockServer::clearUserCommandHandler(chip::app::CommandHandler * commandObj,
Expand Down
8 changes: 8 additions & 0 deletions src/app/data-model/List.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ struct List : public Span<T>
//
using Span<T>::Span;

// Inherited copy constructors are _not_ imported by the using statement
// above, though, so we need to implement that ourselves. This is templated
// on the span's type to allow us to init a List<const Foo> from Span<Foo>.
// Span's constructor handles the checks on the types for us.
template <class U>
constexpr List(const Span<U> & other) : Span<T>(other)
{}

template <size_t N>
constexpr List & operator=(T (&databuf)[N])
{
Expand Down
22 changes: 11 additions & 11 deletions src/app/tests/suites/DL_UsersAndCredentials.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down Expand Up @@ -195,7 +195,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down Expand Up @@ -244,7 +244,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down Expand Up @@ -293,7 +293,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down Expand Up @@ -342,7 +342,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down Expand Up @@ -391,7 +391,7 @@ tests:
- name: "CredentialRule"
value: 2
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down Expand Up @@ -440,7 +440,7 @@ tests:
- name: "CredentialRule"
value: 1
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down Expand Up @@ -489,7 +489,7 @@ tests:
- name: "CredentialRule"
value: 2
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down Expand Up @@ -640,7 +640,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down Expand Up @@ -689,7 +689,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down Expand Up @@ -820,7 +820,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/suites/certification/Test_TC_DRLK_2_11.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/suites/certification/Test_TC_DRLK_2_2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/suites/certification/Test_TC_DRLK_2_3.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/suites/certification/Test_TC_DRLK_2_4.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/suites/certification/Test_TC_DRLK_2_5.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/suites/certification/Test_TC_DRLK_2_7.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/suites/certification/Test_TC_DRLK_2_9.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ tests:
- name: "CredentialRule"
value: 0
- name: "Credentials"
value: null
value: []
- name: "CreatorFabricIndex"
value: 1
- name: "LastModifiedFabricIndex"
Expand Down
Loading

0 comments on commit 4573966

Please sign in to comment.