Skip to content

Commit

Permalink
Make attributes whose values are lists of structs that have nullables…
Browse files Browse the repository at this point in the history 
…/optionals at least compile. (#11105)

This doesn't give us correct behavior across all of our bindings
(chip-tool command line, yaml, darwin, python, java).  But it at least
allows code generation to produce output and allows that output to
compile.

More work is needed to address the various TODO issues.
  • Loading branch information
@bzbarsky-apple @pull
bzbarsky-apple authored and pull[bot] committed May 17, 2023
1 parent 6446aad commit 2814570
Show file tree
Hide file tree
Showing 28 changed files with 1,116 additions and 249 deletions.
15 changes: 15 additions & 0 deletions examples/all-clusters-app/all-clusters-common/all-clusters-app.zap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15486,6 +15486,21 @@
"maxInterval": 65534,
"reportableChange": 0
},
{
"name": "list_nullables_and_optionals_struct",
"code": 35,
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
"reportable": 0,
"minInterval": 1,
"maxInterval": 65534,
"reportableChange": 0
},
{
"name": "ClusterRevision",
"code": 65533,
Expand Down
34 changes: 28 additions & 6 deletions examples/chip-tool/templates/commands.zapt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,19 +151,40 @@ static void On{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}ListAttri
uint16_t i = 0;
while (iter.Next())
{
++i;
#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", entry.{{asLowerCamelCase name}}.size());
{{~#*inline "field"}}entry.{{asLowerCamelCase name}}{{#if isOptional}}.Value(){{/if}}{{/inline~}}
{{~#*inline "fieldValue"}}{{>field}}{{#if isNullable}}.Value(){{/if}}{{/inline~}}
{{#if isOptional}}
if (entry.{{asLowerCamelCase name}}.HasValue()) {
{{/if}}
{{#if isNullable}}
if ({{>field}}.IsNull()) {
ChipLogProgress(chipTool, " {{asSymbol label}}: null");
} else {
{{/if}}
{{#if isArray}}
{{! TODO: Add support for printing list member of struct element of list attribute }}
ChipLogProgress(chipTool, " {{asSymbol label}}: list member of struct element of list attribute printing not supported yet");
{{else if (isOctetString type)}}
ChipLogProgress(Zcl, " {{asSymbol label}}: %zu", {{>fieldValue}}.size());
{{else if (isCharString type)}}
ChipLogProgress(Zcl, " {{asSymbol label}}: %.*s", static_cast<int>(entry.{{asLowerCamelCase name}}.size()), entry.{{asLowerCamelCase name}}.data());
ChipLogProgress(Zcl, " {{asSymbol label}}: %.*s", static_cast<int>({{>fieldValue}}.size()), {{>fieldValue}}.data());
{{else if isStruct}}
{{! TODO: Add support for printing struct member of struct element of list attribute }}
ChipLogProgress(chipTool, " {{asSymbol label}}: struct member of struct element of list attribute printing not supported yet");
{{else}}
ChipLogProgress(chipTool, " {{asLowerCamelCase name}}: {{asPrintFormat type}}", entry.{{asLowerCamelCase name}});
ChipLogProgress(chipTool, " {{asSymbol label}}: {{asPrintFormat type}}", {{>fieldValue}});
{{/if}}
{{#if isNullable}}
}
{{/if}}
{{#if isOptional}}
}
{{/if}}
{{/chip_attribute_list_entryTypes}}
{{else}}
Expand All @@ -175,6 +196,7 @@ static void On{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}ListAttri
ChipLogProgress(chipTool, "{{type}}[%" PRIu16 "]: {{asPrintFormat type}}", i, entry);
{{/if}}
{{/if}}
#endif // CHIP_PROGRESS_LOGGING
}
command->SetCommandExitStatus(iter.GetStatus());
}
Expand Down
16 changes: 16 additions & 0 deletions src/app/clusters/test-cluster-server/test-cluster-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class TestAttrAccess : public AttributeAccessInterface
CHIP_ERROR ReadListInt8uAttribute(AttributeValueEncoder & aEncoder);
CHIP_ERROR ReadListOctetStringAttribute(AttributeValueEncoder & aEncoder);
CHIP_ERROR ReadListStructOctetStringAttribute(AttributeValueEncoder & aEncoder);
CHIP_ERROR ReadListNullablesAndOptionalsStructAttribute(AttributeValueEncoder & aEncoder);
};

TestAttrAccess gAttrAccess;
Expand All @@ -75,6 +76,9 @@ CHIP_ERROR TestAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeVa
case ListStructOctetString::Id: {
return ReadListStructOctetStringAttribute(aEncoder);
}
case ListNullablesAndOptionalsStruct::Id: {
return ReadListNullablesAndOptionalsStructAttribute(aEncoder);
}
default: {
break;
}
Expand Down Expand Up @@ -221,6 +225,18 @@ CHIP_ERROR TestAttrAccess::ReadListStructOctetStringAttribute(AttributeValueEnco
return CHIP_NO_ERROR;
});
}

CHIP_ERROR TestAttrAccess::ReadListNullablesAndOptionalsStructAttribute(AttributeValueEncoder & aEncoder)
{
return aEncoder.EncodeList([](const TagBoundEncoder & encoder) -> CHIP_ERROR {
// Just encode a single default-initialized
// entry for now.
Structs::NullablesAndOptionalsStruct::Type entry;
ReturnErrorOnFailure(encoder.Encode(entry));
return CHIP_NO_ERROR;
});
}

} // namespace

bool emberAfTestClusterClusterTestCallback(app::CommandHandler *, const app::ConcreteCommandPath & commandPath,
Expand Down
2 changes: 2 additions & 0 deletions src/app/zap-templates/zcl/data-model/chip/test-cluster.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ limitations under the License.
<attribute side="server" code="0x0021" define="EPOCH_S" type="EPOCH_S" writable="true" optional="false">epoch_s</attribute>
<attribute side="server" code="0x0022" define="TEST_VENDOR_ID" type="vendor_id"
writable="true" optional="false" default="0">vendor_id</attribute>
<attribute side="server" code="0x0023" define="LIST_OF_STRUCTS_WITH_OPTIONALS" type="ARRAY" entryType="NullablesAndOptionalsStruct"
writable="false" optional="false">list_nullables_and_optionals_struct</attribute>
<!-- Can't enable the SimpleEnum test until we stop force-lowercasing
attribute types in ZAP -->
<!--<attribute side="server" code="0x0023" define="SIMPLE_ENUM" type="SimpleEnum" writable="true" optional="false">simple_enum</attribute>-->
Expand Down
15 changes: 15 additions & 0 deletions src/controller/data_model/controller-clusters.zap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11699,6 +11699,21 @@
"maxInterval": 65534,
"reportableChange": 0
},
{
"name": "list_nullables_and_optionals_struct",
"code": 35,
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
"reportable": 0,
"minInterval": 1,
"maxInterval": 65534,
"reportableChange": 0
},
{
"name": "unsupported",
"code": 255,
Expand Down
27 changes: 23 additions & 4 deletions src/controller/java/templates/CHIPClusters-JNI.zapt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ class CHIP{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}AttributeCall
VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Zcl, "Could not find class chip/devicecontroller/ChipClusters${{asUpperCamelCase parent.name}}Cluster${{asUpperCamelCase name}}Attribute"));
JniClass attributeJniClass(attributeClass);
jmethodID attributeCtor = env->GetMethodID(attributeClass, "<init>"
, "({{#chip_attribute_list_entryTypes}}{{#if (isString type)}}{{#if (isOctetString type)}}[B{{else}}Ljava/lang/String;{{/if}}{{else}}{{asJniSignature type}}{{/if}}{{/chip_attribute_list_entryTypes}})V");
, "({{#chip_attribute_list_entryTypes}}{{#if isOptional}}{{! TODO: Add support for optional types here }}{{else if isNullable}}{{! TODO: Add support for nullable types here }}{{else if isArray}}{{! TODO: Add support for lists here }}{{else if isStruct}}{{! TODO: Add support for structs here }}{{else if (isString type)}}{{#if (isOctetString type)}}[B{{else}}Ljava/lang/String;{{/if}}{{else}}{{asJniSignature type}}{{/if}}{{/chip_attribute_list_entryTypes}})V");
VerifyOrReturn(attributeCtor != nullptr, ChipLogError(Zcl, "Could not find {{asUpperCamelCase name}}Attribute constructor"));
{{/if}}

Expand All @@ -457,8 +457,17 @@ class CHIP{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}AttributeCall
{
auto & entry = iter.GetValue();
{{#if isStruct}}
(void)entry; {{! In case all our struct members are not supported yet }}
{{#chip_attribute_list_entryTypes}}
{{#if (isOctetString type)}}
{{#if isOptional}}
{{! TODO: Add support for optional types here }}
{{else if isNullable}}
{{! TODO: Add support for nullable types here }}
{{else if isArray}}
{{! TODO: Add support for lists here }}
{{else if isStruct}}
{{! TODO: Add support for structs here }}
{{else if (isOctetString type)}}
jbyteArray {{asLowerCamelCase name}} = env->NewByteArray(entry.{{asLowerCamelCase name}}.size());
env->SetByteArrayRegion({{asLowerCamelCase name}}, 0, entry.{{asLowerCamelCase name}}.size(), reinterpret_cast<const jbyte *>(entry.{{asLowerCamelCase name}}.data()));
{{else if (isCharString type)}}
Expand All @@ -469,9 +478,19 @@ class CHIP{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}AttributeCall
{{/if}}
{{/chip_attribute_list_entryTypes}}

jobject attributeObj = env->NewObject(attributeClass, attributeCtor,
jobject attributeObj = env->NewObject(attributeClass, attributeCtor
{{#chip_attribute_list_entryTypes}}
{{asLowerCamelCase name}}{{#not_last}}, {{/not_last}}
{{#if isOptional}}
{{! TODO: Add support for optional types here }}
{{else if isNullable}}
{{! TODO: Add support for nullable types here }}
{{else if isArray}}
{{! TODO: Add support for lists here }}
{{else if isStruct}}
{{! TODO: Add support for structs here }}
{{else}}
, {{asLowerCamelCase name}}
{{/if}}
{{/chip_attribute_list_entryTypes}}
);
VerifyOrReturn(attributeObj != nullptr, ChipLogError(Zcl, "Could not create {{asUpperCamelCase name}}Attribute object"));
Expand Down
30 changes: 28 additions & 2 deletions src/controller/java/templates/ChipClusters-java.zapt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,15 @@ public class ChipClusters {
{{#if isStruct}}
public static class {{asUpperCamelCase name}}Attribute {
{{#chip_attribute_list_entryTypes}}
{{#if (isOctetString type)}}
{{#if isOptional}}
{{! TODO: Add support for optional types here }}
{{else if isNullable}}
{{! TODO: Add support for nullable types here }}
{{else if isArray}}
{{! TODO: Add support for lists here }}
{{else if isStruct}}
{{! TODO: Add support for structs here }}
{{else if (isOctetString type)}}
public byte[] {{asLowerCamelCase name}};
{{else if (isCharString type)}}
public String {{asLowerCamelCase name}};
Expand All @@ -121,7 +129,15 @@ public class ChipClusters {

public {{asUpperCamelCase name}}Attribute(
{{#chip_attribute_list_entryTypes}}
{{#if (isOctetString type)}}
{{#if isOptional}}
{{! TODO: Add support for optional types here }}
{{else if isNullable}}
{{! TODO: Add support for nullable types here }}
{{else if isArray}}
{{! TODO: Add support for lists here }}
{{else if isStruct}}
{{! TODO: Add support for structs here }}
{{else if (isOctetString type)}}
byte[] {{asLowerCamelCase name}}{{#not_last}},{{/not_last}}
{{else if (isCharString type)}}
String {{asLowerCamelCase name}}{{#not_last}},{{/not_last}}
Expand All @@ -131,7 +147,17 @@ public class ChipClusters {
{{/chip_attribute_list_entryTypes}}
) {
{{#chip_attribute_list_entryTypes}}
{{#if isOptional}}
{{! TODO: Add support for optional types here }}
{{else if isNullable}}
{{! TODO: Add support for nullable types here }}
{{else if isArray}}
{{! TODO: Add support for lists here }}
{{else if isStruct}}
{{! TODO: Add support for structs here }}
{{else}}
this.{{asLowerCamelCase name}} = {{asLowerCamelCase name}};
{{/if}}
{{/chip_attribute_list_entryTypes}}
}
}
Expand Down
Loading

0 comments on commit 2814570

Please sign in to comment.