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

Post merge cleanup 36229 ember buffer encode #36266

Conversation

andy31415
Copy link
Contributor

Fixes post review comments from @bzbarsky-apple on #36229

Copy link

Review changes with SemanticDiff.

Copy link

github-actions bot commented Oct 28, 2024

PR #36266: Size comparison from 2142870 to 469c65a

Full report (7 builds for cc13x4_26x4, cc32xx, stm32)
platform target config section 2142870 469c65a change % change
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 836428 836428 0 0.0
RAM 123676 123676 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 821824 821824 0 0.0
RAM 125556 125556 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 768412 768412 0 0.0
RAM 114048 114048 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 752672 752672 0 0.0
RAM 114240 114240 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 626506 626506 0 0.0
RAM 206132 206132 0 0.0
lock CC3235SF_LAUNCHXL FLASH 666482 666482 0 0.0
RAM 206284 206284 0 0.0
stm32 light STM32WB5MM-DK FLASH 481680 481680 0 0.0
RAM 144844 144844 0 0.0

Copy link

github-actions bot commented Oct 28, 2024

PR #36266: Size comparison from 2142870 to ed35ddd

Full report (14 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, stm32, tizen)
platform target config section 2142870 ed35ddd change % change
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 836428 836428 0 0.0
RAM 123676 123676 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 821824 821824 0 0.0
RAM 125556 125556 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 768412 768412 0 0.0
RAM 114048 114048 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 752672 752672 0 0.0
RAM 114240 114240 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 626506 626506 0 0.0
RAM 206132 206132 0 0.0
lock CC3235SF_LAUNCHXL FLASH 666482 666482 0 0.0
RAM 206284 206284 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 914360 914360 0 0.0
RAM 143357 143357 0 0.0
nrf7002dk_nrf5340_cpuapp FLASH 885896 885896 0 0.0
RAM 141496 141496 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 847928 847928 0 0.0
RAM 142265 142265 0 0.0
qpg lighting-app qpg6105+debug FLASH 660392 660392 0 0.0
RAM 105420 105420 0 0.0
lock-app qpg6105+debug FLASH 618460 618460 0 0.0
RAM 99880 99880 0 0.0
stm32 light STM32WB5MM-DK FLASH 481680 481680 0 0.0
RAM 144844 144844 0 0.0
tizen all-clusters-app arm unknown 4912 4912 0 0.0
FLASH 1720944 1720944 0 0.0
RAM 90108 90108 0 0.0
chip-tool-ubsan arm unknown 10792 10792 0 0.0
FLASH 18157806 18157806 0 0.0
RAM 7915456 7915456 0 0.0

Copy link

github-actions bot commented Oct 28, 2024

PR #36266: Size comparison from 2142870 to 7ac4e4f

Full report (11 builds for cc13x4_26x4, cc32xx, qpg, stm32, tizen)
platform target config section 2142870 7ac4e4f change % change
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 836428 836428 0 0.0
RAM 123676 123676 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 821824 821824 0 0.0
RAM 125556 125556 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 768412 768412 0 0.0
RAM 114048 114048 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 752672 752672 0 0.0
RAM 114240 114240 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 626506 626506 0 0.0
RAM 206132 206132 0 0.0
lock CC3235SF_LAUNCHXL FLASH 666482 666482 0 0.0
RAM 206284 206284 0 0.0
qpg lighting-app qpg6105+debug FLASH 660392 660392 0 0.0
RAM 105420 105420 0 0.0
lock-app qpg6105+debug FLASH 618460 618460 0 0.0
RAM 99880 99880 0 0.0
stm32 light STM32WB5MM-DK FLASH 481680 481680 0 0.0
RAM 144844 144844 0 0.0
tizen all-clusters-app arm unknown 4912 4912 0 0.0
FLASH 1720944 1720944 0 0.0
RAM 90108 90108 0 0.0
chip-tool-ubsan arm unknown 10792 10792 0 0.0
FLASH 18157806 18157806 0 0.0
RAM 7915456 7915456 0 0.0

Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

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

#36229 (comment) does not seem to be addressed.

@andy31415
Copy link
Contributor Author

#36229 (comment) does not seem to be addressed.

Done. Did not see header comments. The comment was a copy and paste and it turned out handling both signed and unsigned into a single method was smaller code.

Copy link

github-actions bot commented Oct 28, 2024

PR #36266: Size comparison from 928efd7 to 3d364fa

Full report (10 builds for cc32xx, nrfconnect, qpg, stm32, tizen)
platform target config section 928efd7 3d364fa change % change
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 626506 626506 0 0.0
RAM 206132 206132 0 0.0
lock CC3235SF_LAUNCHXL FLASH 666482 666482 0 0.0
RAM 206284 206284 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 914360 914360 0 0.0
RAM 143357 143357 0 0.0
nrf7002dk_nrf5340_cpuapp FLASH 885896 885896 0 0.0
RAM 141496 141496 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 847928 847928 0 0.0
RAM 142265 142265 0 0.0
qpg lighting-app qpg6105+debug FLASH 660392 660392 0 0.0
RAM 105420 105420 0 0.0
lock-app qpg6105+debug FLASH 618460 618460 0 0.0
RAM 99880 99880 0 0.0
stm32 light STM32WB5MM-DK FLASH 481680 481680 0 0.0
RAM 144844 144844 0 0.0
tizen all-clusters-app arm unknown 4912 4912 0 0.0
FLASH 1720944 1720944 0 0.0
RAM 90108 90108 0 0.0
chip-tool-ubsan arm unknown 10792 10792 0 0.0
FLASH 18157806 18157806 0 0.0
RAM 7915456 7915456 0 0.0

Copy link

github-actions bot commented Oct 28, 2024

PR #36266: Size comparison from 928efd7 to 5a2681f

Full report (11 builds for cc13x4_26x4, cc32xx, qpg, stm32, tizen)
platform target config section 928efd7 5a2681f change % change
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 836428 836428 0 0.0
RAM 123676 123676 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 821824 821824 0 0.0
RAM 125556 125556 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 768412 768412 0 0.0
RAM 114048 114048 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 752672 752672 0 0.0
RAM 114240 114240 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 626506 626506 0 0.0
RAM 206132 206132 0 0.0
lock CC3235SF_LAUNCHXL FLASH 666482 666482 0 0.0
RAM 206284 206284 0 0.0
qpg lighting-app qpg6105+debug FLASH 660392 660392 0 0.0
RAM 105420 105420 0 0.0
lock-app qpg6105+debug FLASH 618460 618460 0 0.0
RAM 99880 99880 0 0.0
stm32 light STM32WB5MM-DK FLASH 481680 481680 0 0.0
RAM 144844 144844 0 0.0
tizen all-clusters-app arm unknown 4912 4912 0 0.0
FLASH 1720944 1720944 0 0.0
RAM 90108 90108 0 0.0
chip-tool-ubsan arm unknown 10792 10792 0 0.0
FLASH 18157806 18157806 0 0.0
RAM 7915456 7915456 0 0.0

Copy link

github-actions bot commented Oct 28, 2024

PR #36266: Size comparison from 928efd7 to 8276b4d

Full report (68 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
platform target config section 928efd7 8276b4d change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1349958 1349958 0 0.0
RAM 104120 104120 0 0.0
bl702 lighting-app bl702+eth FLASH 647354 647354 0 0.0
RAM 25233 25233 0 0.0
bl702+wifi FLASH 824918 824918 0 0.0
RAM 13965 13965 0 0.0
bl706+mfd+rpc+littlefs FLASH 1053908 1053908 0 0.0
RAM 23821 23821 0 0.0
bl702l lighting-app bl702l+mfd+littlefs FLASH 974692 974692 0 0.0
RAM 16468 16468 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 836428 836428 0 0.0
RAM 123676 123676 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 821824 821824 0 0.0
RAM 125556 125556 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 768412 768412 0 0.0
RAM 114048 114048 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 752672 752672 0 0.0
RAM 114240 114240 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 626506 626506 0 0.0
RAM 206132 206132 0 0.0
lock CC3235SF_LAUNCHXL FLASH 666482 666482 0 0.0
RAM 206284 206284 0 0.0
cyw30739 light CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 678389 678389 0 0.0
RAM 78692 78692 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 698233 698233 0 0.0
RAM 81324 81324 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 698233 698233 0 0.0
RAM 81324 81324 0 0.0
CYW930739M2EVB-02 unknown 2040 2040 0 0.0
FLASH 655169 655169 0 0.0
RAM 73760 73760 0 0.0
light-switch CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 614837 614837 0 0.0
RAM 71644 71644 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 634465 634465 0 0.0
RAM 74196 74196 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 634465 634465 0 0.0
RAM 74196 74196 0 0.0
lock CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 634189 634189 0 0.0
RAM 74692 74692 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 653897 653897 0 0.0
RAM 77244 77244 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 653897 653897 0 0.0
RAM 77244 77244 0 0.0
thermostat CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 609445 609445 0 0.0
RAM 68780 68780 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 629297 629297 0 0.0
RAM 71412 71412 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 629297 629297 0 0.0
RAM 71412 71412 0 0.0
efr32 lock-app BRD4187C FLASH 925516 925508 -8 -0.0
RAM 159920 159920 0 0.0
BRD4338a FLASH 741256 741256 0 0.0
RAM 232580 232580 0 0.0
window-app BRD4187C FLASH 1017480 1017480 0 0.0
RAM 128264 128264 0 0.0
esp32 all-clusters-app c3devkit DRAM 95256 95256 0 0.0
FLASH 1538746 1538746 0 0.0
IRAM 82538 82538 0 0.0
m5stack DRAM 116192 116192 0 0.0
FLASH 1548958 1548958 0 0.0
IRAM 117039 117039 0 0.0
linux air-purifier-app debug unknown 4688 4688 0 0.0
FLASH 2736597 2736309 -288 -0.0
RAM 129520 129520 0 0.0
all-clusters-app debug unknown 5528 5528 0 0.0
FLASH 6038902 6038614 -288 -0.0
RAM 523840 523840 0 0.0
all-clusters-minimal-app debug unknown 5424 5424 0 0.0
FLASH 5371494 5371238 -256 -0.0
RAM 242416 242416 0 0.0
bridge-app debug unknown 5408 5408 0 0.0
FLASH 4705056 4704768 -288 -0.0
RAM 218384 218384 0 0.0
chip-tool debug unknown 5960 5960 0 0.0
FLASH 12980390 12980102 -288 -0.0
RAM 584466 584466 0 0.0
chip-tool-ipv6only arm64 unknown 21416 21416 0 0.0
FLASH 11115672 11115208 -464 -0.0
RAM 635408 635408 0 0.0
fabric-admin debug unknown 5792 5792 0 0.0
FLASH 11310181 11309925 -256 -0.0
RAM 584954 584954 0 0.0
fabric-bridge-app debug unknown 4632 4632 0 0.0
FLASH 4531488 4531200 -288 -0.0
RAM 205336 205336 0 0.0
fabric-sync debug unknown 4840 4840 0 0.0
FLASH 5243973 5243685 -288 -0.0
RAM 465416 465416 0 0.0
lighting-app debug+rpc+ui unknown 6056 6056 0 0.0
FLASH 5646113 5645825 -288 -0.0
RAM 228488 228488 0 0.0
lock-app debug unknown 5344 5344 0 0.0
FLASH 4753934 4753646 -288 -0.0
RAM 204472 204472 0 0.0
ota-provider-app debug unknown 4720 4720 0 0.0
FLASH 4380642 4380386 -256 -0.0
RAM 198192 198192 0 0.0
ota-requestor-app debug unknown 4656 4656 0 0.0
FLASH 4519746 4519458 -288 -0.0
RAM 202760 202760 0 0.0
shell debug unknown 4216 4216 0 0.0
FLASH 3065101 3064813 -288 -0.0
RAM 160368 160368 0 0.0
thermostat-no-ble arm64 unknown 9456 9456 0 0.0
FLASH 4123816 4123352 -464 -0.0
RAM 242896 242896 0 0.0
tv-app debug unknown 5624 5624 0 0.0
FLASH 5980405 5980149 -256 -0.0
RAM 596256 596256 0 0.0
tv-casting-app debug unknown 5208 5208 0 0.0
FLASH 11187373 11187085 -288 -0.0
RAM 694536 694536 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 914360 914360 0 0.0
RAM 143357 143357 0 0.0
nrf7002dk_nrf5340_cpuapp FLASH 885896 885896 0 0.0
RAM 141496 141496 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 847928 847928 0 0.0
RAM 142265 142265 0 0.0
nxp contact k32w0+release FLASH 582296 582296 0 0.0
RAM 71092 71092 0 0.0
mcxw71+release FLASH 596776 596776 0 0.0
RAM 63184 63184 0 0.0
light k32w0+release FLASH 618900 618900 0 0.0
RAM 70556 70556 0 0.0
k32w1+release FLASH 682992 682992 0 0.0
RAM 48816 48816 0 0.0
lock mcxw71+release FLASH 746568 746568 0 0.0
RAM 67340 67340 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1643620 1643620 0 0.0
RAM 212408 212408 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1550588 1550588 0 0.0
RAM 209208 209208 0 0.0
light cy8ckit_062s2_43012 FLASH 1466564 1466564 0 0.0
RAM 201200 201200 0 0.0
lock cy8ckit_062s2_43012 FLASH 1464004 1464004 0 0.0
RAM 225560 225560 0 0.0
qpg lighting-app qpg6105+debug FLASH 660392 660392 0 0.0
RAM 105420 105420 0 0.0
lock-app qpg6105+debug FLASH 618460 618460 0 0.0
RAM 99880 99880 0 0.0
stm32 light STM32WB5MM-DK FLASH 481680 481680 0 0.0
RAM 144844 144844 0 0.0
telink bridge-app tlsr9258a FLASH 680902 680902 0 0.0
RAM 91304 91304 0 0.0
contact-sensor-app tlsr9528a_retention FLASH 620500 620500 0 0.0
RAM 50600 50600 0 0.0
light-switch-app-ota-shell-factory-data tlsr9528a FLASH 708438 708438 0 0.0
RAM 73940 73940 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 625344 625344 0 0.0
RAM 144468 144468 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 811354 811354 0 0.0
RAM 99100 99100 0 0.0
tizen all-clusters-app arm unknown 4912 4912 0 0.0
FLASH 1720944 1720944 0 0.0
RAM 90108 90108 0 0.0
chip-tool-ubsan arm unknown 10792 10792 0 0.0
FLASH 18157806 18157806 0 0.0
RAM 7915456 7915456 0 0.0

@andy31415 andy31415 merged commit c50c591 into project-chip:master Oct 28, 2024
67 checks passed
yyzhong-g pushed a commit to yyzhong-g/connectedhomeip that referenced this pull request Dec 12, 2024
* Use chip::app::IsSignedAttributeType

* Fix up put as well as naming for null value and comment

* Fix up nullable tests

* Test that you cannot decode a null value for non-nullable double and single

* Allow NAN for non-nullable floating points

* Add test case for non nullable bool

* Restyle

* Add a header for efr32

* Update src/app/codegen-data-model-provider/EmberAttributeDataBuffer.cpp

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/app/codegen-data-model-provider/EmberAttributeDataBuffer.cpp

Co-authored-by: Boris Zbarsky <[email protected]>

* Remove extra comment

* Replace switch with if

* Comment fix

* Another try to make efr32 build of tests happy

* Move includes around, to try to work around issues within efr32 compiles...

* more updates, this time local efr32 compiles

---------

Co-authored-by: Boris Zbarsky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants