Use cluster-objects response struct in DoorLockServer::getUserCommand…

…Handler. (project-chip#25092) Get rids of some manual TLV use and use of emberAfCurrentEndpoint().
lecndav · Mar 22, 2023 · 1967a67 · 1967a67
1 parent 9709ff3
commit 1967a67
Show file tree

Hide file tree

Showing 12 changed files with 190 additions and 116 deletions.
diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp
@@ -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,

diff --git a/src/app/data-model/List.h b/src/app/data-model/List.h
@@ -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])
     {

diff --git a/src/app/tests/suites/DL_UsersAndCredentials.yaml b/src/app/tests/suites/DL_UsersAndCredentials.yaml
@@ -124,7 +124,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"
@@ -195,7 +195,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"
@@ -244,7 +244,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"
@@ -293,7 +293,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"
@@ -342,7 +342,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"
@@ -391,7 +391,7 @@ tests:
               - name: "CredentialRule"
                 value: 2
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"
@@ -440,7 +440,7 @@ tests:
               - name: "CredentialRule"
                 value: 1
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"
@@ -489,7 +489,7 @@ tests:
               - name: "CredentialRule"
                 value: 2
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"
@@ -640,7 +640,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"
@@ -689,7 +689,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"
@@ -820,7 +820,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"

diff --git a/src/app/tests/suites/certification/Test_TC_DRLK_2_11.yaml b/src/app/tests/suites/certification/Test_TC_DRLK_2_11.yaml
@@ -77,7 +77,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"

diff --git a/src/app/tests/suites/certification/Test_TC_DRLK_2_2.yaml b/src/app/tests/suites/certification/Test_TC_DRLK_2_2.yaml
@@ -72,7 +72,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"

diff --git a/src/app/tests/suites/certification/Test_TC_DRLK_2_3.yaml b/src/app/tests/suites/certification/Test_TC_DRLK_2_3.yaml
@@ -73,7 +73,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"

diff --git a/src/app/tests/suites/certification/Test_TC_DRLK_2_4.yaml b/src/app/tests/suites/certification/Test_TC_DRLK_2_4.yaml
@@ -74,7 +74,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"

diff --git a/src/app/tests/suites/certification/Test_TC_DRLK_2_5.yaml b/src/app/tests/suites/certification/Test_TC_DRLK_2_5.yaml
@@ -75,7 +75,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"

diff --git a/src/app/tests/suites/certification/Test_TC_DRLK_2_7.yaml b/src/app/tests/suites/certification/Test_TC_DRLK_2_7.yaml
@@ -75,7 +75,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"

diff --git a/src/app/tests/suites/certification/Test_TC_DRLK_2_9.yaml b/src/app/tests/suites/certification/Test_TC_DRLK_2_9.yaml
@@ -75,7 +75,7 @@ tests:
               - name: "CredentialRule"
                 value: 0
               - name: "Credentials"
-                value: null
+                value: []
               - name: "CreatorFabricIndex"
                 value: 1
               - name: "LastModifiedFabricIndex"