Switch to using DecodableList for list-typed attribute reads (#10681)

* Convert list attributes to DecodableList for chip-tool yaml tests. * Convert list attributes to DecodableList for Darwin * Convert list attributes to DecodableList for python controller * Convert list attributes to DecodableList for the Android controller * Improve comment per review feedback * Fix tv app to produce lists with the right encoding. * Address review comment to fix Android attribute reading
project-chip · Nov 29, 2021 · cdf1869 · cdf1869
1 parent e4c8960
commit cdf1869
Show file tree

Hide file tree

Showing 48 changed files with 3,087 additions and 2,127 deletions.
diff --git a/examples/chip-tool/commands/tests/TestCommand.cpp b/examples/chip-tool/commands/tests/TestCommand.cpp
@@ -116,6 +116,19 @@ bool TestCommand::CheckValueAsList(const char * itemName, uint64_t current, uint
 bool TestCommand::CheckValueAsString(const char * itemName, const chip::ByteSpan current, const char * expected)
 {
     const chip::ByteSpan expectedArgument = chip::ByteSpan(chip::Uint8::from_const_char(expected), strlen(expected));
+
+    if (!current.data_equal(expectedArgument))
+    {
+        Exit(std::string(itemName) + " value mismatch, expecting " + std::string(expected));
+        return false;
+    }
+
+    return true;
+}
+
+bool TestCommand::CheckValueAsString(const char * itemName, const chip::CharSpan current, const char * expected)
+{
+    const chip::CharSpan expectedArgument(expected, strlen(expected));
     if (!current.data_equal(expectedArgument))
     {
         Exit(std::string(itemName) + " value mismatch, expecting " + std::string(expected));

diff --git a/examples/chip-tool/commands/tests/TestCommand.h b/examples/chip-tool/commands/tests/TestCommand.h
@@ -157,8 +157,27 @@ class TestCommand : public CHIPCommand
         return CheckValueAsListHelper<T>(itemName, iter, std::forward<ValueTypes>(items)...);
     }
 
+    template <typename T>
+    bool CheckValueAsListLength(const char * itemName, chip::app::DataModel::DecodableList<T> list, uint64_t expectedLength)
+    {
+        auto iter      = list.begin();
+        uint64_t count = 0;
+        while (iter.Next())
+        {
+            ++count;
+        }
+        if (iter.GetStatus() != CHIP_NO_ERROR)
+        {
+            Exit(std::string(itemName) + " list length mismatch: expected " + std::to_string(expectedLength) + " but got an error");
+            return false;
+        }
+        return CheckValueAsList(itemName, count, expectedLength);
+    }
+
     bool CheckValueAsString(const char * itemName, chip::ByteSpan current, const char * expected);
 
+    bool CheckValueAsString(const char * itemName, chip::CharSpan current, const char * expected);
+
     chip::Callback::Callback<chip::Controller::OnDeviceConnected> mOnDeviceConnectedCallback;
     chip::Callback::Callback<chip::Controller::OnDeviceConnectionFailure> mOnDeviceConnectionFailureCallback;
 

diff --git a/examples/chip-tool/templates/commands.zapt b/examples/chip-tool/templates/commands.zapt
@@ -5,6 +5,7 @@
 #include <cstdint>
 
 #include <app-common/zap-generated/cluster-objects.h>
+#include <app/data-model/DecodableList.h>
 #include <commands/clusters/ModelCommand.h>
 #include <lib/core/CHIPSafeCasts.h>
 #include <lib/support/BytesToHex.h>
@@ -133,35 +134,53 @@ static void OnCharStringAttributeResponse(void * context, const chip::CharSpan v
 {{#chip_client_clusters}}
 {{#chip_server_cluster_attributes}}
 {{#if isList}}
-static void On{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}ListAttributeResponse(void * context, uint16_t count, {{chipType}} * entries)
+static void On{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}ListAttributeResponse(void * context, const chip::app::DataModel::DecodableList<{{zapTypeToDecodableClusterObjectType type ns=parent.name}}> & list)
 {
-    ChipLogProgress(chipTool, "On{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}ListAttributeResponse: %" PRIu16 " entries", count);
+    ModelCommand * command = static_cast<ModelCommand *>(context);
 
-    for (uint16_t i = 0; i < count; i++)
+    size_t count = 0;
     {
+      auto iter = list.begin();
+      while (iter.Next()) {
+        ++count;
+      }
+      if (iter.GetStatus() != CHIP_NO_ERROR) {
+        command->SetCommandExitStatus(iter.GetStatus());
+        return;
+      }
+    }
+    ChipLogProgress(chipTool, "On{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}ListAttributeResponse: %zu entries", count);
+
+    auto iter = list.begin();
+    uint16_t i = 0;
+    while (iter.Next())
+    {
+#if CHIP_PROGRESS_LOGGING
+        auto & entry = iter.GetValue();
+#endif // CHIP_PROGRESS_LOGGING
+        ++i;
         {{#if isStruct}}
         ChipLogProgress(chipTool, "{{type}}[%" PRIu16 "]:", i);
         {{#chip_attribute_list_entryTypes}}
         {{#if (isOctetString type)}}
-        ChipLogProgress(Zcl, "  {{asSymbol label}}: %zu", entries[i].{{name}}.size());
+        ChipLogProgress(Zcl, "  {{asSymbol label}}: %zu", entry.{{asLowerCamelCase name}}.size());
         {{else if (isCharString type)}}
-        ChipLogProgress(Zcl, "  {{asSymbol label}}: %.*s", static_cast<int>(entries[i].{{name}}.size()), entries[i].{{name}}.data());
+        ChipLogProgress(Zcl, "  {{asSymbol label}}: %.*s", static_cast<int>(entry.{{asLowerCamelCase name}}.size()), entry.{{asLowerCamelCase name}}.data());
         {{else}}
-        ChipLogProgress(chipTool, "  {{name}}: {{asPrintFormat type}}", entries[i].{{name}});
+        ChipLogProgress(chipTool, "  {{asLowerCamelCase name}}: {{asPrintFormat type}}", entry.{{asLowerCamelCase name}});
         {{/if}}
         {{/chip_attribute_list_entryTypes}}
         {{else}}
         {{#if (isOctetString type)}}
-        ChipLogProgress(Zcl, "  {{asSymbol label}}: %zu", entries[i].size());
+        ChipLogProgress(Zcl, "  {{asSymbol label}}: %zu", entry.size());
         {{else if (isCharString type)}}
-        ChipLogProgress(Zcl, "  {{asSymbol label}}: %.*s", static_cast<int>(entries[i].size()), entries[i].data());
+        ChipLogProgress(Zcl, "  {{asSymbol label}}: %.*s", static_cast<int>(entry.size()), entry.data());
         {{else}}
-        ChipLogProgress(chipTool, "{{type}}[%" PRIu16 "]: {{asPrintFormat type}}", i, entries[i]);
+        ChipLogProgress(chipTool, "{{type}}[%" PRIu16 "]: {{asPrintFormat type}}", i, entry);
         {{/if}}
         {{/if}}
     }
 
-    ModelCommand * command = static_cast<ModelCommand *>(context);
     command->SetCommandExitStatus(CHIP_NO_ERROR);
 }
 

diff --git a/examples/chip-tool/templates/partials/test_cluster.zapt b/examples/chip-tool/templates/partials/test_cluster.zapt
@@ -53,7 +53,7 @@ class {{filename}}: public TestCommand
     {{#*inline "successCallback"}}mOnSuccessCallback_{{index}}{{/inline}}
     {{#*inline "failureResponse"}}OnFailureCallback_{{index}}{{/inline}}
     {{#*inline "successResponse"}}OnSuccessCallback_{{index}}{{/inline}}
-    {{#*inline "successArguments"}}void * context{{#chip_tests_item_response_parameters}}, {{#if isList}}uint16_t count, {{/if}}{{chipType}} {{#if isList}}* {{/if}}{{asLowerCamelCase name}}{{/chip_tests_item_response_parameters}}{{/inline}}
+    {{#*inline "successArguments"}}void * context{{#chip_tests_item_response_parameters}}, {{#if isList}}const chip::app::DataModel::DecodableList<{{/if}}{{zapTypeToDecodableClusterObjectType type ns=parent.cluster}}{{#if isList}}> &{{/if}} {{asLowerCamelCase name}}{{/chip_tests_item_response_parameters}}{{/inline}}
     {{#*inline "failureArguments"}}void * context, uint8_t status{{/inline}}
 
     {{#chip_tests_items}}
@@ -75,7 +75,7 @@ class {{filename}}: public TestCommand
 
     static void {{>successResponse}}({{> successArguments}})
     {
-        (static_cast<{{filename}} *>(context))->OnSuccessResponse_{{index}}({{#chip_tests_item_response_parameters}}{{#not_first}}, {{/not_first}}{{#if isList}}count, {{/if}}{{asLowerCamelCase name}}{{/chip_tests_item_response_parameters}});
+        (static_cast<{{filename}} *>(context))->OnSuccessResponse_{{index}}({{#chip_tests_item_response_parameters}}{{#not_first}}, {{/not_first}}{{asLowerCamelCase name}}{{/chip_tests_item_response_parameters}});
     }
 
     {{! "isWaitForReport" can be replaced by "async" if there is a mechanism to remove the report handler}}
@@ -102,7 +102,7 @@ class {{filename}}: public TestCommand
     {{#*inline "failureResponse"}}OnFailureResponse_{{index}}{{/inline}}
     {{#*inline "successResponse"}}OnSuccessResponse_{{index}}{{/inline}}
     {{#*inline "failureArguments"}}uint8_t status{{/inline}}
-    {{#*inline "successArguments"}}{{#chip_tests_item_response_parameters}}{{#not_first}}, {{/not_first}}{{#if isList}}uint16_t count, {{/if}}{{#if isArray}}chip::app::DataModel::DecodableList<{{/if}}{{chipType}}{{#if isArray}}>{{/if}} {{#if isList}}* {{/if}}{{asLowerCamelCase name}}{{/chip_tests_item_response_parameters}}{{/inline}}
+    {{#*inline "successArguments"}}{{#chip_tests_item_response_parameters}}{{#not_first}}, {{/not_first}}{{#if isList}}const chip::app::DataModel::DecodableList<{{/if}}{{#if isArray}}const chip::app::DataModel::DecodableList<{{/if}}{{zapTypeToDecodableClusterObjectType type ns=parent.cluster}}{{#if isList}}>& {{/if}}{{#if isArray}}>& {{/if}} {{asLowerCamelCase name}}{{/chip_tests_item_response_parameters}}{{/inline}}
 
     CHIP_ERROR {{>testCommand}}()
     {
@@ -120,7 +120,7 @@ class {{filename}}: public TestCommand
 
         auto success = [](void * context, const responseType & data) {
             {{! TODO Update CHAR_STRING to be of type chip::CharSpan instead of chip::ByteSpan }}
-            (static_cast<{{filename}} *>(context))->OnSuccessResponse_{{index}}({{#chip_tests_item_response_parameters}}{{#not_first}}, {{/not_first}}{{#if (isCharString type)}}chip::ByteSpan(reinterpret_cast<const unsigned char *>(data.{{asLowerCamelCase name}}.data()), data.{{asLowerCamelCase name}}.size()){{else}}data.{{asLowerCamelCase name}}{{/if}}{{/chip_tests_item_response_parameters}});
+            (static_cast<{{filename}} *>(context))->OnSuccessResponse_{{index}}({{#chip_tests_item_response_parameters}}{{#not_first}}, {{/not_first}}data.{{asLowerCamelCase name}}{{/chip_tests_item_response_parameters}});
         };
 
         auto failure = [](void * context, EmberAfStatus status) {
@@ -164,7 +164,7 @@ class {{filename}}: public TestCommand
             {{~#*inline "item"}}{{asLowerCamelCase name}}{{/inline}}
             {{#if hasExpectedValue}}
               VerifyOrReturn(CheckValue
-                {{~#if isList}}AsList("{{>item}}", count, {{expectedValue.length}})
+                {{~#if isList}}AsListLength("{{>item}}", {{>item}}, {{expectedValue.length}})
                 {{else if isArray}}AsList("{{>item}}", {{>item}}, {{expectedValue}})
                 {{else if (isString type)}}AsString("{{>item}}", {{>item}}, "{{expectedValue}}")
                 {{else}}<{{chipType}}>("{{>item}}", {{>item}}, {{expectedValue}}{{asTypeLiteralSuffix type}})

diff --git a/examples/tv-app/linux/include/application-launcher/ApplicationLauncherManager.cpp b/examples/tv-app/linux/include/application-launcher/ApplicationLauncherManager.cpp
@@ -17,6 +17,8 @@
  */
 
 #include "ApplicationLauncherManager.h"
+#include <app-common/zap-generated/af-structs.h>
+#include <app-common/zap-generated/cluster-objects.h>
 #include <app/clusters/application-launcher-server/application-launcher-server.h>
 #include <app/util/af.h>
 #include <app/util/basic-types.h>
@@ -32,12 +34,13 @@ CHIP_ERROR ApplicationLauncherManager::Init()
     return err;
 }
 
-vector<uint16_t> ApplicationLauncherManager::proxyGetApplicationList()
+CHIP_ERROR ApplicationLauncherManager::proxyGetApplicationList(chip::app::AttributeValueEncoder & aEncoder)
 {
-    vector<uint16_t> applications;
-    applications.push_back(123);
-    applications.push_back(456);
-    return applications;
+    return aEncoder.EncodeList([](const chip::app::TagBoundEncoder & encoder) -> CHIP_ERROR {
+        ReturnErrorOnFailure(encoder.Encode(123u));
+        ReturnErrorOnFailure(encoder.Encode(456u));
+        return CHIP_NO_ERROR;
+    });
 }
 
 ApplicationLauncherResponse applicationLauncherClusterLaunchApp(ApplicationLauncherApp application, std::string data)

diff --git a/examples/tv-app/linux/include/application-launcher/ApplicationLauncherManager.h b/examples/tv-app/linux/include/application-launcher/ApplicationLauncherManager.h
@@ -18,7 +18,7 @@
 
 #pragma once
 
-#include <app-common/zap-generated/af-structs.h>
+#include <app/AttributeAccessInterface.h>
 #include <lib/core/CHIPError.h>
 
 #include <string>
@@ -28,5 +28,5 @@ class ApplicationLauncherManager
 {
 public:
     CHIP_ERROR Init();
-    std::vector<uint16_t> proxyGetApplicationList();
+    CHIP_ERROR proxyGetApplicationList(chip::app::AttributeValueEncoder & aEncoder);
 };
diff --git a/examples/tv-app/linux/include/audio-output/AudioOutputManager.cpp b/examples/tv-app/linux/include/audio-output/AudioOutputManager.cpp
@@ -18,6 +18,7 @@
 
 #include "AudioOutputManager.h"
 
+#include <app-common/zap-generated/cluster-objects.h>
 #include <app/util/af.h>
 #include <app/util/basic-types.h>
 #include <lib/core/CHIPSafeCasts.h>
@@ -39,23 +40,23 @@ CHIP_ERROR AudioOutputManager::Init()
     return err;
 }
 
-vector<AudioOutputInfo> AudioOutputManager::proxyGetListOfAudioOutputInfo()
+CHIP_ERROR AudioOutputManager::proxyGetListOfAudioOutputInfo(chip::app::AttributeValueEncoder & aEncoder)
 {
-    // TODO: Insert code here
-    vector<AudioOutputInfo> audioOutputInfos;
-    int maximumVectorSize = 3;
-    char name[]           = "exampleName";
-
-    for (int i = 0; i < maximumVectorSize; ++i)
-    {
-        AudioOutputInfo audioOutputInfo;
-        audioOutputInfo.outputType = EMBER_ZCL_AUDIO_OUTPUT_TYPE_HDMI;
-        audioOutputInfo.name       = chip::ByteSpan(chip::Uint8::from_char(name), sizeof(name));
-        audioOutputInfo.index      = static_cast<uint8_t>(1 + i);
-        audioOutputInfos.push_back(audioOutputInfo);
-    }
+    return aEncoder.EncodeList([](const chip::app::TagBoundEncoder & encoder) -> CHIP_ERROR {
+        // TODO: Insert code here
+        int maximumVectorSize = 3;
+        char name[]           = "exampleName";
 
-    return audioOutputInfos;
+        for (int i = 0; i < maximumVectorSize; ++i)
+        {
+            chip::app::Clusters::AudioOutput::Structs::AudioOutputInfo::Type audioOutputInfo;
+            audioOutputInfo.outputType = EMBER_ZCL_AUDIO_OUTPUT_TYPE_HDMI;
+            audioOutputInfo.name       = chip::ByteSpan(chip::Uint8::from_char(name), sizeof(name) - 1);
+            audioOutputInfo.index      = static_cast<uint8_t>(1 + i);
+            ReturnErrorOnFailure(encoder.Encode(audioOutputInfo));
+        }
+        return CHIP_NO_ERROR;
+    });
 }
 
 bool audioOutputClusterSelectOutput(uint8_t index)

diff --git a/examples/tv-app/linux/include/audio-output/AudioOutputManager.h b/examples/tv-app/linux/include/audio-output/AudioOutputManager.h
@@ -18,13 +18,13 @@
 
 #pragma once
 
-#include <app-common/zap-generated/af-structs.h>
+#include <app/AttributeAccessInterface.h>
 
 #include <lib/core/CHIPError.h>
 #include <vector>
 class AudioOutputManager
 {
 public:
     CHIP_ERROR Init();
-    std::vector<AudioOutputInfo> proxyGetListOfAudioOutputInfo();
+    CHIP_ERROR proxyGetListOfAudioOutputInfo(chip::app::AttributeValueEncoder & aEncoder);
 };