add add some basic information cluster attributes to cmd line args to the reference app #33303
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
chaitanya jandhyala seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
cecille
reviewed
May 3, 2024
src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp
Outdated
Show resolved
Hide resolved
src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp
Outdated
Show resolved
Hide resolved
bzbarsky-apple
requested changes
May 3, 2024
src/include/platform/internal/GenericConfigurationManagerImpl.h
Outdated
Show resolved
Hide resolved
src/include/platform/internal/GenericConfigurationManagerImpl.ipp
Outdated
Show resolved
Hide resolved
src/include/platform/internal/GenericConfigurationManagerImpl.ipp
Outdated
Show resolved
Hide resolved
|
PR #33303: Size comparison from f0ba374 to 736e622 Increases above 0.2%:
Increases (14 builds for linux)
Decreases (1 build for linux)
Full report (14 builds for linux)
|
src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp
Outdated
Show resolved
Hide resolved
src/include/platform/internal/GenericDeviceInstanceInfoProvider.ipp
Outdated
Show resolved
Hide resolved
cecille
reviewed
May 13, 2024
| if (options.vendorName.HasValue()) | ||
| { | ||
| chip::Span<const char> vendor_name(options.vendorName.Value().c_str(), options.vendorName.Value().size()); | ||
| VerifyOrDie(configManager.StoreVendorName(vendor_name) == CHIP_NO_ERROR); |
There was a problem hiding this comment.
With these verify or die calls, is the user going to be able to tell what went wrong? This can happen fairly easily, right? If the user enters a too-long name?
There was a problem hiding this comment.
verify or die, throws error if the user does not provide value to cmd line arg. if the user enters too long name, it doesn't throw error, but when we read from the basic information cluster, we get IM Error 0x00000501: General error: 0x01 (FAILURE). BTW its same API used for other cmd line args.
cjandhyala
force-pushed
the
serial_number_args_to_app
branch
from
a2fa434 to
feb127e
Compare
|
PR #33303: Size comparison from 47097e0 to feb127e Increases above 0.2%:
Full report (16 builds for linux)
|
cjandhyala
force-pushed
the
serial_number_args_to_app
branch
from
7c72d86 to
cfc477f
Compare
|
PR #33303: Size comparison from 1919112 to cfc477f Increases above 0.2%:
Full report (28 builds for efr32, linux, nxp, qpg, telink)
|
cjandhyala
force-pushed
the
serial_number_args_to_app
branch
from
cfc477f to
a37f7ac
Compare
|
PR #33303: Size comparison from 8786012 to a37f7ac Increases above 0.2%:
Full report (62 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, linux, nxp, qpg, stm32, telink)
|
|
PR #33303: Size comparison from 8786012 to 4c42a96 Increases above 0.2%:
Full report (96 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #33303: Size comparison from 8786012 to 44fa365 Increases above 0.2%:
Full report (96 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
arkq
force-pushed
the
serial_number_args_to_app
branch
from
e5dbc7c to
7760323
Compare
|
PR #33303: Size comparison from 74768a8 to 7760323 Increases above 0.2%:
Full report (96 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
… and software_version_string to read from config in reference linux and darwin platform
…e pointer/length arguments
github-actions
bot
added
github
workflows
gn
tests
icd
|
PR #33303: Size comparison from 24fd0d6 to c4c0e91 Full report (15 builds for cc13x4_26x4, cc32xx, nxp, qpg, stm32)
|
…nectedhomeip into serial_number_args_to_app
|
PR #33303: Size comparison from a7bbd7b to cea2b21 Increases above 0.2%:
Full report (80 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, linux, nrfconnect, nxp, qpg, stm32, telink, tizen)
|
… and software_version_string to read from config in reference linux and darwin platform
|
PR #33303: Size comparison from 7287041 to 08b7479 Increases above 0.2%:
Full report (90 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #33303: Size comparison from 7287041 to 84b9f51 Increases above 0.2%:
Full report (90 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #33303: Size comparison from 7287041 to bb8b3c6 Increases above 0.2%:
Full report (90 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Comment on lines
+367
to
+403
|
|
||
| VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_INTERNAL); | ||
|
|
||
| return err; | ||
| } | ||
|
|
||
| template <class ConfigClass> | ||
| CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSerialNumber(const char * serialNum, size_t serialNumLen) | ||
| CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSerialNumber(CharSpan serialNumber) | ||
| { | ||
| return WriteConfigValueStr(ConfigClass::kConfigKey_SerialNum, serialNumber.data(), serialNumber.size()); | ||
| } | ||
| template <class ConfigClass> | ||
| CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreVendorName(CharSpan vendorName) | ||
| { | ||
|
|
||
| return WriteConfigValueStr(ConfigClass::kConfigKey_VendorName, vendorName.data(), vendorName.size()); | ||
| } | ||
| template <class ConfigClass> | ||
| CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreProductName(CharSpan productName) | ||
| { | ||
| return WriteConfigValueStr(ConfigClass::kConfigKey_ProductName, productName.data(), productName.size()); | ||
| } | ||
| template <class ConfigClass> | ||
| CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreHardwareVersionString(CharSpan hardwareVersionString) | ||
| { | ||
| return WriteConfigValueStr(ConfigClass::kConfigKey_HardwareVersionString, hardwareVersionString.data(), | ||
| hardwareVersionString.size()); | ||
| } | ||
| template <class ConfigClass> | ||
| CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreSoftwareVersionString(CharSpan softwareVersionString) | ||
| { | ||
| return WriteConfigValueStr(ConfigClass::kConfigKey_SoftwareVersionString, softwareVersionString.data(), | ||
| softwareVersionString.size()); | ||
| } | ||
| template <class ConfigClass> | ||
| CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::StoreVendorName(const char * vendorName, size_t vendorNameLen) | ||
| { |
There was a problem hiding this comment.
Do not add this to the Generic configuration manager.
Overall, all the settings for this should com from overrides to DeviceInstanceInfoProvider on Linux only, and NOT by adding more ways to store data in here, which may clash with some external platforms, or require additional flash.
tcarmelveilleux
requested changes
Oct 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixed #33302