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

[setup_payload] Fixed bug with not reducing span size properly #18598

Merged
merged 1 commit into from
May 19, 2022
Merged

[setup_payload] Fixed bug with not reducing span size properly #18598

merged 1 commit into from
May 19, 2022

Conversation

kkasperczyk-no
Copy link
Contributor

Problem

Output buffer span is not reduced properly, as subSpan is passed to the base38Encode method and changed size doesn't
propagate to the outBuffer.

Change overview

Added reducing outBuffer size to the subSpan size + prefixLen size.

Testing

@kkasperczyk-no
[setup_payload] Fixed bug with not reducing span size properly
fb84920 
Output buffer span is not reduced properly, as subspan is
passed to the base38Encode method and changed size doesn't
propagate to the outBuffer.

Added reducing outBuffer size to the subSpan + prefixLen size.
Damian-Nordic
Damian-Nordic approved these changes May 19, 2022
View reviewed changes
Copy link
Contributor

@Damian-Nordic Damian-Nordic left a comment

Choose a reason for hiding this comment

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

Consider adding a unit test. It seems TestQRCode contains some tests for the STL-based parser, but nothing for the basic parser. It should be quite easy to copy one to ensure returned span contains exactly what should be returned.

@github-actions
Copy link

github-actions bot commented May 19, 2022 •
edited
Loading

PR #18598: Size comparison from 70e259c to fb84920

Increases (19 builds for cyw30739, efr32, esp32, k32w, linux, nrfconnect, p6, telink)
platform target config section 70e259c fb84920 change % change
cyw30739 light cyw930739m2evb_01 (read/write) 625570 625586 16 0.0
.app_xip_area 529184 529200 16 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 570998 571014 16 0.0
.app_xip_area 466352 466368 16 0.0
efr32 lighting-app BRD4161A (read only) 917676 917692 16 0.0
.text 917668 917684 16 0.0
BRD4161A+rpc (read only) 951848 951864 16 0.0
.text 951840 951856 16 0.0
BRD4161A+rs911x (read only) 791140 791156 16 0.0
.text 791132 791148 16 0.0
lock-app BRD4161A+wf200 (read only) 947140 947156 16 0.0
.text 947132 947148 16 0.0
window-app BRD4161A (read only) 897796 897812 16 0.0
.text 897788 897804 16 0.0
esp32 all-clusters-app c3devkit (read only) 1000894 1000918 24 0.0
.flash.text 1000894 1000918 24 0.0
m5stack (read only) 1056019 1056039 20 0.0
.flash.text 1050635 1050655 20 0.0
k32w light k32w061+release (read/write) 683032 683048 16 0.0
.text 599088 599104 16 0.0
lock k32w061+release (read/write) 729608 729624 16 0.0
.text 645272 645288 16 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9078228 9078260 32 0.0
.text 7169076 7169108 32 0.0
thermostat-no-ble arm64 (read only) 2353036 2353068 32 0.0
.text 1975808 1975840 32 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1181151 1181167 16 0.0
text 810720 810736 16 0.0
p6 all-clusters-app default (read/write) 2539096 2539112 16 0.0
.text 1497360 1497376 16 0.0
light-app default (read/write) 2425512 2425528 16 0.0
.text 1383776 1383792 16 0.0
lock-app default (read/write) 2436224 2436240 16 0.0
.text 1394488 1394504 16 0.0
telink light-switch-app tlsr9518adk80d (read/write) 782752 782768 16 0.0
text 553478 553496 18 0.0
lighting-app tlsr9518adk80d (read/write) 802844 802860 16 0.0
text 570244 570262 18 0.0
Full report (21 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 70e259c fb84920 change % change
cyw30739 light cyw930739m2evb_01 (read/write) 625570 625586 16 0.0
.app_xip_area 529184 529200 16 0.0
.bss 79028 79028 0 0.0
.data 708 708 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 628542 628542 0 0.0
.app_xip_area 533612 533612 0 0.0
.bss 77604 77604 0 0.0
.data 672 672 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 570998 571014 16 0.0
.app_xip_area 466352 466368 16 0.0
.bss 87024 87024 0 0.0
.data 584 584 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 917676 917692 16 0.0
(read/write) 133244 133244 0 0.0
.bss 131184 131184 0 0.0
.data 2060 2060 0 0.0
.text 917668 917684 16 0.0
BRD4161A+rpc (read only) 951848 951864 16 0.0
(read/write) 149928 149928 0 0.0
.bss 147664 147664 0 0.0
.data 2264 2264 0 0.0
.text 951840 951856 16 0.0
BRD4161A+rs911x (read only) 791140 791156 16 0.0
(read/write) 129512 129512 0 0.0
.bss 127444 127444 0 0.0
.data 2068 2068 0 0.0
.text 791132 791148 16 0.0
lock-app BRD4161A+wf200 (read only) 947140 947156 16 0.0
(read/write) 123996 123996 0 0.0
.bss 121972 121972 0 0.0
.data 2024 2024 0 0.0
.text 947132 947148 16 0.0
window-app BRD4161A (read only) 897796 897812 16 0.0
(read/write) 133304 133304 0 0.0
.bss 131256 131256 0 0.0
.data 2048 2048 0 0.0
.text 897788 897804 16 0.0
esp32 all-clusters-app c3devkit (read only) 1000894 1000918 24 0.0
(read/write) 1478218 1478218 0 0.0
.dram0.bss 68208 68208 0 0.0
.dram0.data 14624 14624 0 0.0
.flash.rodata 209960 209960 0 0.0
.flash.text 1000894 1000918 24 0.0
.iram0.text 62954 62954 0 0.0
m5stack (read only) 1056019 1056039 20 0.0
(read/write) 480212 480212 0 0.0
.dram0.bss 73728 73728 0 0.0
.dram0.data 34200 34200 0 0.0
.flash.rodata 240288 240288 0 0.0
.flash.text 1050635 1050655 20 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 683032 683048 16 0.0
.bss 80224 80224 0 0.0
.data 2016 2016 0 0.0
.text 599088 599104 16 0.0
lock k32w061+release (read/write) 729608 729624 16 0.0
.bss 80656 80656 0 0.0
.data 1976 1976 0 0.0
.text 645272 645288 16 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9078228 9078260 32 0.0
(read/write) 646209 646209 0 0.0
.bss 42209 42209 0 0.0
.data 1192 1192 0 0.0
.data.rel.ro 583944 583944 0 0.0
.dynamic 560 560 0 0.0
.got 15016 15016 0 0.0
.init 24 24 0 0.0
.init_array 184 184 0 0.0
.rodata 444948 444948 0 0.0
.text 7169076 7169108 32 0.0
thermostat-no-ble arm64 (read only) 2353036 2353068 32 0.0
(read/write) 176289 176289 0 0.0
.bss 87585 87585 0 0.0
.data 1520 1520 0 0.0
.data.rel.ro 79376 79376 0 0.0
.dynamic 560 560 0 0.0
.got 4768 4768 0 0.0
.init 24 24 0 0.0
.init_array 376 376 0 0.0
.rodata 147244 147244 0 0.0
.text 1975808 1975840 32 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2420736 2420736 0 0.0
.bss 202668 202668 0 0.0
.data 5872 5872 0 0.0
.text 1383380 1383380 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1181151 1181167 16 0.0
bss 138380 138380 0 0.0
rodata 153176 153176 0 0.0
text 810720 810736 16 0.0
p6 all-clusters-app default (read/write) 2539096 2539112 16 0.0
.bss 136176 136176 0 0.0
.data 2808 2808 0 0.0
.text 1497360 1497376 16 0.0
light-app default (read/write) 2425512 2425528 16 0.0
.bss 129488 129488 0 0.0
.data 2608 2608 0 0.0
.text 1383776 1383792 16 0.0
lock-app default (read/write) 2436224 2436240 16 0.0
.bss 129304 129304 0 0.0
.data 2568 2568 0 0.0
.text 1394488 1394504 16 0.0
telink light-switch-app tlsr9518adk80d (read/write) 782752 782768 16 0.0
bss 70608 70608 0 0.0
noinit 40416 40416 0 0.0
text 553478 553496 18 0.0
lighting-app tlsr9518adk80d (read/write) 802844 802860 16 0.0
bss 70864 70864 0 0.0
noinit 40416 40416 0 0.0
text 570244 570262 18 0.0

@andy31415 andy31415 merged commit 0381682 into project-chip:master May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Damian-Nordic Damian-Nordic Damian-Nordic approved these changes

@andy31415 andy31415 andy31415 approved these changes

@anush-apple anush-apple Awaiting requested review from anush-apple

@arkq arkq Awaiting requested review from arkq

@Byungjoo-Lee Byungjoo-Lee Awaiting requested review from Byungjoo-Lee

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

@carol-apple carol-apple Awaiting requested review from carol-apple

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@franck-apple franck-apple Awaiting requested review from franck-apple

@gjc13 gjc13 Awaiting requested review from gjc13

@hawk248 hawk248 Awaiting requested review from hawk248

@harsha-rajendran harsha-rajendran Awaiting requested review from harsha-rajendran

@isiu-apple isiu-apple Awaiting requested review from isiu-apple

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@jtung-apple jtung-apple Awaiting requested review from jtung-apple

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@rgoliver rgoliver Awaiting requested review from rgoliver

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@tecimovic tecimovic Awaiting requested review from tecimovic

@turon turon Awaiting requested review from turon

@vijs vijs Awaiting requested review from vijs

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@woody-apple woody-apple Awaiting requested review from woody-apple

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yufengwangca yufengwangca Awaiting requested review from yufengwangca

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

Assignees
No one assigned
Labels
review - approved setup payload
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kkasperczyk-no @andy31415 @Damian-Nordic