Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix chip-tool handling of same-named structs in different clusters. (#…
Browse files Browse the repository at this point in the history
…26253)

The chip-tool code was assuming that the only way a struct with the same name
could occur in multiple clusters was if it was the same struct, shared by the
two clusters.  But that's just not true in the spec.  For example, there are
totally different TargetStruct structs in different clusters.

This fixes chip-tool to do the same thing as cluster-objects in terms of
deciding what the set of structs we have to deal with is.
bzbarsky-apple authored and pull[bot] committed Oct 17, 2023

Verified

This commit was signed with the committer’s verified signature.
vdwees jesse
1 parent c6ba480 commit 1091044
Showing 13 changed files with 2,822 additions and 2,531 deletions.
62 changes: 13 additions & 49 deletions examples/chip-tool/templates/ComplexArgumentParser-src.zapt
Original file line number Diff line number Diff line change
@@ -2,52 +2,16 @@

#include <commands/clusters/ComplexArgument.h>

{{#structs_with_clusters groupByStructName=1}}
CHIP_ERROR ComplexArgumentParser::Setup(const char * label, chip::app::Clusters::{{#unless (is_number_greater_than structClusterCount 1)}}{{as_camel_cased clusterName false}}{{else}}detail{{/unless}}::Structs::{{name}}::Type & request, Json::Value & value)
{
VerifyOrReturnError(value.isObject(), CHIP_ERROR_INVALID_ARGUMENT);

// Copy to track which members we already processed.
Json::Value valueCopy(value);

{{#zcl_struct_items}}
{{#unless isOptional}}
{{~! Fabric index fields are not sent on writes, so don't force people to
provide them. ~}}
{{#unless (is_num_equal fieldIdentifier 254)}}
ReturnErrorOnFailure(ComplexArgumentParser::EnsureMemberExist("{{parent.name}}.{{asLowerCamelCase label}}", "{{asLowerCamelCase label}}", value.isMember("{{asLowerCamelCase label}}")));
{{/unless}}
{{/unless}}
{{/zcl_struct_items}}

char labelWithMember[kMaxLabelLength];
{{#zcl_struct_items}}
{{#if isOptional}}
if (value.isMember("{{asLowerCamelCase label}}"))
{
{{else if (is_num_equal fieldIdentifier 254)}}
if (value.isMember("{{asLowerCamelCase label}}"))
{
{{/if}}
snprintf(labelWithMember, sizeof(labelWithMember), "%s.%s", label, "{{asLowerCamelCase label}}");
ReturnErrorOnFailure(ComplexArgumentParser::Setup(labelWithMember, request.{{asLowerCamelCase label}}, value["{{asLowerCamelCase label}}"]));
{{#if isOptional}}
}
{{else if (is_num_equal fieldIdentifier 254)}}
}
{{/if}}
valueCopy.removeMember("{{asLowerCamelCase label}}");

{{/zcl_struct_items}}

return ComplexArgumentParser::EnsureNoMembersRemaining(label, valueCopy);
}

void ComplexArgumentParser::Finalize(chip::app::Clusters::{{#unless (is_number_greater_than structClusterCount 1)}}{{as_camel_cased clusterName false}}{{else}}detail{{/unless}}::Structs::{{name}}::Type & request)
{
{{#zcl_struct_items}}
ComplexArgumentParser::Finalize(request.{{asLowerCamelCase label}});
{{/zcl_struct_items}}
}
{{/structs_with_clusters}}

{{#zcl_structs}}
{{#if has_more_than_one_cluster}}
{{> struct_parser_impl namespace="detail"}}
{{/if}}
{{/zcl_structs}}

{{#zcl_clusters}}
{{#zcl_structs}}
{{#unless has_more_than_one_cluster}}
{{> struct_parser_impl namespace=(as_camel_cased ../name false)}}
{{/unless}}
{{/zcl_structs}}
{{/zcl_clusters}}
16 changes: 12 additions & 4 deletions examples/chip-tool/templates/ComplexArgumentParser.zapt
Original file line number Diff line number Diff line change
@@ -5,8 +5,16 @@
#include <lib/core/CHIPError.h>
#include <app-common/zap-generated/cluster-objects.h>

{{#structs_with_clusters groupByStructName=1}}
static CHIP_ERROR Setup(const char * label, chip::app::Clusters::{{#unless (is_number_greater_than structClusterCount 1)}}{{as_camel_cased clusterName false}}{{else}}detail{{/unless}}::Structs::{{name}}::Type & request, Json::Value & value);
{{#zcl_structs}}
{{#if has_more_than_one_cluster}}
{{> struct_parser_decl namespace="detail"}}
{{/if}}
{{/zcl_structs}}

static void Finalize(chip::app::Clusters::{{#unless (is_number_greater_than structClusterCount 1)}}{{as_camel_cased clusterName false}}{{else}}detail{{/unless}}::Structs::{{name}}::Type & request);
{{/structs_with_clusters}}
{{#zcl_clusters}}
{{#zcl_structs}}
{{#unless has_more_than_one_cluster}}
{{> struct_parser_decl namespace=(as_camel_cased ../name false)}}
{{/unless}}
{{/zcl_structs}}
{{/zcl_clusters}}
30 changes: 12 additions & 18 deletions examples/chip-tool/templates/logging/DataModelLogger-src.zapt
Original file line number Diff line number Diff line change
@@ -4,25 +4,19 @@

using namespace chip::app::Clusters;

{{#structs_with_clusters groupByStructName=1}}
CHIP_ERROR DataModelLogger::LogValue(const char * label, size_t indent, const chip::app::Clusters::{{#unless (is_number_greater_than structClusterCount 1)}}{{as_camel_cased clusterName false}}{{else}}detail{{/unless}}::Structs::{{name}}::DecodableType & value)
{
DataModelLogger::LogString(label, indent, "{");
{{#zcl_struct_items}}
{
CHIP_ERROR err = LogValue("{{asUpperCamelCase label}}", indent + 1, value.{{asLowerCamelCase label}});
if (err != CHIP_NO_ERROR)
{
DataModelLogger::LogString(indent + 1, "Struct truncated due to invalid value for '{{asUpperCamelCase label}}'");
return err;
}
}
{{/zcl_struct_items}}
DataModelLogger::LogString(indent, "}");
{{#zcl_structs}}
{{#if has_more_than_one_cluster}}
{{> struct_logger_impl namespace="detail"}}
{{/if}}
{{/zcl_structs}}

return CHIP_NO_ERROR;
}
{{/structs_with_clusters}}
{{#zcl_clusters}}
{{#zcl_structs}}
{{#unless has_more_than_one_cluster}}
{{> struct_logger_impl namespace=(as_camel_cased ../name false)}}
{{/unless}}
{{/zcl_structs}}
{{/zcl_clusters}}

{{#zcl_clusters}}
{{#zcl_events}}
16 changes: 13 additions & 3 deletions examples/chip-tool/templates/logging/DataModelLogger.zapt
Original file line number Diff line number Diff line change
@@ -3,9 +3,19 @@
#include <lib/core/CHIPError.h>
#include <app-common/zap-generated/cluster-objects.h>

{{#structs_with_clusters groupByStructName=1}}
static CHIP_ERROR LogValue(const char * label, size_t indent, const chip::app::Clusters::{{#unless (is_number_greater_than structClusterCount 1)}}{{as_camel_cased clusterName false}}{{else}}detail{{/unless}}::Structs::{{name}}::DecodableType & value);
{{/structs_with_clusters}}
{{#zcl_structs}}
{{#if has_more_than_one_cluster}}
{{> struct_logger_decl namespace="detail"}}
{{/if}}
{{/zcl_structs}}

{{#zcl_clusters}}
{{#zcl_structs}}
{{#unless has_more_than_one_cluster}}
{{> struct_logger_decl namespace=(as_camel_cased ../name false)}}
{{/unless}}
{{/zcl_structs}}
{{/zcl_clusters}}

{{#zcl_clusters}}
{{#zcl_events}}
2 changes: 2 additions & 0 deletions examples/chip-tool/templates/partials/StructLoggerDecl.zapt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
static CHIP_ERROR LogValue(const char * label, size_t indent, const chip::app::Clusters::{{namespace}}::Structs::{{name}}::DecodableType & value);

18 changes: 18 additions & 0 deletions examples/chip-tool/templates/partials/StructLoggerImpl.zapt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
CHIP_ERROR DataModelLogger::LogValue(const char * label, size_t indent, const chip::app::Clusters::{{namespace}}::Structs::{{name}}::DecodableType & value)
{
DataModelLogger::LogString(label, indent, "{");
{{#zcl_struct_items}}
{
CHIP_ERROR err = LogValue("{{asUpperCamelCase label}}", indent + 1, value.{{asLowerCamelCase label}});
if (err != CHIP_NO_ERROR)
{
DataModelLogger::LogString(indent + 1, "Struct truncated due to invalid value for '{{asUpperCamelCase label}}'");
return err;
}
}
{{/zcl_struct_items}}
DataModelLogger::LogString(indent, "}");

return CHIP_NO_ERROR;
}

4 changes: 4 additions & 0 deletions examples/chip-tool/templates/partials/StructParserDecl.zapt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
static CHIP_ERROR Setup(const char * label, chip::app::Clusters::{{namespace}}::Structs::{{name}}::Type & request, Json::Value & value);

static void Finalize(chip::app::Clusters::{{namespace}}::Structs::{{name}}::Type & request);

47 changes: 47 additions & 0 deletions examples/chip-tool/templates/partials/StructParserImpl.zapt
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
CHIP_ERROR ComplexArgumentParser::Setup(const char * label, chip::app::Clusters::{{namespace}}::Structs::{{name}}::Type & request, Json::Value & value)
{
VerifyOrReturnError(value.isObject(), CHIP_ERROR_INVALID_ARGUMENT);

// Copy to track which members we already processed.
Json::Value valueCopy(value);

{{#zcl_struct_items}}
{{#unless isOptional}}
{{~! Fabric index fields are not sent on writes, so don't force people to
provide them. ~}}
{{#unless (is_num_equal fieldIdentifier 254)}}
ReturnErrorOnFailure(ComplexArgumentParser::EnsureMemberExist("{{parent.name}}.{{asLowerCamelCase label}}", "{{asLowerCamelCase label}}", value.isMember("{{asLowerCamelCase label}}")));
{{/unless}}
{{/unless}}
{{/zcl_struct_items}}

char labelWithMember[kMaxLabelLength];
{{#zcl_struct_items}}
{{#if isOptional}}
if (value.isMember("{{asLowerCamelCase label}}"))
{
{{else if (is_num_equal fieldIdentifier 254)}}
if (value.isMember("{{asLowerCamelCase label}}"))
{
{{/if}}
snprintf(labelWithMember, sizeof(labelWithMember), "%s.%s", label, "{{asLowerCamelCase label}}");
ReturnErrorOnFailure(ComplexArgumentParser::Setup(labelWithMember, request.{{asLowerCamelCase label}}, value["{{asLowerCamelCase label}}"]));
{{#if isOptional}}
}
{{else if (is_num_equal fieldIdentifier 254)}}
}
{{/if}}
valueCopy.removeMember("{{asLowerCamelCase label}}");

{{/zcl_struct_items}}

return ComplexArgumentParser::EnsureNoMembersRemaining(label, valueCopy);
}

void ComplexArgumentParser::Finalize(chip::app::Clusters::{{namespace}}::Structs::{{name}}::Type & request)
{
{{#zcl_struct_items}}
ComplexArgumentParser::Finalize(request.{{asLowerCamelCase label}});
{{/zcl_struct_items}}
}

16 changes: 16 additions & 0 deletions examples/chip-tool/templates/templates.json
Original file line number Diff line number Diff line change
@@ -21,6 +21,22 @@
{
"name": "cluster_header",
"path": "../../../src/app/zap-templates/partials/cluster_header.zapt"
},
{
"name": "struct_parser_decl",
"path": "partials/StructParserDecl.zapt"
},
{
"name": "struct_parser_impl",
"path": "partials/StructParserImpl.zapt"
},
{
"name": "struct_logger_decl",
"path": "partials/StructLoggerDecl.zapt"
},
{
"name": "struct_logger_impl",
"path": "partials/StructLoggerImpl.zapt"
}
],
"templates": [
3,105 changes: 1,581 additions & 1,524 deletions zzz_generated/chip-tool/zap-generated/cluster/ComplexArgumentParser.cpp

Large diffs are not rendered by default.

269 changes: 163 additions & 106 deletions zzz_generated/chip-tool/zap-generated/cluster/ComplexArgumentParser.h

Large diffs are not rendered by default.

1,591 changes: 824 additions & 767 deletions zzz_generated/chip-tool/zap-generated/cluster/logging/DataModelLogger.cpp

Large diffs are not rendered by default.

177 changes: 117 additions & 60 deletions zzz_generated/chip-tool/zap-generated/cluster/logging/DataModelLogger.h

Large diffs are not rendered by default.

0 comments on commit 1091044

Please sign in to comment.