From 5eb33dc297d6754bd7668e8cc3a151e22cd34cd2 Mon Sep 17 00:00:00 2001 From: ajoletta-amzn Date: Fri, 4 Aug 2023 17:17:34 -0700 Subject: [PATCH 1/6] Added the ability to transmit and display device name in the Matter SDK Added the ability to transmit and display device name in the Matter SDK --- .../all-clusters-app/linux/main-common.cpp | 1 + src/app/server/Dnssd.cpp | 16 +++++++++---- src/include/platform/ConfigurationManager.h | 17 +++++++------- .../platform/DeviceInstanceInfoProvider.h | 9 ++++++++ .../GenericConfigurationManagerImpl.h | 2 +- .../GenericConfigurationManagerImpl.ipp | 6 ++--- .../GenericDeviceInstanceInfoProvider.h | 1 + .../GenericDeviceInstanceInfoProvider.ipp | 12 ++++++++++ src/platform/Ameba/FactoryDataProvider.cpp | 2 +- src/platform/Ameba/FactoryDataProvider.h | 1 + src/platform/android/AndroidConfig.cpp | 1 + src/platform/android/AndroidConfig.h | 1 + .../DeviceInstanceInfoProviderImpl.cpp | 23 +++++++++++++++++++ .../android/DeviceInstanceInfoProviderImpl.h | 1 + .../chip/platform/ConfigurationManager.java | 1 + .../PreferencesConfigurationManager.java | 8 +++++++ .../common/FactoryDataProvider.cpp | 5 ++++ .../bouffalolab/common/FactoryDataProvider.h | 1 + src/platform/fake/ConfigurationManagerImpl.h | 2 +- .../nrfconnect/FactoryDataProvider.cpp | 6 +++++ src/platform/nrfconnect/FactoryDataProvider.h | 1 + .../nxp/k32w/common/FactoryDataProvider.cpp | 6 +++++ .../nxp/k32w/common/FactoryDataProvider.h | 2 ++ .../nxp/mw320/FactoryDataProvider.cpp | 5 ++++ src/platform/nxp/mw320/FactoryDataProvider.h | 1 + src/platform/qpg/FactoryDataProvider.cpp | 5 ++++ src/platform/qpg/FactoryDataProvider.h | 1 + src/platform/telink/FactoryDataProvider.cpp | 6 +++++ src/platform/telink/FactoryDataProvider.h | 1 + 29 files changed, 125 insertions(+), 19 deletions(-) diff --git a/examples/all-clusters-app/linux/main-common.cpp b/examples/all-clusters-app/linux/main-common.cpp index a6deafde128983..af72c4a1d49d4e 100644 --- a/examples/all-clusters-app/linux/main-common.cpp +++ b/examples/all-clusters-app/linux/main-common.cpp @@ -144,6 +144,7 @@ class ExampleDeviceInstanceInfoProvider : public DeviceInstanceInfoProvider public: void Init(DeviceInstanceInfoProvider * defaultProvider) { mDefaultProvider = defaultProvider; } + CHIP_ERROR GetDeviceName(MutableCharSpan & deviceNameSpan) override { return mDefaultProvider->GetDeviceName(deviceNameSpan); } CHIP_ERROR GetVendorName(char * buf, size_t bufSize) override { return mDefaultProvider->GetVendorName(buf, bufSize); } CHIP_ERROR GetVendorId(uint16_t & vendorId) override { return mDefaultProvider->GetVendorId(vendorId); } CHIP_ERROR GetProductName(char * buf, size_t bufSize) override { return mDefaultProvider->GetProductName(buf, bufSize); } diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 86a70a0a8c69e2..c3411160f0f71d 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -234,11 +234,19 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi advertiseParameters.SetDeviceType(chip::Optional::Value(val32)); } - char deviceName[chip::Dnssd::kKeyDeviceNameMaxLength + 1]; - if (DeviceLayer::ConfigurationMgr().IsCommissionableDeviceNameEnabled() && - DeviceLayer::ConfigurationMgr().GetCommissionableDeviceName(deviceName, sizeof(deviceName)) == CHIP_NO_ERROR) + char deviceName[chip::Dnssd::kKeyDeviceNameMaxLength]; + MutableCharSpan deviceNameSpan(deviceName); + if (DeviceLayer::ConfigurationMgr().IsCommissionableDeviceNameEnabled()) { - advertiseParameters.SetDeviceName(chip::Optional::Value(deviceName)); + CHIP_ERROR err; + if ((err = DeviceLayer::ConfigurationMgr().GetCommissionableDeviceName(deviceNameSpan)) == CHIP_NO_ERROR) + { + advertiseParameters.SetDeviceName(chip::Optional::Value(deviceNameSpan.data())); + } + else + { + ChipLogError(Discovery, "Failed to read commissionableDeviceName %" CHIP_ERROR_FORMAT, err.Format()); + } } advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig()).SetTcpSupported(Optional(INET_CONFIG_ENABLE_TCP_ENDPOINT)); diff --git a/src/include/platform/ConfigurationManager.h b/src/include/platform/ConfigurationManager.h index e846e420f5dc79..2d037b9988000e 100644 --- a/src/include/platform/ConfigurationManager.h +++ b/src/include/platform/ConfigurationManager.h @@ -64,6 +64,7 @@ class ConfigurationManager enum { + kMaxDeviceNameLength = 32, kMaxVendorNameLength = 32, kMaxProductNameLength = 32, kMaxLocationLength = 2, @@ -138,14 +139,14 @@ class ConfigurationManager virtual void LogDeviceConfig() = 0; - virtual bool IsCommissionableDeviceTypeEnabled() = 0; - virtual CHIP_ERROR GetDeviceTypeId(uint32_t & deviceType) = 0; - virtual bool IsCommissionableDeviceNameEnabled() = 0; - virtual CHIP_ERROR GetCommissionableDeviceName(char * buf, size_t bufSize) = 0; - virtual CHIP_ERROR GetInitialPairingHint(uint16_t & pairingHint) = 0; - virtual CHIP_ERROR GetInitialPairingInstruction(char * buf, size_t bufSize) = 0; - virtual CHIP_ERROR GetSecondaryPairingHint(uint16_t & pairingHint) = 0; - virtual CHIP_ERROR GetSecondaryPairingInstruction(char * buf, size_t bufSize) = 0; + virtual bool IsCommissionableDeviceTypeEnabled() = 0; + virtual CHIP_ERROR GetDeviceTypeId(uint32_t & deviceType) = 0; + virtual bool IsCommissionableDeviceNameEnabled() = 0; + virtual CHIP_ERROR GetCommissionableDeviceName(MutableCharSpan & deviceNameSpan) = 0; + virtual CHIP_ERROR GetInitialPairingHint(uint16_t & pairingHint) = 0; + virtual CHIP_ERROR GetInitialPairingInstruction(char * buf, size_t bufSize) = 0; + virtual CHIP_ERROR GetSecondaryPairingHint(uint16_t & pairingHint) = 0; + virtual CHIP_ERROR GetSecondaryPairingInstruction(char * buf, size_t bufSize) = 0; virtual CHIP_ERROR GetLocationCapability(uint8_t & location); diff --git a/src/include/platform/DeviceInstanceInfoProvider.h b/src/include/platform/DeviceInstanceInfoProvider.h index c072df6b55a999..48cd65f43fab59 100644 --- a/src/include/platform/DeviceInstanceInfoProvider.h +++ b/src/include/platform/DeviceInstanceInfoProvider.h @@ -29,6 +29,15 @@ class DeviceInstanceInfoProvider DeviceInstanceInfoProvider() = default; virtual ~DeviceInstanceInfoProvider() = default; + /** + * @brief Obtain the Device Name from the device's factory data. + * + * @param[out] deviceName Reference to location where the device name will be copied + * @returns CHIP_NO_ERROR on success, or another CHIP_ERROR from the underlying implementation + * if access fails. + */ + virtual CHIP_ERROR GetDeviceName(MutableCharSpan & deviceName) = 0; + /** * @brief Obtain the Vendor Name from the device's factory data. * diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.h b/src/include/platform/internal/GenericConfigurationManagerImpl.h index 21310be54641d9..d3d624b02bb537 100644 --- a/src/include/platform/internal/GenericConfigurationManagerImpl.h +++ b/src/include/platform/internal/GenericConfigurationManagerImpl.h @@ -85,7 +85,7 @@ class GenericConfigurationManagerImpl : public ConfigurationManager bool IsCommissionableDeviceTypeEnabled() override; CHIP_ERROR GetDeviceTypeId(uint32_t & deviceType) override; bool IsCommissionableDeviceNameEnabled() override; - CHIP_ERROR GetCommissionableDeviceName(char * buf, size_t bufSize) override; + CHIP_ERROR GetCommissionableDeviceName(MutableCharSpan & deviceNameSpan) override; CHIP_ERROR GetInitialPairingHint(uint16_t & pairingHint) override; CHIP_ERROR GetInitialPairingInstruction(char * buf, size_t bufSize) override; CHIP_ERROR GetSecondaryPairingHint(uint16_t & pairingHint) override; diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.ipp b/src/include/platform/internal/GenericConfigurationManagerImpl.ipp index 4f77ae9f975162..6305d445dc1c58 100644 --- a/src/include/platform/internal/GenericConfigurationManagerImpl.ipp +++ b/src/include/platform/internal/GenericConfigurationManagerImpl.ipp @@ -640,11 +640,9 @@ bool GenericConfigurationManagerImpl::IsCommissionableDeviceNameEna } template -CHIP_ERROR GenericConfigurationManagerImpl::GetCommissionableDeviceName(char * buf, size_t bufSize) +CHIP_ERROR GenericConfigurationManagerImpl::GetCommissionableDeviceName(MutableCharSpan & deviceNameSpan) { - ReturnErrorCodeIf(bufSize < sizeof(CHIP_DEVICE_CONFIG_DEVICE_NAME), CHIP_ERROR_BUFFER_TOO_SMALL); - strcpy(buf, CHIP_DEVICE_CONFIG_DEVICE_NAME); - return CHIP_NO_ERROR; + return GetDeviceInstanceInfoProvider()->GetDeviceName(deviceNameSpan); } template diff --git a/src/include/platform/internal/GenericDeviceInstanceInfoProvider.h b/src/include/platform/internal/GenericDeviceInstanceInfoProvider.h index 831b49a0037e7e..96593847bbd8a9 100644 --- a/src/include/platform/internal/GenericDeviceInstanceInfoProvider.h +++ b/src/include/platform/internal/GenericDeviceInstanceInfoProvider.h @@ -41,6 +41,7 @@ class GenericDeviceInstanceInfoProvider : public DeviceInstanceInfoProvider mGenericConfigManager(configManager) {} + CHIP_ERROR GetDeviceName(MutableCharSpan & deviceNameSpan) override; CHIP_ERROR GetVendorName(char * buf, size_t bufSize) override; CHIP_ERROR GetVendorId(uint16_t & vendorId) override; CHIP_ERROR GetProductName(char * buf, size_t bufSize) override; diff --git a/src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp b/src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp index 549c3eb360aa95..ea0cef7cef5f87 100644 --- a/src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp +++ b/src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp @@ -21,6 +21,18 @@ namespace chip { namespace DeviceLayer { namespace Internal { +template +CHIP_ERROR GenericDeviceInstanceInfoProvider::GetDeviceName(MutableCharSpan & deviceNameSpan) +{ + constexpr char deviceName[] = CHIP_DEVICE_CONFIG_DEVICE_NAME; + + ReturnErrorCodeIf(sizeof(deviceName) - 1 > deviceNameSpan.size(), CHIP_ERROR_BUFFER_TOO_SMALL); + memcpy(deviceNameSpan.data(), deviceName, sizeof(deviceName) - 1); + deviceNameSpan.reduce_size(sizeof(deviceName) - 1); + + return CHIP_NO_ERROR; +} + template CHIP_ERROR GenericDeviceInstanceInfoProvider::GetVendorId(uint16_t & vendorId) { diff --git a/src/platform/Ameba/FactoryDataProvider.cpp b/src/platform/Ameba/FactoryDataProvider.cpp index 71243a1b7cfe84..15537a8a074f55 100644 --- a/src/platform/Ameba/FactoryDataProvider.cpp +++ b/src/platform/Ameba/FactoryDataProvider.cpp @@ -414,7 +414,7 @@ CHIP_ERROR FactoryDataProvider::GetSetupPasscode(uint32_t & setupPasscode) return err; } -CHIP_ERROR FactoryDataProvider::SetSetupPasscode(uint32_t setupPasscode) +CHIP_ERROR FactoryDataProvider::GetDeviceName(MutableCharSpan & deviceNameSpan) { return CHIP_ERROR_NOT_IMPLEMENTED; } diff --git a/src/platform/Ameba/FactoryDataProvider.h b/src/platform/Ameba/FactoryDataProvider.h index c0f09c300dd0a2..325c4b972ccf43 100644 --- a/src/platform/Ameba/FactoryDataProvider.h +++ b/src/platform/Ameba/FactoryDataProvider.h @@ -48,6 +48,7 @@ class FactoryDataProvider : public chip::Credentials::DeviceAttestationCredentia CHIP_ERROR SetSetupPasscode(uint32_t setupPasscode) override; // ===== Members functions that implement the DeviceInstanceInfoProvider + CHIP_ERROR GetDeviceName(MutableCharSpan & deviceNameSpan) override; CHIP_ERROR GetVendorName(char * buf, size_t bufSize) override; CHIP_ERROR GetVendorId(uint16_t & vendorId) override; CHIP_ERROR GetProductName(char * buf, size_t bufSize) override; diff --git a/src/platform/android/AndroidConfig.cpp b/src/platform/android/AndroidConfig.cpp index ec9221f08ba12e..28cddf9dea892c 100644 --- a/src/platform/android/AndroidConfig.cpp +++ b/src/platform/android/AndroidConfig.cpp @@ -60,6 +60,7 @@ const char AndroidConfig::kConfigNamespace_ChipCounters[] = "chip-counters"; // Keys stored in the Chip-factory namespace const AndroidConfig::Key AndroidConfig::kConfigKey_SerialNum = { kConfigNamespace_ChipFactory, "serial-num" }; +const AndroidConfig::Key AndroidConfig::kConfigKey_MfrDeviceName = { kConfigNamespace_ChipFactory, "device-name" }; const AndroidConfig::Key AndroidConfig::kConfigKey_MfrDeviceId = { kConfigNamespace_ChipFactory, "device-id" }; const AndroidConfig::Key AndroidConfig::kConfigKey_MfrDeviceCert = { kConfigNamespace_ChipFactory, "device-cert" }; const AndroidConfig::Key AndroidConfig::kConfigKey_MfrDeviceICACerts = { kConfigNamespace_ChipFactory, "device-ca-certs" }; diff --git a/src/platform/android/AndroidConfig.h b/src/platform/android/AndroidConfig.h index 507a2b62e87da8..1646d50cd25c70 100644 --- a/src/platform/android/AndroidConfig.h +++ b/src/platform/android/AndroidConfig.h @@ -54,6 +54,7 @@ class AndroidConfig // Key definitions for well-known keys. static const Key kConfigKey_SerialNum; + static const Key kConfigKey_MfrDeviceName; static const Key kConfigKey_MfrDeviceId; static const Key kConfigKey_MfrDeviceCert; static const Key kConfigKey_MfrDeviceICACerts; diff --git a/src/platform/android/DeviceInstanceInfoProviderImpl.cpp b/src/platform/android/DeviceInstanceInfoProviderImpl.cpp index cd07215c378e74..be8b1076556cbd 100644 --- a/src/platform/android/DeviceInstanceInfoProviderImpl.cpp +++ b/src/platform/android/DeviceInstanceInfoProviderImpl.cpp @@ -73,6 +73,29 @@ CHIP_ERROR DeviceInstanceInfoProviderImpl::GetProductLabel(char * buf, size_t bu return Internal::AndroidConfig::ReadConfigValueStr(Internal::AndroidConfig::kConfigKey_ProductLabel, buf, bufSize, dateLen); } +CHIP_ERROR DeviceInstanceInfoProviderImpl::GetDeviceName(MutableCharSpan & deviceNameSpan) +{ + CHIP_ERROR err; + size_t deviceNameSize = 0; + char androidDeviceName[ConfigurationManager::kMaxDeviceNameLength]; + MutableCharSpan androidDeviceNameSpan(androidDeviceName); + + err = Internal::AndroidConfig::ReadConfigValueStr(Internal::AndroidConfig::kConfigKey_MfrDeviceName, + androidDeviceNameSpan.data(), androidDeviceNameSpan.size(), deviceNameSize); + ReturnErrorCodeIf(deviceNameSpan.size() < deviceNameSize, CHIP_ERROR_BUFFER_TOO_SMALL); + memcpy(deviceNameSpan.data(), androidDeviceNameSpan.data(), deviceNameSize); + deviceNameSpan.data()[deviceNameSize] = '\0'; + deviceNameSpan.reduce_size(deviceNameSize + 1); + if (err == CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND) + { + ReturnErrorCodeIf(deviceNameSpan.size() < sizeof(CHIP_DEVICE_CONFIG_DEVICE_NAME), CHIP_ERROR_BUFFER_TOO_SMALL); + memcpy(deviceNameSpan.data(), CHIP_DEVICE_CONFIG_DEVICE_NAME, sizeof(CHIP_DEVICE_CONFIG_DEVICE_NAME)); + + deviceNameSpan.reduce_size(sizeof(CHIP_DEVICE_CONFIG_DEVICE_NAME)); + } + return CHIP_NO_ERROR; +} + CHIP_ERROR DeviceInstanceInfoProviderImpl::GetProductName(char * buf, size_t bufSize) { CHIP_ERROR err; diff --git a/src/platform/android/DeviceInstanceInfoProviderImpl.h b/src/platform/android/DeviceInstanceInfoProviderImpl.h index 489861d502fbce..e57e331035afa5 100644 --- a/src/platform/android/DeviceInstanceInfoProviderImpl.h +++ b/src/platform/android/DeviceInstanceInfoProviderImpl.h @@ -27,6 +27,7 @@ namespace DeviceLayer { class DeviceInstanceInfoProviderImpl : public Internal::GenericDeviceInstanceInfoProvider { public: + CHIP_ERROR GetDeviceName(MutableCharSpan & deviceNameSpan) override; CHIP_ERROR GetProductName(char * buf, size_t bufSize) override; CHIP_ERROR GetProductId(uint16_t & productId) override; CHIP_ERROR GetPartNumber(char * buf, size_t bufSize) override; diff --git a/src/platform/android/java/chip/platform/ConfigurationManager.java b/src/platform/android/java/chip/platform/ConfigurationManager.java index 3c9ab3fa91ba14..881f9417640369 100644 --- a/src/platform/android/java/chip/platform/ConfigurationManager.java +++ b/src/platform/android/java/chip/platform/ConfigurationManager.java @@ -26,6 +26,7 @@ public interface ConfigurationManager { // Keys stored in the Chip-factory namespace String kConfigKey_SerialNum = "serial-num"; + String kConfigKey_MfrDeviceName = "device-name"; String kConfigKey_MfrDeviceId = "device-id"; String kConfigKey_MfrDeviceCert = "device-cert"; String kConfigKey_MfrDeviceICACerts = "device-ca-certs"; diff --git a/src/platform/android/java/chip/platform/PreferencesConfigurationManager.java b/src/platform/android/java/chip/platform/PreferencesConfigurationManager.java index 0069f033c6d77b..b2bc9fc10a2fef 100644 --- a/src/platform/android/java/chip/platform/PreferencesConfigurationManager.java +++ b/src/platform/android/java/chip/platform/PreferencesConfigurationManager.java @@ -102,6 +102,14 @@ public String readConfigValueStr(String namespace, String name) String key = getKey(namespace, name); switch (key) { + /** + * Human readable name of the device name. + * + *

return a different value than src/include/platform/CHIPDeviceConfig.h for debug + */ + case kConfigNamespace_ChipFactory + ":" + kConfigKey_MfrDeviceName: + return "TEST_ANDROID_DEVICE_NAME"; + /** * Human readable name of the device model. return a different value than * src/include/platform/CHIPDeviceConfig.h for debug diff --git a/src/platform/bouffalolab/common/FactoryDataProvider.cpp b/src/platform/bouffalolab/common/FactoryDataProvider.cpp index 5970549234ffa2..6fa855225a5319 100644 --- a/src/platform/bouffalolab/common/FactoryDataProvider.cpp +++ b/src/platform/bouffalolab/common/FactoryDataProvider.cpp @@ -398,6 +398,11 @@ CHIP_ERROR FactoryDataProvider::SetSetupPasscode(uint32_t setupPasscode) return CHIP_ERROR_NOT_IMPLEMENTED; } +CHIP_ERROR FactoryDataProvider::GetDeviceName(MutableCharSpan & deviceNameSpan) +{ + return CHIP_ERROR_NOT_IMPLEMENTED; +} + CHIP_ERROR FactoryDataProvider::GetVendorName(char * buf, size_t bufSize) { #if CONFIG_BOUFFALOLAB_FACTORY_DATA_ENABLE diff --git a/src/platform/bouffalolab/common/FactoryDataProvider.h b/src/platform/bouffalolab/common/FactoryDataProvider.h index 80cc166034e7f3..55777774fdc39f 100644 --- a/src/platform/bouffalolab/common/FactoryDataProvider.h +++ b/src/platform/bouffalolab/common/FactoryDataProvider.h @@ -50,6 +50,7 @@ class FactoryDataProvider : public chip::Credentials::DeviceAttestationCredentia CHIP_ERROR SetSetupPasscode(uint32_t setupPasscode) override; // ===== Members functions that implement the DeviceInstanceInfoProvider + CHIP_ERROR GetDeviceName(MutableCharSpan & deviceNameSpan) override; CHIP_ERROR GetVendorName(char * buf, size_t bufSize) override; CHIP_ERROR GetVendorId(uint16_t & vendorId) override; CHIP_ERROR GetProductName(char * buf, size_t bufSize) override; diff --git a/src/platform/fake/ConfigurationManagerImpl.h b/src/platform/fake/ConfigurationManagerImpl.h index 9347d22052c247..cc7cc7d1c9b844 100644 --- a/src/platform/fake/ConfigurationManagerImpl.h +++ b/src/platform/fake/ConfigurationManagerImpl.h @@ -67,7 +67,7 @@ class ConfigurationManagerImpl : public ConfigurationManager bool IsCommissionableDeviceTypeEnabled() override { return false; } CHIP_ERROR GetDeviceTypeId(uint32_t & deviceType) override { return CHIP_ERROR_NOT_IMPLEMENTED; } bool IsCommissionableDeviceNameEnabled() override { return false; } - CHIP_ERROR GetCommissionableDeviceName(char * buf, size_t bufSize) override { return CHIP_ERROR_NOT_IMPLEMENTED; } + CHIP_ERROR GetCommissionableDeviceName(MutableCharSpan & deviceNameSpan) override { return CHIP_ERROR_NOT_IMPLEMENTED; } CHIP_ERROR GetInitialPairingHint(uint16_t & pairingHint) override { return CHIP_ERROR_NOT_IMPLEMENTED; } CHIP_ERROR GetInitialPairingInstruction(char * buf, size_t bufSize) override { return CHIP_ERROR_NOT_IMPLEMENTED; } CHIP_ERROR GetSecondaryPairingHint(uint16_t & pairingHint) override { return CHIP_ERROR_NOT_IMPLEMENTED; } diff --git a/src/platform/nrfconnect/FactoryDataProvider.cpp b/src/platform/nrfconnect/FactoryDataProvider.cpp index caa1ad434f9254..2eb0b91ac31d6e 100644 --- a/src/platform/nrfconnect/FactoryDataProvider.cpp +++ b/src/platform/nrfconnect/FactoryDataProvider.cpp @@ -239,6 +239,12 @@ CHIP_ERROR FactoryDataProvider::SetSetupPasscode(uint32_t setu return CHIP_ERROR_NOT_IMPLEMENTED; } +template +CHIP_ERROR FactoryDataProvider::GetDeviceName(MutableCharSpan & deviceNameSpan) +{ + return CHIP_ERROR_NOT_IMPLEMENTED; +} + template CHIP_ERROR FactoryDataProvider::GetVendorName(char * buf, size_t bufSize) { diff --git a/src/platform/nrfconnect/FactoryDataProvider.h b/src/platform/nrfconnect/FactoryDataProvider.h index 15dae3ab7e3a0e..ed315d2fc573cb 100644 --- a/src/platform/nrfconnect/FactoryDataProvider.h +++ b/src/platform/nrfconnect/FactoryDataProvider.h @@ -127,6 +127,7 @@ class FactoryDataProvider : public chip::Credentials::DeviceAttestationCredentia CHIP_ERROR SetSetupPasscode(uint32_t setupPasscode) override; // ===== Members functions that implement the DeviceInstanceInfoProvider + CHIP_ERROR GetDeviceName(MutableCharSpan & deviceNameSpan) override; CHIP_ERROR GetVendorName(char * buf, size_t bufSize) override; CHIP_ERROR GetVendorId(uint16_t & vendorId) override; CHIP_ERROR GetProductName(char * buf, size_t bufSize) override; diff --git a/src/platform/nxp/k32w/common/FactoryDataProvider.cpp b/src/platform/nxp/k32w/common/FactoryDataProvider.cpp index 98bb1653210601..d521bfa9dca0a6 100644 --- a/src/platform/nxp/k32w/common/FactoryDataProvider.cpp +++ b/src/platform/nxp/k32w/common/FactoryDataProvider.cpp @@ -57,6 +57,7 @@ FactoryDataProvider::FactoryDataProvider() maxLengths[FactoryDataId::kVidId] = sizeof(uint16_t); maxLengths[FactoryDataId::kPidId] = sizeof(uint16_t); maxLengths[FactoryDataId::kCertDeclarationId] = Credentials::kMaxCMSSignedCDMessage; + maxLengths[FactoryDataId::kDeviceName] = ConfigurationManager::kMaxDeviceNameLength; maxLengths[FactoryDataId::kVendorNameId] = ConfigurationManager::kMaxVendorNameLength; maxLengths[FactoryDataId::kProductNameId] = ConfigurationManager::kMaxProductNameLength; maxLengths[FactoryDataId::kSerialNumberId] = ConfigurationManager::kMaxSerialNumberLength; @@ -222,6 +223,11 @@ CHIP_ERROR FactoryDataProvider::SetSetupPasscode(uint32_t setupPasscode) return CHIP_ERROR_NOT_IMPLEMENTED; } +CHIP_ERROR FactoryDataProvider::GetDeviceName(MutableCharSpan & deviceNameSpan) +{ + return CHIP_ERROR_NOT_IMPLEMENTED; +} + CHIP_ERROR FactoryDataProvider::GetVendorName(char * buf, size_t bufSize) { uint16_t length = 0; diff --git a/src/platform/nxp/k32w/common/FactoryDataProvider.h b/src/platform/nxp/k32w/common/FactoryDataProvider.h index db10a8f6accb23..92a7ea9d779366 100644 --- a/src/platform/nxp/k32w/common/FactoryDataProvider.h +++ b/src/platform/nxp/k32w/common/FactoryDataProvider.h @@ -78,6 +78,7 @@ class FactoryDataProvider : public DeviceInstanceInfoProvider, kVidId, kPidId, kCertDeclarationId, + kDeviceNameId, kVendorNameId, kProductNameId, kSerialNumberId, @@ -130,6 +131,7 @@ class FactoryDataProvider : public DeviceInstanceInfoProvider, CHIP_ERROR SignWithDeviceAttestationKey(const ByteSpan & messageToSign, MutableByteSpan & outSignBuffer) override; // ===== Members functions that implement the GenericDeviceInstanceInfoProvider + CHIP_ERROR GetDeviceName(MutableCharSpan & deviceNameSpan) override; CHIP_ERROR GetVendorName(char * buf, size_t bufSize) override; CHIP_ERROR GetVendorId(uint16_t & vendorId) override; CHIP_ERROR GetProductName(char * buf, size_t bufSize) override; diff --git a/src/platform/nxp/mw320/FactoryDataProvider.cpp b/src/platform/nxp/mw320/FactoryDataProvider.cpp index 5b36958f306d5b..0099b60fea3bc5 100644 --- a/src/platform/nxp/mw320/FactoryDataProvider.cpp +++ b/src/platform/nxp/mw320/FactoryDataProvider.cpp @@ -267,6 +267,11 @@ CHIP_ERROR FactoryDataProvider::SetSetupPasscode(uint32_t setupPasscode) } #if CHIP_DEVICE_CONFIG_ENABLE_DEVICE_INSTANCE_INFO_PROVIDER +CHIP_ERROR FactoryDataProvider::GetDeviceName(MutableCharSpan & devicenameSpan) +{ + return CHIP_ERROR_NOT_IMPLEMENTED; +} + CHIP_ERROR FactoryDataProvider::GetVendorName(char * buf, size_t bufSize) { return CHIP_ERROR_NOT_IMPLEMENTED; diff --git a/src/platform/nxp/mw320/FactoryDataProvider.h b/src/platform/nxp/mw320/FactoryDataProvider.h index 07afa4a393c5f7..8800b1d6e66362 100644 --- a/src/platform/nxp/mw320/FactoryDataProvider.h +++ b/src/platform/nxp/mw320/FactoryDataProvider.h @@ -62,6 +62,7 @@ class FactoryDataProvider : public CommissionableDataProvider, #if CHIP_DEVICE_CONFIG_ENABLE_DEVICE_INSTANCE_INFO_PROVIDER // ===== Members functions that implement the GenericDeviceInstanceInfoProvider + CHIP_ERROR GetDeviceName(MutableCharSpan & deviceNameSpan) override; CHIP_ERROR GetVendorName(char * buf, size_t bufSize) override; CHIP_ERROR GetVendorId(uint16_t & vendorId) override; CHIP_ERROR GetProductName(char * buf, size_t bufSize) override; diff --git a/src/platform/qpg/FactoryDataProvider.cpp b/src/platform/qpg/FactoryDataProvider.cpp index a3bdab2fcf4088..74e81ab8bedcb4 100644 --- a/src/platform/qpg/FactoryDataProvider.cpp +++ b/src/platform/qpg/FactoryDataProvider.cpp @@ -240,6 +240,11 @@ CHIP_ERROR FactoryDataProvider::GetProductLabel(char * buf, size_t bufSize) return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; } +CHIP_ERROR FactoryDataProvider::GetDeviceName(MutableCharSpan & deviceNameSpan) +{ + return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; +} + CHIP_ERROR FactoryDataProvider::GetVendorName(char * buf, size_t bufSize) { qvStatus_t status; diff --git a/src/platform/qpg/FactoryDataProvider.h b/src/platform/qpg/FactoryDataProvider.h index ef4faa3389d903..f9202480d7d6b4 100644 --- a/src/platform/qpg/FactoryDataProvider.h +++ b/src/platform/qpg/FactoryDataProvider.h @@ -50,6 +50,7 @@ class FactoryDataProvider : public chip::Credentials::DeviceAttestationCredentia CHIP_ERROR SetSetupPasscode(uint32_t setupPasscode) override; // ===== Members functions that implement the DeviceInstanceInfoProvider + CHIP_ERROR GetDeviceName(MutableCharSpan & deviceNameSpan) override; CHIP_ERROR GetVendorName(char * buf, size_t bufSize) override; CHIP_ERROR GetVendorId(uint16_t & vendorId) override; CHIP_ERROR GetProductName(char * buf, size_t bufSize) override; diff --git a/src/platform/telink/FactoryDataProvider.cpp b/src/platform/telink/FactoryDataProvider.cpp index 806cdb2cfe9391..ad0d9ee9f1995d 100644 --- a/src/platform/telink/FactoryDataProvider.cpp +++ b/src/platform/telink/FactoryDataProvider.cpp @@ -238,6 +238,12 @@ CHIP_ERROR FactoryDataProvider::SetSetupPasscode(uint32_t setu return CHIP_ERROR_NOT_IMPLEMENTED; } +template +CHIP_ERROR FactoryDataProvider::GetDeviceName(MutableCharSpan & deviceNameSpan) +{ + return CHIP_ERROR_NOT_IMPLEMENTED; +} + template CHIP_ERROR FactoryDataProvider::GetVendorName(char * buf, size_t bufSize) { diff --git a/src/platform/telink/FactoryDataProvider.h b/src/platform/telink/FactoryDataProvider.h index 829831e554aa33..56ee532251c42f 100644 --- a/src/platform/telink/FactoryDataProvider.h +++ b/src/platform/telink/FactoryDataProvider.h @@ -91,6 +91,7 @@ class FactoryDataProvider : public chip::Credentials::DeviceAttestationCredentia CHIP_ERROR SetSetupPasscode(uint32_t setupPasscode) override; // ===== Members functions that implement the DeviceInstanceInfoProvider + CHIP_ERROR GetDeviceName(MutableCharSpan & deviceNameSpan) override; CHIP_ERROR GetVendorName(char * buf, size_t bufSize) override; CHIP_ERROR GetVendorId(uint16_t & vendorId) override; CHIP_ERROR GetProductName(char * buf, size_t bufSize) override; From a5a27867d265b7e713b7d3ff18013e2e42114bb7 Mon Sep 17 00:00:00 2001 From: ajoletta-amzn Date: Fri, 11 Aug 2023 11:11:48 -0700 Subject: [PATCH 2/6] Changed variable name for max deviceName length --- src/include/platform/ConfigurationManager.h | 2 +- src/platform/android/DeviceInstanceInfoProviderImpl.cpp | 2 +- src/platform/nxp/k32w/common/FactoryDataProvider.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/include/platform/ConfigurationManager.h b/src/include/platform/ConfigurationManager.h index 2d037b9988000e..77ac7dc6627815 100644 --- a/src/include/platform/ConfigurationManager.h +++ b/src/include/platform/ConfigurationManager.h @@ -64,7 +64,7 @@ class ConfigurationManager enum { - kMaxDeviceNameLength = 32, + kMaxDeviceNameLen = 32, kMaxVendorNameLength = 32, kMaxProductNameLength = 32, kMaxLocationLength = 2, diff --git a/src/platform/android/DeviceInstanceInfoProviderImpl.cpp b/src/platform/android/DeviceInstanceInfoProviderImpl.cpp index be8b1076556cbd..faf888e6ecb221 100644 --- a/src/platform/android/DeviceInstanceInfoProviderImpl.cpp +++ b/src/platform/android/DeviceInstanceInfoProviderImpl.cpp @@ -77,7 +77,7 @@ CHIP_ERROR DeviceInstanceInfoProviderImpl::GetDeviceName(MutableCharSpan & devic { CHIP_ERROR err; size_t deviceNameSize = 0; - char androidDeviceName[ConfigurationManager::kMaxDeviceNameLength]; + char androidDeviceName[ConfigurationManager::kMaxDeviceNameLen]; MutableCharSpan androidDeviceNameSpan(androidDeviceName); err = Internal::AndroidConfig::ReadConfigValueStr(Internal::AndroidConfig::kConfigKey_MfrDeviceName, diff --git a/src/platform/nxp/k32w/common/FactoryDataProvider.cpp b/src/platform/nxp/k32w/common/FactoryDataProvider.cpp index d521bfa9dca0a6..ac46b11d9f3244 100644 --- a/src/platform/nxp/k32w/common/FactoryDataProvider.cpp +++ b/src/platform/nxp/k32w/common/FactoryDataProvider.cpp @@ -57,7 +57,7 @@ FactoryDataProvider::FactoryDataProvider() maxLengths[FactoryDataId::kVidId] = sizeof(uint16_t); maxLengths[FactoryDataId::kPidId] = sizeof(uint16_t); maxLengths[FactoryDataId::kCertDeclarationId] = Credentials::kMaxCMSSignedCDMessage; - maxLengths[FactoryDataId::kDeviceName] = ConfigurationManager::kMaxDeviceNameLength; + maxLengths[FactoryDataId::kDeviceName] = ConfigurationManager::kMaxDeviceNameLen; maxLengths[FactoryDataId::kVendorNameId] = ConfigurationManager::kMaxVendorNameLength; maxLengths[FactoryDataId::kProductNameId] = ConfigurationManager::kMaxProductNameLength; maxLengths[FactoryDataId::kSerialNumberId] = ConfigurationManager::kMaxSerialNumberLength; From 8b9f7cde2018a740d164225a717f6105c8d5ace9 Mon Sep 17 00:00:00 2001 From: ajoletta-amzn Date: Fri, 11 Aug 2023 12:22:09 -0700 Subject: [PATCH 3/6] Moved appending null terminator from getDeviceName to the DNSSD just before SetDeviceName is called --- src/app/server/Dnssd.cpp | 4 +++- src/platform/Ameba/FactoryDataProvider.cpp | 5 +++++ .../DeviceInstanceInfoProviderImpl.cpp | 19 +++++++++++-------- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index c3411160f0f71d..013a55623e40e8 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -241,7 +241,9 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi CHIP_ERROR err; if ((err = DeviceLayer::ConfigurationMgr().GetCommissionableDeviceName(deviceNameSpan)) == CHIP_NO_ERROR) { - advertiseParameters.SetDeviceName(chip::Optional::Value(deviceNameSpan.data())); + // Appending a null terminator since SetDeviceName expects a null terminated string + deviceName[deviceNameSpan.size()] = '\0'; + advertiseParameters.SetDeviceName(chip::Optional::Value(deviceName)); } else { diff --git a/src/platform/Ameba/FactoryDataProvider.cpp b/src/platform/Ameba/FactoryDataProvider.cpp index 15537a8a074f55..2cc004940728f3 100644 --- a/src/platform/Ameba/FactoryDataProvider.cpp +++ b/src/platform/Ameba/FactoryDataProvider.cpp @@ -414,6 +414,11 @@ CHIP_ERROR FactoryDataProvider::GetSetupPasscode(uint32_t & setupPasscode) return err; } +CHIP_ERROR FactoryDataProvider::SetSetupPasscode(uint32_t setupPasscode) +{ + return CHIP_ERROR_NOT_IMPLEMENTED; +} + CHIP_ERROR FactoryDataProvider::GetDeviceName(MutableCharSpan & deviceNameSpan) { return CHIP_ERROR_NOT_IMPLEMENTED; diff --git a/src/platform/android/DeviceInstanceInfoProviderImpl.cpp b/src/platform/android/DeviceInstanceInfoProviderImpl.cpp index faf888e6ecb221..d1858e2a83551d 100644 --- a/src/platform/android/DeviceInstanceInfoProviderImpl.cpp +++ b/src/platform/android/DeviceInstanceInfoProviderImpl.cpp @@ -82,16 +82,19 @@ CHIP_ERROR DeviceInstanceInfoProviderImpl::GetDeviceName(MutableCharSpan & devic err = Internal::AndroidConfig::ReadConfigValueStr(Internal::AndroidConfig::kConfigKey_MfrDeviceName, androidDeviceNameSpan.data(), androidDeviceNameSpan.size(), deviceNameSize); - ReturnErrorCodeIf(deviceNameSpan.size() < deviceNameSize, CHIP_ERROR_BUFFER_TOO_SMALL); - memcpy(deviceNameSpan.data(), androidDeviceNameSpan.data(), deviceNameSize); - deviceNameSpan.data()[deviceNameSize] = '\0'; - deviceNameSpan.reduce_size(deviceNameSize + 1); - if (err == CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND) + + if (err = CHIP_NO_ERROR) + { + ReturnErrorCodeIf(deviceNameSpan.size() < deviceNameSize, CHIP_ERROR_BUFFER_TOO_SMALL); + memcpy(deviceNameSpan.data(), androidDeviceNameSpan.data(), deviceNameSize); + deviceNameSpan.reduce_size(deviceNameSize); + } + else if (err == CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND) { - ReturnErrorCodeIf(deviceNameSpan.size() < sizeof(CHIP_DEVICE_CONFIG_DEVICE_NAME), CHIP_ERROR_BUFFER_TOO_SMALL); - memcpy(deviceNameSpan.data(), CHIP_DEVICE_CONFIG_DEVICE_NAME, sizeof(CHIP_DEVICE_CONFIG_DEVICE_NAME)); + ReturnErrorCodeIf(deviceNameSpan.size() < strlen(CHIP_DEVICE_CONFIG_DEVICE_NAME), CHIP_ERROR_BUFFER_TOO_SMALL); + memcpy(deviceNameSpan.data(), CHIP_DEVICE_CONFIG_DEVICE_NAME, strlen(CHIP_DEVICE_CONFIG_DEVICE_NAME)); - deviceNameSpan.reduce_size(sizeof(CHIP_DEVICE_CONFIG_DEVICE_NAME)); + deviceNameSpan.reduce_size(strlen(CHIP_DEVICE_CONFIG_DEVICE_NAME)); } return CHIP_NO_ERROR; } From d12368db621b34fa395d233f3fc4c3a1f879ba15 Mon Sep 17 00:00:00 2001 From: ajoletta-amzn Date: Fri, 11 Aug 2023 13:43:39 -0700 Subject: [PATCH 4/6] Added the == to address @sharadb-amazon --- src/platform/android/DeviceInstanceInfoProviderImpl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/android/DeviceInstanceInfoProviderImpl.cpp b/src/platform/android/DeviceInstanceInfoProviderImpl.cpp index d1858e2a83551d..264b767067c4d7 100644 --- a/src/platform/android/DeviceInstanceInfoProviderImpl.cpp +++ b/src/platform/android/DeviceInstanceInfoProviderImpl.cpp @@ -83,7 +83,7 @@ CHIP_ERROR DeviceInstanceInfoProviderImpl::GetDeviceName(MutableCharSpan & devic err = Internal::AndroidConfig::ReadConfigValueStr(Internal::AndroidConfig::kConfigKey_MfrDeviceName, androidDeviceNameSpan.data(), androidDeviceNameSpan.size(), deviceNameSize); - if (err = CHIP_NO_ERROR) + if (err == CHIP_NO_ERROR) { ReturnErrorCodeIf(deviceNameSpan.size() < deviceNameSize, CHIP_ERROR_BUFFER_TOO_SMALL); memcpy(deviceNameSpan.data(), androidDeviceNameSpan.data(), deviceNameSize); From 0455c9245f24560675cf3a4ce2a9710e184b3ba8 Mon Sep 17 00:00:00 2001 From: ajoletta-amzn Date: Mon, 14 Aug 2023 16:19:10 -0700 Subject: [PATCH 5/6] Addressing feedback from bzbarsky-apple --- src/app/server/Dnssd.cpp | 6 +++--- src/include/platform/ConfigurationManager.h | 7 ++++--- src/include/platform/DeviceInstanceInfoProvider.h | 3 ++- .../internal/GenericDeviceInstanceInfoProvider.ipp | 6 +----- src/lib/dnssd/TxtFields.h | 4 ++++ .../android/DeviceInstanceInfoProviderImpl.cpp | 12 ++++-------- 6 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 013a55623e40e8..41ae02ccae3473 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -234,8 +234,8 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi advertiseParameters.SetDeviceType(chip::Optional::Value(val32)); } - char deviceName[chip::Dnssd::kKeyDeviceNameMaxLength]; - MutableCharSpan deviceNameSpan(deviceName); + char deviceName[Dnssd::kKeyDeviceNameMaxLength + 1]; + MutableCharSpan deviceNameSpan(deviceName, Dnssd::kKeyDeviceNameMaxLength); if (DeviceLayer::ConfigurationMgr().IsCommissionableDeviceNameEnabled()) { CHIP_ERROR err; @@ -243,7 +243,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi { // Appending a null terminator since SetDeviceName expects a null terminated string deviceName[deviceNameSpan.size()] = '\0'; - advertiseParameters.SetDeviceName(chip::Optional::Value(deviceName)); + advertiseParameters.SetDeviceName(Optional::Value(deviceName)); } else { diff --git a/src/include/platform/ConfigurationManager.h b/src/include/platform/ConfigurationManager.h index 77ac7dc6627815..364f0fc7d17c14 100644 --- a/src/include/platform/ConfigurationManager.h +++ b/src/include/platform/ConfigurationManager.h @@ -139,9 +139,10 @@ class ConfigurationManager virtual void LogDeviceConfig() = 0; - virtual bool IsCommissionableDeviceTypeEnabled() = 0; - virtual CHIP_ERROR GetDeviceTypeId(uint32_t & deviceType) = 0; - virtual bool IsCommissionableDeviceNameEnabled() = 0; + virtual bool IsCommissionableDeviceTypeEnabled() = 0; + virtual CHIP_ERROR GetDeviceTypeId(uint32_t & deviceType) = 0; + virtual bool IsCommissionableDeviceNameEnabled() = 0; + // deviceNameSpan gets resized to the actual size of the character data virtual CHIP_ERROR GetCommissionableDeviceName(MutableCharSpan & deviceNameSpan) = 0; virtual CHIP_ERROR GetInitialPairingHint(uint16_t & pairingHint) = 0; virtual CHIP_ERROR GetInitialPairingInstruction(char * buf, size_t bufSize) = 0; diff --git a/src/include/platform/DeviceInstanceInfoProvider.h b/src/include/platform/DeviceInstanceInfoProvider.h index 48cd65f43fab59..000f6184fd8206 100644 --- a/src/include/platform/DeviceInstanceInfoProvider.h +++ b/src/include/platform/DeviceInstanceInfoProvider.h @@ -32,7 +32,8 @@ class DeviceInstanceInfoProvider /** * @brief Obtain the Device Name from the device's factory data. * - * @param[out] deviceName Reference to location where the device name will be copied + * @param[out] deviceName Buffer to copy the name into. On success, will be + resized to the actual size of the name. * @returns CHIP_NO_ERROR on success, or another CHIP_ERROR from the underlying implementation * if access fails. */ diff --git a/src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp b/src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp index ea0cef7cef5f87..142736f28697d8 100644 --- a/src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp +++ b/src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp @@ -26,11 +26,7 @@ CHIP_ERROR GenericDeviceInstanceInfoProvider::GetDeviceName(Mutable { constexpr char deviceName[] = CHIP_DEVICE_CONFIG_DEVICE_NAME; - ReturnErrorCodeIf(sizeof(deviceName) - 1 > deviceNameSpan.size(), CHIP_ERROR_BUFFER_TOO_SMALL); - memcpy(deviceNameSpan.data(), deviceName, sizeof(deviceName) - 1); - deviceNameSpan.reduce_size(sizeof(deviceName) - 1); - - return CHIP_NO_ERROR; + return CopyCharSpanToMutableCharSpan(CharSpan::fromCharString(deviceName), deviceNameSpan); } template diff --git a/src/lib/dnssd/TxtFields.h b/src/lib/dnssd/TxtFields.h index fdd616d1d2b1dc..e308457f49c378 100644 --- a/src/lib/dnssd/TxtFields.h +++ b/src/lib/dnssd/TxtFields.h @@ -47,6 +47,10 @@ static constexpr size_t kKeyRotatingDeviceIdMaxLength = 100; static constexpr size_t kKeyPairingInstructionMaxLength = 128; static constexpr size_t kKeyPairingHintMaxLength = 10; +static_assert(kMaxDeviceNameLen == kKeyDeviceNameMaxLength, + "Max device name length constants are not matching: ConfigurationManager::kMaxDeviceNameLen and " + "Dnssd::kKeyDeviceNameMaxLength"); + enum class TxtKeyUse : uint8_t { kNone, diff --git a/src/platform/android/DeviceInstanceInfoProviderImpl.cpp b/src/platform/android/DeviceInstanceInfoProviderImpl.cpp index 264b767067c4d7..492bde80c40067 100644 --- a/src/platform/android/DeviceInstanceInfoProviderImpl.cpp +++ b/src/platform/android/DeviceInstanceInfoProviderImpl.cpp @@ -78,23 +78,19 @@ CHIP_ERROR DeviceInstanceInfoProviderImpl::GetDeviceName(MutableCharSpan & devic CHIP_ERROR err; size_t deviceNameSize = 0; char androidDeviceName[ConfigurationManager::kMaxDeviceNameLen]; - MutableCharSpan androidDeviceNameSpan(androidDeviceName); - err = Internal::AndroidConfig::ReadConfigValueStr(Internal::AndroidConfig::kConfigKey_MfrDeviceName, - androidDeviceNameSpan.data(), androidDeviceNameSpan.size(), deviceNameSize); + err = Internal::AndroidConfig::ReadConfigValueStr(Internal::AndroidConfig::kConfigKey_MfrDeviceName, androidDeviceName, + sizeof(androidDeviceName), deviceNameSize); if (err == CHIP_NO_ERROR) { ReturnErrorCodeIf(deviceNameSpan.size() < deviceNameSize, CHIP_ERROR_BUFFER_TOO_SMALL); - memcpy(deviceNameSpan.data(), androidDeviceNameSpan.data(), deviceNameSize); + memcpy(deviceNameSpan.data(), androidDeviceName, deviceNameSize); deviceNameSpan.reduce_size(deviceNameSize); } else if (err == CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND) { - ReturnErrorCodeIf(deviceNameSpan.size() < strlen(CHIP_DEVICE_CONFIG_DEVICE_NAME), CHIP_ERROR_BUFFER_TOO_SMALL); - memcpy(deviceNameSpan.data(), CHIP_DEVICE_CONFIG_DEVICE_NAME, strlen(CHIP_DEVICE_CONFIG_DEVICE_NAME)); - - deviceNameSpan.reduce_size(strlen(CHIP_DEVICE_CONFIG_DEVICE_NAME)); + return CopyCharSpanToMutableCharSpan(CharSpan::fromCharString(CHIP_DEVICE_CONFIG_DEVICE_NAME), deviceNameSpan); } return CHIP_NO_ERROR; } From bc98e7001d04dd1ff47bb7ea162becb5dceedbad Mon Sep 17 00:00:00 2001 From: ajoletta-amzn Date: Tue, 12 Sep 2023 15:37:39 -0700 Subject: [PATCH 6/6] Addressing comments from bzbarsky-apple. --- src/app/server/Dnssd.cpp | 4 ++++ src/lib/dnssd/TxtFields.h | 4 ---- src/platform/android/DeviceInstanceInfoProviderImpl.cpp | 5 +++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 41ae02ccae3473..0bfab04af1e761 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -44,6 +44,10 @@ namespace chip { namespace app { namespace { +static_assert(ConfigurationManager::kMaxDeviceNameLen == Dnssd::kKeyDeviceNameMaxLength, + "Max device name length constants are not matching: ConfigurationManager::kMaxDeviceNameLen and " + "Dnssd::kKeyDeviceNameMaxLength"); + void OnPlatformEvent(const DeviceLayer::ChipDeviceEvent * event) { switch (event->Type) diff --git a/src/lib/dnssd/TxtFields.h b/src/lib/dnssd/TxtFields.h index e308457f49c378..fdd616d1d2b1dc 100644 --- a/src/lib/dnssd/TxtFields.h +++ b/src/lib/dnssd/TxtFields.h @@ -47,10 +47,6 @@ static constexpr size_t kKeyRotatingDeviceIdMaxLength = 100; static constexpr size_t kKeyPairingInstructionMaxLength = 128; static constexpr size_t kKeyPairingHintMaxLength = 10; -static_assert(kMaxDeviceNameLen == kKeyDeviceNameMaxLength, - "Max device name length constants are not matching: ConfigurationManager::kMaxDeviceNameLen and " - "Dnssd::kKeyDeviceNameMaxLength"); - enum class TxtKeyUse : uint8_t { kNone, diff --git a/src/platform/android/DeviceInstanceInfoProviderImpl.cpp b/src/platform/android/DeviceInstanceInfoProviderImpl.cpp index 492bde80c40067..cb0d6c6482df48 100644 --- a/src/platform/android/DeviceInstanceInfoProviderImpl.cpp +++ b/src/platform/android/DeviceInstanceInfoProviderImpl.cpp @@ -87,12 +87,13 @@ CHIP_ERROR DeviceInstanceInfoProviderImpl::GetDeviceName(MutableCharSpan & devic ReturnErrorCodeIf(deviceNameSpan.size() < deviceNameSize, CHIP_ERROR_BUFFER_TOO_SMALL); memcpy(deviceNameSpan.data(), androidDeviceName, deviceNameSize); deviceNameSpan.reduce_size(deviceNameSize); + return CHIP_NO_ERROR; } - else if (err == CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND) + if (err == CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND) { return CopyCharSpanToMutableCharSpan(CharSpan::fromCharString(CHIP_DEVICE_CONFIG_DEVICE_NAME), deviceNameSpan); } - return CHIP_NO_ERROR; + return err; } CHIP_ERROR DeviceInstanceInfoProviderImpl::GetProductName(char * buf, size_t bufSize)