Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some places where we use OCTET_STRING when we should use CHAR_STRING #10745

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -179,8 +179,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