Skip to content

Commit

Permalink
Fix some places where we use OCTET_STRING when we should use CHAR_STR…
Browse files Browse the repository at this point in the history
…ING (#10745)

* Fix various OCTET_STRING bits that should be CHAR_STRING.

Fixes #5542
Fixes #6112
Fixes #7112
Fixes #7322
Fixes #7654
Fixes #7655
Fixes #8704
Fixes #8705
Fixes #8706
Fixes #8707
Fixes #9797
Fixes #9798
Fixes #10508
Fixes #10509

* Regenerate generated files
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jan 28, 2022
1 parent ada569b commit 7e1240f
Show file tree
Hide file tree
Showing 51 changed files with 382 additions and 301 deletions.
4 changes: 2 additions & 2 deletions examples/bridge-app/esp32/main/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ void EncodeFixedLabel(const char * label, const char * value, uint8_t * buffer,
uint16_t listCount = 1;
_LabelStruct labelStruct;

labelStruct.label = chip::ByteSpan(Uint8::from_const_char(label), strlen(label));
labelStruct.value = chip::ByteSpan(Uint8::from_const_char(value), strlen(value));
labelStruct.label = chip::CharSpan(label, strlen(label));
labelStruct.value = chip::CharSpan(value, strlen(value));

emberAfCopyList(ZCL_FIXED_LABEL_CLUSTER_ID, am, true, buffer, reinterpret_cast<uint8_t *>(&labelStruct), 1);
emberAfCopyList(ZCL_FIXED_LABEL_CLUSTER_ID, am, true, buffer, reinterpret_cast<uint8_t *>(&listCount), 0);
Expand Down
8 changes: 6 additions & 2 deletions examples/bridge-app/linux/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,14 @@ void EncodeFixedLabel(const char * label, const char * value, uint8_t * buffer,
uint16_t listCount = 1;
_LabelStruct labelStruct;

labelStruct.label = chip::ByteSpan(reinterpret_cast<const uint8_t *>(label), kFixedLabelElementsOctetStringSize);
// TODO: This size is obviously wrong. See
// https://github.com/project-chip/connectedhomeip/issues/10743
labelStruct.label = CharSpan(label, kFixedLabelElementsOctetStringSize);

strncpy(zclOctetStrBuf, value, sizeof(zclOctetStrBuf));
labelStruct.value = chip::ByteSpan(reinterpret_cast<uint8_t *>(&zclOctetStrBuf[0]), sizeof(zclOctetStrBuf));
// TODO: This size is obviously wrong. See
// https://github.com/project-chip/connectedhomeip/issues/10743
labelStruct.value = CharSpan(&zclOctetStrBuf[0], sizeof(zclOctetStrBuf));

emberAfCopyList(ZCL_FIXED_LABEL_CLUSTER_ID, am, true, buffer, reinterpret_cast<uint8_t *>(&labelStruct), 1);
emberAfCopyList(ZCL_FIXED_LABEL_CLUSTER_ID, am, true, buffer, reinterpret_cast<uint8_t *>(&listCount), 0);
Expand Down
1 change: 0 additions & 1 deletion examples/chip-tool/templates/partials/test_cluster.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ class {{filename}}: public TestCommand
{{/chip_tests_item_parameters}}

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}}data.{{asLowerCamelCase name}}{{/chip_tests_item_response_parameters}});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ CHIP_ERROR AudioOutputManager::proxyGetListOfAudioOutputInfo(chip::app::Attribut
{
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.name = chip::CharSpan(name, sizeof(name) - 1);
audioOutputInfo.index = static_cast<uint8_t>(1 + i);
ReturnErrorOnFailure(encoder.Encode(audioOutputInfo));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ CHIP_ERROR MediaInputManager::proxyGetInputList(chip::app::AttributeValueEncoder
for (int i = 0; i < maximumVectorSize; ++i)
{
chip::app::Clusters::MediaInput::Structs::MediaInputInfo::Type mediaInput;
mediaInput.description = chip::ByteSpan(chip::Uint8::from_char(description), sizeof(description) - 1);
mediaInput.name = chip::ByteSpan(chip::Uint8::from_char(name), sizeof(name) - 1);
mediaInput.description = chip::CharSpan(description, sizeof(description) - 1);
mediaInput.name = chip::CharSpan(name, sizeof(name) - 1);
mediaInput.inputType = EMBER_ZCL_MEDIA_INPUT_TYPE_HDMI;
mediaInput.index = static_cast<uint8_t>(1 + i);
ReturnErrorOnFailure(encoder.Encode(mediaInput));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ CHIP_ERROR TargetNavigatorManager::proxyGetTargetInfoList(chip::app::AttributeVa
for (int i = 0; i < maximumVectorSize; ++i)
{
chip::app::Clusters::TargetNavigator::Structs::NavigateTargetTargetInfo::Type targetInfo;
targetInfo.name = chip::ByteSpan(chip::Uint8::from_char(name), sizeof(name) - 1);
targetInfo.name = chip::CharSpan(name, sizeof(name) - 1);
targetInfo.identifier = static_cast<uint8_t>(1 + i);
ReturnErrorOnFailure(encoder.Encode(targetInfo));
}
Expand Down
6 changes: 3 additions & 3 deletions examples/tv-app/linux/include/tv-channel/TvChannelManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ CHIP_ERROR TvChannelManager::proxyGetTvChannelList(chip::app::AttributeValueEnco
for (int i = 0; i < maximumVectorSize; ++i)
{
chip::app::Clusters::TvChannel::Structs::TvChannelInfo::Type channelInfo;
channelInfo.affiliateCallSign = ByteSpan(Uint8::from_char(affiliateCallSign), sizeof(affiliateCallSign) - 1);
channelInfo.callSign = ByteSpan(Uint8::from_char(callSign), sizeof(callSign) - 1);
channelInfo.name = ByteSpan(Uint8::from_char(name), sizeof(name) - 1);
channelInfo.affiliateCallSign = CharSpan(affiliateCallSign, sizeof(affiliateCallSign) - 1);
channelInfo.callSign = CharSpan(callSign, sizeof(callSign) - 1);
channelInfo.name = CharSpan(name, sizeof(name) - 1);
channelInfo.majorNumber = static_cast<uint8_t>(1 + i);
channelInfo.minorNumber = static_cast<uint16_t>(2 + i);
ReturnErrorOnFailure(encoder.Encode(channelInfo));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,7 @@ CHIP_ERROR OperationalCredentialsAttrAccess::ReadFabricsList(EndpointId endpoint
fabricDescriptor.vendorId = fabricInfo.GetVendorId();
fabricDescriptor.fabricId = fabricInfo.GetFabricId();

// TODO: The type of 'label' should be 'CharSpan', need to fix the XML definition for broken member type.
fabricDescriptor.label =
ByteSpan(Uint8::from_const_char(fabricInfo.GetFabricLabel().data()), fabricInfo.GetFabricLabel().size());
fabricDescriptor.label = fabricInfo.GetFabricLabel();
fabricDescriptor.rootPublicKey = fabricInfo.GetRootPubkey();

ReturnErrorOnFailure(encoder.Encode(fabricDescriptor));
Expand Down Expand Up @@ -176,7 +174,7 @@ EmberAfStatus writeFabric(FabricIndex fabricIndex, FabricId fabricId, NodeId nod
fabricDescriptor->NodeId = nodeId;
if (!fabricLabel.empty())
{
fabricDescriptor->Label = ByteSpan(Uint8::from_const_char(fabricLabel.data()), fabricLabel.size());
fabricDescriptor->Label = fabricLabel;
}

emberAfPrintln(EMBER_AF_PRINT_DEBUG,
Expand Down Expand Up @@ -407,7 +405,7 @@ namespace {

FabricInfo gFabricBeingCommissioned;

CHIP_ERROR SendNOCResponse(app::Command * commandObj, EmberAfNodeOperationalCertStatus status, uint8_t index, ByteSpan debug_text)
CHIP_ERROR SendNOCResponse(app::Command * commandObj, EmberAfNodeOperationalCertStatus status, uint8_t index, CharSpan debug_text)
{
app::CommandPathParams cmdParams = { emberAfCurrentEndpoint(), /* group id */ 0, ZCL_OPERATIONAL_CREDENTIALS_CLUSTER_ID,
ZCL_NOC_RESPONSE_COMMAND_ID, (app::CommandPathFlags::kEndpointIdValid) };
Expand All @@ -422,8 +420,7 @@ CHIP_ERROR SendNOCResponse(app::Command * commandObj, EmberAfNodeOperationalCert
{
ReturnErrorOnFailure(writer->Put(TLV::ContextTag(1), index));
}
// TODO: Change DebugText to CHAR_STRING once strings are supported in command/response fields
ReturnErrorOnFailure(writer->Put(TLV::ContextTag(2), debug_text));
ReturnErrorOnFailure(writer->PutString(TLV::ContextTag(2), debug_text));
return commandObj->FinishCommand();
}

Expand Down Expand Up @@ -494,7 +491,7 @@ bool emberAfOperationalCredentialsClusterAddNOCCallback(app::CommandHandler * co
exit:

gFabricBeingCommissioned.Reset();
SendNOCResponse(commandObj, nocResponse, fabricIndex, ByteSpan());
SendNOCResponse(commandObj, nocResponse, fabricIndex, CharSpan());

if (nocResponse != EMBER_ZCL_NODE_OPERATIONAL_CERT_STATUS_SUCCESS)
{
Expand Down Expand Up @@ -538,7 +535,7 @@ bool emberAfOperationalCredentialsClusterUpdateNOCCallback(app::CommandHandler *

exit:

SendNOCResponse(commandObj, nocResponse, fabricIndex, ByteSpan());
SendNOCResponse(commandObj, nocResponse, fabricIndex, CharSpan());

if (nocResponse != EMBER_ZCL_NODE_OPERATIONAL_CERT_STATUS_SUCCESS)
{
Expand Down
7 changes: 1 addition & 6 deletions src/app/clusters/ota-provider/ota-provider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,19 +189,14 @@ bool emberAfOtaSoftwareUpdateProviderClusterQueryImageCallback(app::CommandHandl

ChipLogDetail(Zcl, "OTA Provider received QueryImage");

// TODO: (#7112) support location param and verify length once CHAR_STRING is supported
// Using location parameter is blocked by #5542 (use Span for string arguments). For now, there is no way to safely get the
// length of the location string because it is not guaranteed to be null-terminated.
Span<const char> locationSpan;

if (metadataForProvider.size() > kMaxMetadataLen)
{
ChipLogError(Zcl, "metadata size %zu exceeds max %zu", metadataForProvider.size(), kMaxMetadataLen);
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_ARGUMENT);
}

status = delegate->HandleQueryImage(commandObj, vendorId, productId, hardwareVersion, softwareVersion, protocolsSupported,
locationSpan, requestorCanConsent, metadataForProvider);
commandData.location, requestorCanConsent, metadataForProvider);
if (status != EMBER_ZCL_STATUS_SUCCESS)
{
emberAfSendImmediateDefaultResponse(status);
Expand Down
6 changes: 6 additions & 0 deletions src/app/zap-templates/templates/app/attribute-size-src.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <app/util/af.h>
#include <app/util/attribute-list-byte-span.h>
#include <app/util/basic-types.h>
#include <lib/core/CHIPSafeCasts.h>
#include <lib/support/SafeInt.h>
#include <lib/support/logging/CHIPLogging.h>

Expand Down Expand Up @@ -85,7 +86,12 @@ uint16_t emberAfCopyList(ClusterId clusterId, EmberAfAttributeMetadata * am, boo
{{chipType}} * entry = reinterpret_cast<{{chipType}} *>(write ? src : dest);
{{#chip_attribute_list_entryTypes}}
{{#if (isString type)}}
{{#if (isOctetString type)}}
ByteSpan * {{name}}Span = &entry->{{name}}; // {{type}}
{{else}}
ByteSpan {{name}}SpanStorage(Uint8::from_const_char(entry->{{name}}.data()), entry->{{name}}.size()); // {{type}}
ByteSpan * {{name}}Span = &{{name}}SpanStorage;
{{/if}}
if (CHIP_NO_ERROR != (write ? WriteByteSpan(dest + entryOffset, {{size}}, {{name}}Span) : ReadByteSpan(src + entryOffset, {{size}}, {{name}}Span)))
{
ChipLogError(Zcl, "Index %" PRId32 " is invalid. Not enough remaining space", index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ limitations under the License.
<cluster code="0x050b"/>
<item name="index" type="INT8U"/>
<item name="outputType" type="AudioOutputType"/>
<item name="name" type="OCTET_STRING" length="32"/> <!-- Change this to CHAR_STRING once it is supported #6112 -->
<item name="name" type="CHAR_STRING" length="32"/>
</struct>

<enum name="AudioOutputType" type="ENUM8">
Expand All @@ -58,4 +58,4 @@ limitations under the License.
<item name="Other" value="0x05"/>
</enum>

</configurator>
</configurator>
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ limitations under the License.

<struct name="LabelStruct">
<cluster code="0x0040"/>
<item name="label" type="OCTET_STRING" length="16"/> <!-- TODO: Change this to CHAR_STRING once it is supported #6112 -->
<item name="value" type="OCTET_STRING" length="16"/> <!-- TODO: Change this to CHAR_STRING once it is supported #6112 -->
<item name="label" type="CHAR_STRING" length="16"/>
<item name="value" type="CHAR_STRING" length="16"/>
</struct>

<cluster>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ limitations under the License.
</enum>
<struct name="NetworkInterfaceType">
<cluster code="0x0033"/>
<!-- TODO: CHAR_STRING not supported yet in structs. -->
<item name="Name" type="OCTET_STRING" length="32"/>
<item name="Name" type="CHAR_STRING" length="32"/>
<item name="FabricConnected" type="BOOLEAN"/>
<item name="OffPremiseServicesReachableIPv4" type="BOOLEAN"/>
<item name="OffPremiseServicesReachableIPv6" type="BOOLEAN"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ limitations under the License.
<cluster code="0x0507"/>
<item name="index" type="INT8U"/>
<item name="inputType" type="MediaInputType"/>
<item name="name" type="OCTET_STRING" length="32"/> <!-- Change this to CHAR_STRING once it is supported #6112 -->
<item name="description" type="OCTET_STRING" length="32"/> <!-- Change this to CHAR_STRING once it is supported #6112 -->
<item name="name" type="CHAR_STRING" length="32"/>
<item name="description" type="CHAR_STRING" length="32"/>
</struct>

<enum name="MediaInputType" type="ENUM8">
Expand All @@ -73,4 +73,4 @@ limitations under the License.
<item name="Other" value="0x0B"/>
</enum>

</configurator>
</configurator>
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ limitations under the License.
<item name="VendorId" type="INT16U"/> <!-- Change INT16U to new type VendorID #2395 -->
<item name="FabricId" type="FABRIC_ID"/>
<item name="NodeId" type="NODE_ID"/>
<item name="Label" type="OCTET_STRING" length="32"/><!-- TODO: Add Label once CHAR_STRING or OCTET_STRING works -->
<item name="Label" type="CHAR_STRING" length="32"/>
</struct>

<enum name="NodeOperationalCertStatus" type="ENUM8">
Expand Down Expand Up @@ -108,12 +108,11 @@ limitations under the License.
<arg name="ICACValue" type="OCTET_STRING" optional="true"/>
</command>

<!-- TODO: Change DebugText to CHAR_STRING once strings are supported in command/response fields -->
<command source="server" code="0x08" name="NOCResponse" optional="false">
<description>Response to AddNOC or UpdateNOC commands.</description>
<arg name="StatusCode" type="INT8U"/>
<arg name="FabricIndex" type="INT8U"/>
<arg name="DebugText" type="OCTET_STRING"/>
<arg name="DebugText" type="CHAR_STRING"/>
</command>

<command source="client" code="0x09" name="UpdateFabricLabel" optional="false">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ limitations under the License.
<struct name="ThreadMetrics">
<cluster code="0x0034"/>
<item name="Id" type="INT64U"/>
<!-- TODO: CHAR_STRING not supported yet in structs. -->
<item name="Name" type="OCTET_STRING" length="8"/>
<item name="Name" type="CHAR_STRING" length="8"/>
<item name="StackFreeCurrent" type="INT32U"/>
<item name="StackFreeMinimum" type="INT32U"/>
<item name="StackSize" type="INT32U"/>
Expand All @@ -39,4 +38,4 @@ limitations under the License.
<description>Reception of this command SHALL reset the values: The StackFreeMinimum field of the ThreadMetrics attribute, CurrentHeapHighWaterMark attribute</description>
</command>
</cluster>
</configurator>
</configurator>
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ limitations under the License.
<struct name="NavigateTargetTargetInfo">
<cluster code="0x0505"/>
<item name="identifier" type="INT8U"/>
<item name="name" type="OCTET_STRING" length="32"/> <!-- Change this to CHAR_STRING once it is supported #6112 -->
<item name="name" type="CHAR_STRING" length="32"/>
</struct>
</configurator>
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ limitations under the License.
<cluster code="0x0504"/>
<item name="majorNumber" type="INT16U"/>
<item name="minorNumber" type="INT16U"/>
<item name="name" type="OCTET_STRING" length="32"/> <!-- Change this to CHAR_STRING once it is supported #6112 -->
<item name="callSign" type="OCTET_STRING" length="32"/> <!-- Change this to CHAR_STRING once it is supported #6112 -->
<item name="affiliateCallSign" type="OCTET_STRING" length="32"/> <!-- Change this to CHAR_STRING once it is supported #6112 -->
<item name="name" type="CHAR_STRING" length="32"/>
<item name="callSign" type="CHAR_STRING" length="32"/>
<item name="affiliateCallSign" type="CHAR_STRING" length="32"/>
</struct>

<struct name="TvChannelLineupInfo">
Expand All @@ -83,4 +83,4 @@ limitations under the License.
<item name="NoMatches" value="0x01"/>
</enum>

</configurator>
</configurator>
2 changes: 1 addition & 1 deletion src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1447,7 +1447,7 @@ void DeviceCommissioner::OnAddNOCFailureResponse(void * context, uint8_t status)
}

void DeviceCommissioner::OnOperationalCertificateAddResponse(void * context, uint8_t StatusCode, uint8_t FabricIndex,
ByteSpan DebugText)
CharSpan DebugText)
{
ChipLogProgress(Controller, "Device returned status %d on receiving the NOC", StatusCode);
DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
Expand Down
2 changes: 1 addition & 1 deletion src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
/* Callback when adding operational certs to device results in failure */
static void OnAddNOCFailureResponse(void * context, uint8_t status);
/* Callback when the device confirms that it has added the operational certificates */
static void OnOperationalCertificateAddResponse(void * context, uint8_t StatusCode, uint8_t FabricIndex, ByteSpan DebugText);
static void OnOperationalCertificateAddResponse(void * context, uint8_t StatusCode, uint8_t FabricIndex, CharSpan DebugText);

/* Callback when the device confirms that it has added the root certificate */
static void OnRootCertSuccessResponse(void * context);
Expand Down
6 changes: 4 additions & 2 deletions src/controller/java/templates/CHIPClusters-JNI.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,8 @@ class CHIP{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}AttributeCall
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)}}
// Implement after ByteSpan is emitted instead of uint8_t *.
UtfString {{asLowerCamelCase name}}Str(env, entry.{{asLowerCamelCase name}});
jstring {{asLowerCamelCase name}}({{asLowerCamelCase name}}Str.jniValue());
{{else}}
{{asJniBasicType type}} {{asLowerCamelCase name}} = entry.{{asLowerCamelCase name}};
{{/if}}
Expand All @@ -481,7 +482,8 @@ class CHIP{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}AttributeCall
jbyteArray {{asLowerCamelCase name}} = env->NewByteArray(entry.size());
env->SetByteArrayRegion({{asLowerCamelCase name}}, 0, entry.size(), reinterpret_cast<const jbyte *>(entry.data()));
{{else if (isCharString type)}}
// Implement after ByteSpan is emitted instead of uint8_t *
UtfString {{asLowerCamelCase name}}Str(env, entry);
jstring {{asLowerCamelCase name}}({{asLowerCamelCase name}}Str.jniValue());
{{else}}
jclass entryTypeCls;
JniReferences::GetInstance().GetClassRef(env, "java/lang/{{asJavaBasicTypeForZclType type true}}", entryTypeCls);
Expand Down
Loading

0 comments on commit 7e1240f

Please sign in to comment.