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.

Sign up for GitHub

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

Jump to bottom

Changing AcceptedCommands Interface #37646

Open
wants to merge 40 commits into
base: master
Choose a base branch
from
Open

Changing AcceptedCommands Interface #37646

wants to merge 40 commits into from

Conversation

ratgr
Copy link
Contributor

@ratgr ratgr commented Feb 19, 2025 •
edited
Loading

Fixes #37139

  • Currently it changes only the CHI::EnumerateAcceptedCommands Interface
  • Make GeneratedCommands compatible with AcceptedCommands
  • Update all example apps using it

Cluster Changes:

Network Comissioning

Spec/NetworkCommissioning#Commands
image
NetworkCommissioning::EnumerateAcceptedCommands

{ ScanNetworks::Id, {}, Priv::kAdminister },   //
{ RemoveNetwork::Id, {}, Priv::kAdminister },  //
{ ConnectNetwork::Id, {}, Priv::kAdminister }, //
{ ReorderNetwork::Id, {}, Priv::kAdminister }, //
{ AddOrUpdateThreadNetwork::Id | AddOrUpdateWiFiNetwork::Id, {}, Priv::kAdminister }
{ QueryIdentity::Id, {}, Priv::kAdminister },

Device Energy Management

Spec/DeviceEnergyManagement#Commands
image
DeviceEnergyManagement::EnumerateAcceptedCommands

{ PowerAdjustRequest::Id,             {}, Priv::kOperate },      //
{ CancelPowerAdjustRequest::Id,       {}, Priv::kOperate } //
{ StartTimeAdjustRequest::Id,         {}, Priv::kOperate }
{ PauseRequest::Id,                   {}, Priv::kOperate }, //
{ ResumeRequest::Id,                  {}, Priv::kOperate } //
{ ModifyForecastRequest::Id,          {}, Priv::kOperate }
{ RequestConstraintBasedForecast::Id, {}, Priv::kOperate }
{ CancelRequest::Id,                  {}, Priv::kOperate }

Energy Evse

Spec/EnergyEvse#Commands
image
EnergyEvse::EnumerateAcceptedCommands

{ Disable::Id,           QF::kTimed, Priv::kOperate },        
{ EnableCharging::Id,    QF::kTimed, Priv::kOperate }, 
{ EnableDischarging::Id, QF::kTimed, Priv::kOperate }
{ StartDiagnostics::Id,  QF::kTimed, Priv::kOperate }
{ SetTargets::Id,        QF::kTimed, Priv::kOperate },   //
{ GetTargets::Id,        QF::kTimed, Priv::kOperate },   //
{ ClearTargets::Id,      QF::kTimed, Priv::kOperate }, //

Resource Monitoring

Spec/ResourceMonitoring#Commands
image
ResourceMonitoring::EnumerateAcceptedCommands

{ ResetCondition::Id, {}, Priv::kOperate }

Spec/SoftwareDiagnostics#Commands
image
SoftwareDiagnostics::EnumerateAcceptedCommands

{ResetWatermarks::Id, {}, Privilege::kManage }

Testing

Manually tested

@semanticdiff-com SemanticDiff.com
Copy link

semanticdiff-com bot commented Feb 19, 2025 •
edited
Loading

Review changes with  SemanticDiff

khodya
khodya reviewed Feb 19, 2025
View reviewed changes
@ratgr ratgr marked this pull request as draft February 23, 2025 22:45
@ratgr ratgr force-pushed the command-handler-interface-full-data branch from 8f1e074 to 601f2cc Compare February 24, 2025 02:03
@ratgr ratgr force-pushed the command-handler-interface-full-data branch from 601f2cc to 3a551c1 Compare February 24, 2025 02:06
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 24, 2025 •
edited
Loading

PR #37646: Size comparison from 43a8a9b to 3a551c1

Increases above 0.2%:

platform target config section 43a8a9b 3a551c1 change % change
bl702 lighting-app bl702+wifi RAM 22233 22305 72 0.3
bl702l contact-sensor-app bl702l+mfd+littlefs RAM 26896 26976 80 0.3
lighting-app bl702l+mfd+littlefs RAM 24644 24716 72 0.3
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
platform target config section 43a8a9b 3a551c1 change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1095140 1094842 -298 -0.0
RAM 94802 94890 88 0.1
bl702 lighting-app bl702+eth FLASH 651586 651346 -240 -0.0
RAM 33509 33509 0 0.0
bl702+wifi FLASH 827750 827458 -292 -0.0
RAM 22233 22305 72 0.3
bl706+mfd+rpc+littlefs FLASH 1060912 1060618 -294 -0.0
RAM 32157 32229 72 0.2
bl702l contact-sensor-app bl702l+mfd+littlefs FLASH 892098 891806 -292 -0.0
RAM 26896 26976 80 0.3
lighting-app bl702l+mfd+littlefs FLASH 974994 974702 -292 -0.0
RAM 24644 24716 72 0.3
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 815268 815004 -264 -0.0
RAM 120256 120336 80 0.1
lock-ftd LP_EM_CC1354P10_6 FLASH 824180 823908 -272 -0.0
RAM 125352 125424 72 0.1
pump-app LP_EM_CC1354P10_6 FLASH 771048 770752 -296 -0.0
RAM 113724 113804 80 0.1
pump-controller-app LP_EM_CC1354P10_6 FLASH 755324 755020 -304 -0.0
RAM 113932 114012 80 0.1
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 538810 538586 -224 -0.0
RAM 205112 205112 0 0.0
lock CC3235SF_LAUNCHXL FLASH 572966 572742 -224 -0.0
RAM 205360 205360 0 0.0
cyw30739 light CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 656397 656133 -264 -0.0
RAM 75324 75404 80 0.1
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 676257 675985 -272 -0.0
RAM 77964 78044 80 0.1
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 676257 675985 -272 -0.0
RAM 77964 78044 80 0.1
CYW930739M2EVB-02 unknown 2040 2040 0 0.0
FLASH 633181 632925 -256 -0.0
RAM 70392 70472 80 0.1
light-switch CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 616261 615997 -264 -0.0
RAM 71532 71612 80 0.1
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 635897 635633 -264 -0.0
RAM 74076 74156 80 0.1
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 635897 635633 -264 -0.0
RAM 74076 74156 80 0.1
lock CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 635757 635485 -272 -0.0
RAM 74540 74612 72 0.1
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 655473 655209 -264 -0.0
RAM 77084 77156 72 0.1
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 655473 655209 -264 -0.0
RAM 77084 77156 72 0.1
thermostat CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 612105 611833 -272 -0.0
RAM 68628 68700 72 0.1
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 631965 631693 -272 -0.0
RAM 71268 71340 72 0.1
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 631965 631693 -272 -0.0
RAM 71268 71340 72 0.1
efr32 lock-app BRD4187C FLASH 939152 938904 -248 -0.0
RAM 159920 160024 104 0.1
BRD4338a FLASH 732240 731960 -280 -0.0
RAM 234836 234904 68 0.0
window-app BRD4187C FLASH 1032000 1031720 -280 -0.0
RAM 128024 128096 72 0.1
esp32 all-clusters-app c3devkit DRAM 97312 97400 88 0.1
FLASH 1581932 1581824 -108 -0.0
IRAM 83820 83820 0 0.0
m5stack DRAM 116100 116188 88 0.1
FLASH 1549958 1549774 -184 -0.0
IRAM 117039 117039 0 0.0
linux air-purifier-app debug unknown 4720 4720 0 0.0
FLASH 2649711 2647659 -2052 -0.1
RAM 111088 111184 96 0.1
all-clusters-app debug unknown 5528 5528 0 0.0
FLASH 5921216 5920060 -1156 -0.0
RAM 513712 513808 96 0.0
all-clusters-minimal-app debug unknown 5424 5424 0 0.0
FLASH 5266218 5264134 -2084 -0.0
RAM 221112 221208 96 0.0
bridge-app debug unknown 5440 5440 0 0.0
FLASH 4624892 4622810 -2082 -0.0
RAM 199816 199880 64 0.0
camera-app debug unknown 5424 5424 0 0.0
FLASH 4671672 4669590 -2082 -0.0
RAM 194592 194688 96 0.0
chip-tool debug unknown 6096 6096 0 0.0
FLASH 13301813 13299491 -2322 -0.0
RAM 603392 603392 0 0.0
chip-tool-ipv6only arm64 unknown 21976 21976 0 0.0
FLASH 11322688 11320800 -1888 -0.0
RAM 655184 655184 0 0.0
fabric-admin debug unknown 5784 5784 0 0.0
FLASH 11565129 11562807 -2322 -0.0
RAM 603176 603176 0 0.0
fabric-bridge-app debug unknown 4688 4688 0 0.0
FLASH 4450216 4448100 -2116 -0.0
RAM 187016 187112 96 0.1
fabric-sync debug unknown 4944 4944 0 0.0
FLASH 5569029 5566949 -2080 -0.0
RAM 470400 470496 96 0.0
lighting-app debug+rpc+ui unknown 6152 6152 0 0.0
FLASH 5514305 5512193 -2112 -0.0
RAM 203952 204016 64 0.0
lock-app debug unknown 5392 5392 0 0.0
FLASH 4688328 4686246 -2082 -0.0
RAM 191144 191240 96 0.1
ota-provider-app debug unknown 4728 4728 0 0.0
FLASH 4310508 4308400 -2108 -0.0
RAM 179832 179928 96 0.1
ota-requestor-app debug unknown 4680 4680 0 0.0
FLASH 4440666 4438526 -2140 -0.0
RAM 184320 184416 96 0.1
shell debug unknown 4216 4216 0 0.0
FLASH 2949020 2946716 -2304 -0.1
RAM 143672 143672 0 0.0
thermostat-no-ble arm64 unknown 9448 9448 0 0.0
FLASH 4042232 4040368 -1864 -0.0
RAM 228096 228176 80 0.0
tv-app debug unknown 5720 5720 0 0.0
FLASH 5907941 5905925 -2016 -0.0
RAM 593832 593896 64 0.0
tv-casting-app debug unknown 5296 5296 0 0.0
FLASH 11472621 11470589 -2032 -0.0
RAM 718656 718752 96 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 907676 907728 52 0.0
RAM 142323 142327 4 0.0
nrf7002dk_nrf5340_cpuapp FLASH 901752 901888 136 0.0
RAM 124663 124667 4 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 846280 846020 -260 -0.0
RAM 141251 141255 4 0.0
nxp contact k32w0+release FLASH 585184 584920 -264 -0.0
RAM 70876 70948 72 0.1
mcxw71+release FLASH 600640 600368 -272 -0.0
RAM 63096 63168 72 0.1
light k32w0+release FLASH 611156 610892 -264 -0.0
RAM 70164 70244 80 0.1
k32w1+release FLASH 685640 685384 -256 -0.0
RAM 48584 48664 80 0.2
lock mcxw71+release FLASH 749480 749216 -264 -0.0
RAM 67500 67572 72 0.1
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1646172 1646260 88 0.0
RAM 211472 211544 72 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1552940 1552660 -280 -0.0
RAM 208288 208360 72 0.0
light cy8ckit_062s2_43012 FLASH 1438612 1438332 -280 -0.0
RAM 197040 197120 80 0.0
lock cy8ckit_062s2_43012 FLASH 1467500 1467220 -280 -0.0
RAM 224704 224776 72 0.0
qpg lighting-app qpg6105+debug FLASH 662156 661876 -280 -0.0
RAM 105116 105196 80 0.1
lock-app qpg6105+debug FLASH 620264 619984 -280 -0.0
RAM 99664 99744 80 0.1
stm32 light STM32WB5MM-DK FLASH 459800 459528 -272 -0.1
RAM 141472 141552 80 0.1
telink bridge-app tl7218x FLASH 665014 664750 -264 -0.0
RAM 90728 90736 8 0.0
contact-sensor-app tlsr9528a_retention FLASH 622058 621786 -272 -0.0
RAM 31488 31496 8 0.0
light-app-ota-shell-factory-data tl3218x FLASH 745198 744934 -264 -0.0
RAM 40396 40404 8 0.0
tl7218x FLASH 753760 753496 -264 -0.0
RAM 97540 97548 8 0.0
light-switch-app-ota-compress-lzma-factory-data tl7218x_retention FLASH 680830 680566 -264 -0.0
RAM 52192 52200 8 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 709392 709128 -264 -0.0
RAM 73400 73408 8 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 600572 600308 -264 -0.0
RAM 138812 138820 8 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 788714 788450 -264 -0.0
RAM 96388 96396 8 0.0
tizen all-clusters-app arm unknown 5076 5076 0 0.0
FLASH 1750772 1750092 -680 -0.0
RAM 93260 93340 80 0.1
chip-tool-ubsan arm unknown 11492 11492 0 0.0
FLASH 18982830 18979854 -2976 -0.0
RAM 8305976 8304840 -1136 -0.0

andy31415
andy31415 previously requested changes Feb 24, 2025
View reviewed changes
Copy link
Contributor

@andy31415 andy31415 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes: the moving of the files and adding of admin_storage is suspicious. please at a minimum:

  • do not add admin_storage.json to the PR
  • do not move files from data-model-provider into data-model. They are not the same thing. If you have dependency/compile issues you probably need to fix dependencies in chip_data_model.gni or similar.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Feb 26, 2025
@github-actions GitHub Actions
Copy link

PR #37646: Size comparison from 54afa4a to 3037bfc

Full report (1 build for stm32)
platform target config section 54afa4a 3037bfc change % change
stm32 light STM32WB5MM-DK FLASH 459840 459632 -208 -0.0
RAM 141472 141472 0 0.0

andy31415
andy31415 reviewed Feb 26, 2025
View reviewed changes
@ratgr @andy31415
Update docs/upgrading.md
7f56e9f 
Co-authored-by: Andrei Litvin <andy314@gmail.com>
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 26, 2025 •
edited
Loading

PR #37646: Size comparison from 54afa4a to 7f56e9f

Full report (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nxp, psoc6, qpg, stm32, telink, tizen)
platform target config section 54afa4a 7f56e9f change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1096892 1096636 -256 -0.0
RAM 94842 94850 8 0.0
bl702 lighting-app bl702+eth FLASH 651870 651614 -256 -0.0
RAM 33509 33509 0 0.0
bl702+wifi FLASH 829142 828882 -260 -0.0
RAM 22233 22233 0 0.0
bl706+mfd+rpc+littlefs FLASH 1061538 1061278 -260 -0.0
RAM 32157 32157 0 0.0
bl702l contact-sensor-app bl702l+mfd+littlefs FLASH 892382 892122 -260 -0.0
RAM 26896 26904 8 0.0
lighting-app bl702l+mfd+littlefs FLASH 975278 975018 -260 -0.0
RAM 24644 24644 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 817152 816944 -208 -0.0
RAM 120272 120272 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 826072 825880 -192 -0.0
RAM 125368 125368 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 772956 772724 -232 -0.0
RAM 113740 113740 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 757240 756992 -248 -0.0
RAM 113948 113948 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 540646 540406 -240 -0.0
RAM 205128 205128 0 0.0
lock CC3235SF_LAUNCHXL FLASH 574794 574554 -240 -0.0
RAM 205376 205376 0 0.0
cyw30739 light CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 658861 658653 -208 -0.0
RAM 75412 75412 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 678713 678505 -208 -0.0
RAM 78052 78052 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 678713 678505 -208 -0.0
RAM 78052 78052 0 0.0
CYW930739M2EVB-02 unknown 2040 2040 0 0.0
FLASH 635645 635445 -200 -0.0
RAM 70480 70480 0 0.0
light-switch CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 619101 618893 -208 -0.0
RAM 71652 71652 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 638737 638529 -208 -0.0
RAM 74196 74196 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 638737 638529 -208 -0.0
RAM 74196 74196 0 0.0
lock CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 638589 638381 -208 -0.0
RAM 74660 74660 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 658305 658097 -208 -0.0
RAM 77204 77204 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 658305 658097 -208 -0.0
RAM 77204 77204 0 0.0
thermostat CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 614937 614721 -216 -0.0
RAM 68748 68748 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 634789 634573 -216 -0.0
RAM 71388 71388 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 634789 634573 -216 -0.0
RAM 71388 71388 0 0.0
efr32 lock-app BRD4187C FLASH 939672 939472 -200 -0.0
RAM 159920 159920 0 0.0
BRD4338a FLASH 732656 732424 -232 -0.0
RAM 234828 234828 0 0.0
window-app BRD4187C FLASH 1032104 1031872 -232 -0.0
RAM 128024 128024 0 0.0
esp32 all-clusters-app c3devkit DRAM 98656 98656 0 0.0
FLASH 1589616 1589618 2 0.0
IRAM 83820 83820 0 0.0
m5stack DRAM 117436 117436 0 0.0
FLASH 1556626 1556762 136 0.0
IRAM 117039 117039 0 0.0
linux air-purifier-app debug unknown 4728 4728 0 0.0
FLASH 2650809 2649225 -1584 -0.1
RAM 111088 111088 0 0.0
all-clusters-app debug unknown 5536 5536 0 0.0
FLASH 5962582 5962532 -50 -0.0
RAM 514832 514832 0 0.0
all-clusters-minimal-app debug unknown 5432 5432 0 0.0
FLASH 5297294 5295676 -1618 -0.0
RAM 221272 221272 0 0.0
bridge-app debug unknown 5448 5448 0 0.0
FLASH 4649642 4648026 -1616 -0.0
RAM 200144 200144 0 0.0
camera-app debug unknown 5432 5432 0 0.0
FLASH 4672770 4671154 -1616 -0.0
RAM 194592 194592 0 0.0
chip-tool debug unknown 6096 6096 0 0.0
FLASH 13303265 13300949 -2316 -0.0
RAM 603392 603392 0 0.0
chip-tool-ipv6only arm64 unknown 21976 21976 0 0.0
FLASH 11496136 11494248 -1888 -0.0
RAM 656112 656112 0 0.0
fabric-admin debug unknown 5784 5784 0 0.0
FLASH 11568059 11565743 -2316 -0.0
RAM 603176 603176 0 0.0
fabric-bridge-app debug unknown 4696 4696 0 0.0
FLASH 4453208 4451590 -1618 -0.0
RAM 187016 187016 0 0.0
fabric-sync debug unknown 4952 4952 0 0.0
FLASH 5570197 5568581 -1616 -0.0
RAM 470400 470400 0 0.0
lighting-app debug+rpc+ui unknown 6160 6160 0 0.0
FLASH 5516481 5514865 -1616 -0.0
RAM 203952 203952 0 0.0
lock-app debug unknown 5400 5400 0 0.0
FLASH 4689458 4687842 -1616 -0.0
RAM 191144 191144 0 0.0
ota-provider-app debug unknown 4736 4736 0 0.0
FLASH 4311670 4309954 -1716 -0.0
RAM 179832 179832 0 0.0
ota-requestor-app debug unknown 4688 4688 0 0.0
FLASH 4441990 4440242 -1748 -0.0
RAM 184320 184320 0 0.0
shell debug unknown 4216 4216 0 0.0
FLASH 2979724 2977532 -2192 -0.1
RAM 144344 144344 0 0.0
thermostat-no-ble arm64 unknown 9448 9448 0 0.0
FLASH 4139160 4137576 -1584 -0.0
RAM 229016 229016 0 0.0
tv-app debug unknown 5728 5728 0 0.0
FLASH 5909045 5907477 -1568 -0.0
RAM 593832 593832 0 0.0
tv-casting-app debug unknown 5304 5304 0 0.0
FLASH 11473757 11472189 -1568 -0.0
RAM 718656 718656 0 0.0
nxp contact k32w0+release FLASH 587368 587152 -216 -0.0
RAM 70980 70980 0 0.0
mcxw71+release FLASH 601192 600992 -200 -0.0
RAM 63096 63096 0 0.0
light k32w0+release FLASH 613100 612884 -216 -0.0
RAM 70268 70268 0 0.0
k32w1+release FLASH 685824 685608 -216 -0.0
RAM 48584 48584 0 0.0
lock mcxw71+release FLASH 750032 749816 -216 -0.0
RAM 67500 67500 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1655684 1655916 232 0.0
RAM 212264 212264 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1562380 1562132 -248 -0.0
RAM 208560 208560 0 0.0
light cy8ckit_062s2_43012 FLASH 1441180 1440948 -232 -0.0
RAM 197296 197296 0 0.0
lock cy8ckit_062s2_43012 FLASH 1470068 1469836 -232 -0.0
RAM 224960 224960 0 0.0
qpg lighting-app qpg6105+debug FLASH 663772 663556 -216 -0.0
RAM 105156 105156 0 0.0
lock-app qpg6105+debug FLASH 622240 622024 -216 -0.0
RAM 99768 99768 0 0.0
stm32 light STM32WB5MM-DK FLASH 459840 459632 -208 -0.0
RAM 141472 141472 0 0.0
telink bridge-app tl7218x FLASH 669192 668896 -296 -0.0
RAM 90752 90752 0 0.0
contact-sensor-app tlsr9528a_retention FLASH 622078 621782 -296 -0.0
RAM 31488 31488 0 0.0
light-app-ota-shell-factory-data tl3218x FLASH 745386 745090 -296 -0.0
RAM 40396 40396 0 0.0
tl7218x FLASH 753948 753652 -296 -0.0
RAM 97540 97540 0 0.0
light-switch-app-ota-compress-lzma-factory-data tl7218x_retention FLASH 681018 680722 -296 -0.0
RAM 52192 52192 0 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 709580 709284 -296 -0.0
RAM 73400 73400 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 600760 600464 -296 -0.0
RAM 138812 138812 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 788988 788692 -296 -0.0
RAM 96388 96388 0 0.0
tizen all-clusters-app arm unknown 5116 5116 0 0.0
FLASH 1766136 1765776 -360 -0.0
RAM 93844 93844 0 0.0
chip-tool-ubsan arm unknown 11492 11492 0 0.0
FLASH 18983958 18981110 -2848 -0.0
RAM 8306328 8305216 -1112 -0.0

andy31415
andy31415 approved these changes Feb 26, 2025
View reviewed changes
Copy link
Contributor

@andy31415 andy31415 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approviding assuming remaining comments addressed:

  • please double-check all command flags with the spec. Maybe add a summary in the PR for all replaced so we make sure they are ok. These are tedious to check :(

  • we may choose to appendelements everywhere even for 1-element lists as that is less clunky than ensuresize+append. We can add a tech debt issue to have an append that auto-reallocates (not having that was on purpose since I thought it would encourage people to static allocate more ... but unsure if that is effective)

@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 26, 2025 •
edited
Loading

PR #37646: Size comparison from 54afa4a to afc80b1

Full report (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nxp, psoc6, qpg, stm32, telink, tizen)
platform target config section 54afa4a afc80b1 change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1096892 1096636 -256 -0.0
RAM 94842 94850 8 0.0
bl702 lighting-app bl702+eth FLASH 651870 651614 -256 -0.0
RAM 33509 33509 0 0.0
bl702+wifi FLASH 829142 828882 -260 -0.0
RAM 22233 22233 0 0.0
bl706+mfd+rpc+littlefs FLASH 1061538 1061278 -260 -0.0
RAM 32157 32157 0 0.0
bl702l contact-sensor-app bl702l+mfd+littlefs FLASH 892382 892122 -260 -0.0
RAM 26896 26904 8 0.0
lighting-app bl702l+mfd+littlefs FLASH 975278 975018 -260 -0.0
RAM 24644 24644 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 817152 816944 -208 -0.0
RAM 120272 120272 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 826072 825880 -192 -0.0
RAM 125368 125368 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 772956 772724 -232 -0.0
RAM 113740 113740 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 757240 756992 -248 -0.0
RAM 113948 113948 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 540646 540406 -240 -0.0
RAM 205128 205128 0 0.0
lock CC3235SF_LAUNCHXL FLASH 574794 574554 -240 -0.0
RAM 205376 205376 0 0.0
cyw30739 light CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 658861 658653 -208 -0.0
RAM 75412 75412 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 678713 678505 -208 -0.0
RAM 78052 78052 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 678713 678505 -208 -0.0
RAM 78052 78052 0 0.0
CYW930739M2EVB-02 unknown 2040 2040 0 0.0
FLASH 635645 635445 -200 -0.0
RAM 70480 70480 0 0.0
light-switch CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 619101 618893 -208 -0.0
RAM 71652 71652 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 638737 638529 -208 -0.0
RAM 74196 74196 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 638737 638529 -208 -0.0
RAM 74196 74196 0 0.0
lock CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 638589 638381 -208 -0.0
RAM 74660 74660 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 658305 658097 -208 -0.0
RAM 77204 77204 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 658305 658097 -208 -0.0
RAM 77204 77204 0 0.0
thermostat CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 614937 614721 -216 -0.0
RAM 68748 68748 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 634789 634573 -216 -0.0
RAM 71388 71388 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 634789 634573 -216 -0.0
RAM 71388 71388 0 0.0
efr32 lock-app BRD4187C FLASH 939672 939472 -200 -0.0
RAM 159920 159920 0 0.0
BRD4338a FLASH 732656 732424 -232 -0.0
RAM 234828 234828 0 0.0
window-app BRD4187C FLASH 1032104 1031872 -232 -0.0
RAM 128024 128024 0 0.0
esp32 all-clusters-app c3devkit DRAM 98656 98656 0 0.0
FLASH 1589616 1589620 4 0.0
IRAM 83820 83820 0 0.0
m5stack DRAM 117436 117436 0 0.0
FLASH 1556626 1556766 140 0.0
IRAM 117039 117039 0 0.0
linux air-purifier-app debug unknown 4728 4728 0 0.0
FLASH 2650809 2649305 -1504 -0.1
RAM 111088 111088 0 0.0
all-clusters-app debug unknown 5536 5536 0 0.0
FLASH 5962582 5962612 30 0.0
RAM 514832 514832 0 0.0
all-clusters-minimal-app debug unknown 5432 5432 0 0.0
FLASH 5297294 5295756 -1538 -0.0
RAM 221272 221272 0 0.0
bridge-app debug unknown 5448 5448 0 0.0
FLASH 4649642 4648106 -1536 -0.0
RAM 200144 200144 0 0.0
camera-app debug unknown 5432 5432 0 0.0
FLASH 4672770 4671234 -1536 -0.0
RAM 194592 194592 0 0.0
chip-tool debug unknown 6096 6096 0 0.0
FLASH 13303265 13300949 -2316 -0.0
RAM 603392 603392 0 0.0
chip-tool-ipv6only arm64 unknown 21976 21976 0 0.0
FLASH 11496136 11494248 -1888 -0.0
RAM 656112 656112 0 0.0
fabric-admin debug unknown 5784 5784 0 0.0
FLASH 11568059 11565743 -2316 -0.0
RAM 603176 603176 0 0.0
fabric-bridge-app debug unknown 4696 4696 0 0.0
FLASH 4453208 4451670 -1538 -0.0
RAM 187016 187016 0 0.0
fabric-sync debug unknown 4952 4952 0 0.0
FLASH 5570197 5568661 -1536 -0.0
RAM 470400 470400 0 0.0
lighting-app debug+rpc+ui unknown 6160 6160 0 0.0
FLASH 5516481 5514945 -1536 -0.0
RAM 203952 203952 0 0.0
lock-app debug unknown 5400 5400 0 0.0
FLASH 4689458 4687922 -1536 -0.0
RAM 191144 191144 0 0.0
ota-provider-app debug unknown 4736 4736 0 0.0
FLASH 4311670 4310034 -1636 -0.0
RAM 179832 179832 0 0.0
ota-requestor-app debug unknown 4688 4688 0 0.0
FLASH 4441990 4440322 -1668 -0.0
RAM 184320 184320 0 0.0
shell debug unknown 4216 4216 0 0.0
FLASH 2979724 2977532 -2192 -0.1
RAM 144344 144344 0 0.0
thermostat-no-ble arm64 unknown 9448 9448 0 0.0
FLASH 4139160 4137576 -1584 -0.0
RAM 229016 229016 0 0.0
tv-app debug unknown 5728 5728 0 0.0
FLASH 5909045 5907557 -1488 -0.0
RAM 593832 593832 0 0.0
tv-casting-app debug unknown 5304 5304 0 0.0
FLASH 11473757 11472269 -1488 -0.0
RAM 718656 718656 0 0.0
nxp contact k32w0+release FLASH 587368 587152 -216 -0.0
RAM 70980 70980 0 0.0
mcxw71+release FLASH 601192 600992 -200 -0.0
RAM 63096 63096 0 0.0
light k32w0+release FLASH 613100 612884 -216 -0.0
RAM 70268 70268 0 0.0
k32w1+release FLASH 685824 685608 -216 -0.0
RAM 48584 48584 0 0.0
lock mcxw71+release FLASH 750032 749816 -216 -0.0
RAM 67500 67500 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1655684 1655916 232 0.0
RAM 212264 212264 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1562380 1562132 -248 -0.0
RAM 208560 208560 0 0.0
light cy8ckit_062s2_43012 FLASH 1441180 1440948 -232 -0.0
RAM 197296 197296 0 0.0
lock cy8ckit_062s2_43012 FLASH 1470068 1469836 -232 -0.0
RAM 224960 224960 0 0.0
qpg lighting-app qpg6105+debug FLASH 663772 663556 -216 -0.0
RAM 105156 105156 0 0.0
lock-app qpg6105+debug FLASH 622240 622024 -216 -0.0
RAM 99768 99768 0 0.0
stm32 light STM32WB5MM-DK FLASH 459840 459632 -208 -0.0
RAM 141472 141472 0 0.0
telink bridge-app tl7218x FLASH 669192 668898 -294 -0.0
RAM 90752 90752 0 0.0
contact-sensor-app tlsr9528a_retention FLASH 622078 621784 -294 -0.0
RAM 31488 31488 0 0.0
light-app-ota-shell-factory-data tl3218x FLASH 745386 745092 -294 -0.0
RAM 40396 40396 0 0.0
tl7218x FLASH 753948 753654 -294 -0.0
RAM 97540 97540 0 0.0
light-switch-app-ota-compress-lzma-factory-data tl7218x_retention FLASH 681018 680724 -294 -0.0
RAM 52192 52192 0 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 709580 709286 -294 -0.0
RAM 73400 73400 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 600760 600466 -294 -0.0
RAM 138812 138812 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 788988 788694 -294 -0.0
RAM 96388 96388 0 0.0
tizen all-clusters-app arm unknown 5116 5116 0 0.0
FLASH 1766136 1765800 -336 -0.0
RAM 93844 93844 0 0.0
chip-tool-ubsan arm unknown 11492 11492 0 0.0
FLASH 18983958 18981110 -2848 -0.0
RAM 8306328 8305216 -1112 -0.0

@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 27, 2025 •
edited
Loading

PR #37646: Size comparison from 54afa4a to 8c10c16

Increases above 0.2%:

platform target config section 54afa4a 8c10c16 change % change
linux air-purifier-app debug unknown 4728 4752 24 0.5
RAM 111088 112272 1184 1.1
all-clusters-app debug unknown 5536 5560 24 0.4
RAM 514832 516016 1184 0.2
all-clusters-minimal-app debug unknown 5432 5456 24 0.4
RAM 221272 222456 1184 0.5
bridge-app debug unknown 5448 5472 24 0.4
RAM 200144 201328 1184 0.6
camera-app debug unknown 5432 5456 24 0.4
RAM 194592 195776 1184 0.6
chip-tool debug unknown 6096 6112 16 0.3
fabric-admin debug unknown 5784 5800 16 0.3
fabric-bridge-app debug unknown 4696 4720 24 0.5
RAM 187016 188168 1152 0.6
fabric-sync debug unknown 4952 4976 24 0.5
RAM 470400 471584 1184 0.3
lighting-app debug+rpc+ui unknown 6160 6184 24 0.4
RAM 203952 205136 1184 0.6
lock-app debug unknown 5400 5424 24 0.4
RAM 191144 192328 1184 0.6
ota-provider-app debug unknown 4736 4760 24 0.5
RAM 179832 180984 1152 0.6
ota-requestor-app debug unknown 4688 4712 24 0.5
RAM 184320 185472 1152 0.6
shell debug unknown 4216 4240 24 0.6
RAM 144344 145528 1184 0.8
thermostat-no-ble arm64 RAM 229016 229832 816 0.4
tv-app debug unknown 5728 5752 24 0.4
tv-casting-app debug unknown 5304 5320 16 0.3
Full report (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nxp, psoc6, qpg, stm32, telink, tizen)
platform target config section 54afa4a 8c10c16 change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1096892 1096636 -256 -0.0
RAM 94842 94850 8 0.0
bl702 lighting-app bl702+eth FLASH 651870 651614 -256 -0.0
RAM 33509 33509 0 0.0
bl702+wifi FLASH 829142 828882 -260 -0.0
RAM 22233 22233 0 0.0
bl706+mfd+rpc+littlefs FLASH 1061538 1061278 -260 -0.0
RAM 32157 32157 0 0.0
bl702l contact-sensor-app bl702l+mfd+littlefs FLASH 892382 892122 -260 -0.0
RAM 26896 26904 8 0.0
lighting-app bl702l+mfd+littlefs FLASH 975278 975018 -260 -0.0
RAM 24644 24644 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 817152 816960 -192 -0.0
RAM 120272 120272 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 826072 825880 -192 -0.0
RAM 125368 125368 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 772956 772724 -232 -0.0
RAM 113740 113740 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 757240 757008 -232 -0.0
RAM 113948 113948 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 540646 540406 -240 -0.0
RAM 205128 205128 0 0.0
lock CC3235SF_LAUNCHXL FLASH 574794 574554 -240 -0.0
RAM 205376 205376 0 0.0
cyw30739 light CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 658861 658653 -208 -0.0
RAM 75412 75412 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 678713 678505 -208 -0.0
RAM 78052 78052 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 678713 678505 -208 -0.0
RAM 78052 78052 0 0.0
CYW930739M2EVB-02 unknown 2040 2040 0 0.0
FLASH 635645 635445 -200 -0.0
RAM 70480 70480 0 0.0
light-switch CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 619101 618901 -200 -0.0
RAM 71652 71652 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 638737 638537 -200 -0.0
RAM 74196 74196 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 638737 638537 -200 -0.0
RAM 74196 74196 0 0.0
lock CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 638589 638381 -208 -0.0
RAM 74660 74660 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 658305 658097 -208 -0.0
RAM 77204 77204 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 658305 658097 -208 -0.0
RAM 77204 77204 0 0.0
thermostat CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 614937 614729 -208 -0.0
RAM 68748 68748 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 634789 634581 -208 -0.0
RAM 71388 71388 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 634789 634581 -208 -0.0
RAM 71388 71388 0 0.0
efr32 lock-app BRD4187C FLASH 939672 939472 -200 -0.0
RAM 159920 159920 0 0.0
BRD4338a FLASH 732656 732424 -232 -0.0
RAM 234828 234828 0 0.0
window-app BRD4187C FLASH 1032104 1031872 -232 -0.0
RAM 128024 128024 0 0.0
esp32 all-clusters-app c3devkit DRAM 98656 98656 0 0.0
FLASH 1589616 1589582 -34 -0.0
IRAM 83820 83820 0 0.0
m5stack DRAM 117436 117436 0 0.0
FLASH 1556626 1556702 76 0.0
IRAM 117039 117039 0 0.0
linux air-purifier-app debug unknown 4728 4752 24 0.5
FLASH 2650809 2651729 920 0.0
RAM 111088 112272 1184 1.1
all-clusters-app debug unknown 5536 5560 24 0.4
FLASH 5962582 5964624 2042 0.0
RAM 514832 516016 1184 0.2
all-clusters-minimal-app debug unknown 5432 5456 24 0.4
FLASH 5297294 5298180 886 0.0
RAM 221272 222456 1184 0.5
bridge-app debug unknown 5448 5472 24 0.4
FLASH 4649642 4650530 888 0.0
RAM 200144 201328 1184 0.6
camera-app debug unknown 5432 5456 24 0.4
FLASH 4672770 4673658 888 0.0
RAM 194592 195776 1184 0.6
chip-tool debug unknown 6096 6112 16 0.3
FLASH 13303265 13302635 -630 -0.0
RAM 603392 603424 32 0.0
chip-tool-ipv6only arm64 unknown 21976 21976 0 0.0
FLASH 11496136 11495992 -144 -0.0
RAM 656112 656128 16 0.0
fabric-admin debug unknown 5784 5800 16 0.3
FLASH 11568059 11567429 -630 -0.0
RAM 603176 603208 32 0.0
fabric-bridge-app debug unknown 4696 4720 24 0.5
FLASH 4453208 4454094 886 0.0
RAM 187016 188168 1152 0.6
fabric-sync debug unknown 4952 4976 24 0.5
FLASH 5570197 5571077 880 0.0
RAM 470400 471584 1184 0.3
lighting-app debug+rpc+ui unknown 6160 6184 24 0.4
FLASH 5516481 5517361 880 0.0
RAM 203952 205136 1184 0.6
lock-app debug unknown 5400 5424 24 0.4
FLASH 4689458 4690348 890 0.0
RAM 191144 192328 1184 0.6
ota-provider-app debug unknown 4736 4760 24 0.5
FLASH 4311670 4312458 788 0.0
RAM 179832 180984 1152 0.6
ota-requestor-app debug unknown 4688 4712 24 0.5
FLASH 4441990 4442746 756 0.0
RAM 184320 185472 1152 0.6
shell debug unknown 4216 4240 24 0.6
FLASH 2979724 2979964 240 0.0
RAM 144344 145528 1184 0.8
thermostat-no-ble arm64 unknown 9448 9448 0 0.0
FLASH 4139160 4140088 928 0.0
RAM 229016 229832 816 0.4
tv-app debug unknown 5728 5752 24 0.4
FLASH 5909045 5909989 944 0.0
RAM 593832 595016 1184 0.2
tv-casting-app debug unknown 5304 5320 16 0.3
FLASH 11473757 11473949 192 0.0
RAM 718656 718656 0 0.0
nxp contact k32w0+release FLASH 587368 587152 -216 -0.0
RAM 70980 70980 0 0.0
mcxw71+release FLASH 601192 600992 -200 -0.0
RAM 63096 63096 0 0.0
light k32w0+release FLASH 613100 612884 -216 -0.0
RAM 70268 70268 0 0.0
k32w1+release FLASH 685824 685608 -216 -0.0
RAM 48584 48584 0 0.0
lock mcxw71+release FLASH 750032 749832 -200 -0.0
RAM 67500 67500 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1655684 1655868 184 0.0
RAM 212264 212264 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1562380 1562132 -248 -0.0
RAM 208560 208560 0 0.0
light cy8ckit_062s2_43012 FLASH 1441180 1440948 -232 -0.0
RAM 197296 197296 0 0.0
lock cy8ckit_062s2_43012 FLASH 1470068 1469836 -232 -0.0
RAM 224960 224960 0 0.0
qpg lighting-app qpg6105+debug FLASH 663772 663572 -200 -0.0
RAM 105156 105156 0 0.0
lock-app qpg6105+debug FLASH 622240 622024 -216 -0.0
RAM 99768 99768 0 0.0
stm32 light STM32WB5MM-DK FLASH 459840 459632 -208 -0.0
RAM 141472 141472 0 0.0
telink bridge-app tl7218x FLASH 669192 668900 -292 -0.0
RAM 90752 90752 0 0.0
contact-sensor-app tlsr9528a_retention FLASH 622078 621786 -292 -0.0
RAM 31488 31488 0 0.0
light-app-ota-shell-factory-data tl3218x FLASH 745386 745094 -292 -0.0
RAM 40396 40396 0 0.0
tl7218x FLASH 753948 753656 -292 -0.0
RAM 97540 97540 0 0.0
light-switch-app-ota-compress-lzma-factory-data tl7218x_retention FLASH 681018 680726 -292 -0.0
RAM 52192 52192 0 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 709580 709288 -292 -0.0
RAM 73400 73400 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 600760 600468 -292 -0.0
RAM 138812 138812 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 788988 788696 -292 -0.0
RAM 96388 96388 0 0.0
tizen all-clusters-app arm unknown 5116 5116 0 0.0
FLASH 1766136 1765664 -472 -0.0
RAM 93844 93844 0 0.0
chip-tool-ubsan arm unknown 11492 11492 0 0.0
FLASH 18983958 18981110 -2848 -0.0
RAM 8306328 8305216 -1112 -0.0

bzbarsky-apple
bzbarsky-apple requested changes Feb 27, 2025
View reviewed changes
docs/upgrading.md Outdated
Comment on lines 192 to 193
Be sure to use `EnsureAppendCapacity` before single element Append
`ListBuilder::Append` as it will not grow the buffer if not available already
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence has grammar issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check new phrasing

docs/upgrading.md
using QF = DataModel::CommandQualityFlags;
using Priv = chip::Access::Privilege;

ReturnErrorOnFailure(builder.AppendElements({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this not need EnsureAppendCapacity before AppendElements?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, AppendElements already ensures capacity before, only when using Append is necessary

src/app/data-model-provider/MetadataList.h
///
/// Automatically attempts to allocate sufficient space to fulfill the element
/// requirements.
CHIP_ERROR AppendElements(std::initializer_list<T> span) { return AppendElementArrayRaw(span.begin(), span.size()); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so are the capacity bits before these calls just optimizations then? That should be documented.

Copy link
Contributor Author

@ratgr ratgr Feb 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, and no, Single element Append doesn't allocate and just fails, MultiAppend, aka AppendElements, does allocate as needed.
I don't like the inconsistency either, I feel is error prone, chatted with @andy31415, and will create a ticket to tech debt to figure out a better way and fix otherwise

I suggested renaming Append to TryAppend for the non-allocating function and optionally adding an Append which does the Ensure, also optionally a TryAppendElements for symmetry, and further optimization.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a background, we tried making the Auto-append clunky to encourage people to use "static arrays" instead (larger memory allocations if needed and ability to re-use pointers). In practice when things are conditional, it seems we still kind of default to "append". .... I would probably err on the side of just renaming append to TryAppend and documenting that you may not want to actually use this and prefer to call AppendElements. Or we can also completely drop Append from the public interface.

What I want to avoid is everyone doing Append, Append, Append because it is convenient to write and cause many re-allocations as a result.

@ratgr @andy31415
Update src/app/clusters/network-commissioning/network-commissioning.cpp
088f797 
Co-authored-by: Andrei Litvin <andy314@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andy31415 andy31415 andy31415 approved these changes

@khodya khodya khodya left review comments

@bzbarsky-apple bzbarsky-apple bzbarsky-apple requested changes

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
app controller documentation Improvements or additions to documentation review - pending
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Convert CommandHandlerInterface command listing to provide full data
6 participants
@ratgr @andy31415 @tcarmelveilleux @khodya @bzbarsky-apple @restyled-commits