Skip to content

Commit

Permalink
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.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Feb 16, 2024
1 parent aacd0c8 commit 0696182
Show file tree
Hide file tree
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
Expand Up @@ -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
Expand Up @@ -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
Expand Up @@ -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}}
Expand Down
16 changes: 13 additions & 3 deletions examples/chip-tool/templates/logging/DataModelLogger.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
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
Expand Up @@ -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": [
Expand Down
Loading

0 comments on commit 0696182

Please sign in to comment.