Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use cluster-objects response struct in DoorLockServer::getUserCommandHandler. #25092

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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