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

Fix Linux example discriminators to default to 3840 #16477

Merged

Conversation

tcarmelveilleux
Copy link
Contributor

Problem

  • Previous change lost the "default" discriminator people
    were using for a long time (3840) from when all-clusters-app
    is run without specifying the discriminator argument.
  • All users should always specify a non-default discriminator,
    but bringing back the "default" temporarily to avoid
    issues in test procedures.
  • Cert tests and proper usage of testing scripts always explicitly
    set discriminator, so the issue does not arise in CI

Change overview

  • Make default 3840 again, instead of 0

Testing

  • Running all-clusters-app with no args uses 3840
  • Cert tests pass

- Previous change lost the "default" discriminator people
  were using for a long time (3840) from when all-clusters-app
  is run without specifying the discriminator argument.
- All users should always specify a non-default discriminator,
  but bringing back the "default" temporarily to avoid
  issues in test procedures.
- Cert tests and proper usage of testing scripts always explicitly
  set discriminator, so the issue does not arise in CI

Testing done:

- Running all-clusters-app with no args uses 3840
- Cert tests pass
@github-actions
Copy link

github-actions bot commented Mar 19, 2022

PR #16477: Size comparison from a3390fd to 14ed213

Increases (1 build for linux)
platform target config section a3390fd 14ed213 change % change
linux thermostat-no-ble arm64 (read only) 2224068 2224676 608 0.0
(read/write) 146369 146385 16 0.0
.bss 62273 62289 16 0.0
.rodata 138148 138468 320 0.2
.text 1865856 1866144 288 0.0
Full report (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section a3390fd 14ed213 change % change
cyw30739 light cyw930739m2evb_01 (read/write) 603874 603874 0 0.0
.app_xip_area 510972 510972 0 0.0
.bss 75656 75656 0 0.0
.data 596 596 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 561654 561654 0 0.0
.app_xip_area 470280 470280 0 0.0
.bss 74160 74160 0 0.0
.data 560 560 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 571506 571506 0 0.0
.app_xip_area 470484 470484 0 0.0
.bss 83488 83488 0 0.0
.data 500 500 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 921992 921992 0 0.0
(read/write) 128760 128760 0 0.0
.bss 126768 126768 0 0.0
.data 1988 1988 0 0.0
.text 921984 921984 0 0.0
BRD4161A+rpc (read only) 950812 950812 0 0.0
(read/write) 144712 144712 0 0.0
.bss 142544 142544 0 0.0
.data 2168 2168 0 0.0
.text 950804 950804 0 0.0
window-app BRD4161A (read only) 852296 852296 0 0.0
(read/write) 126712 126712 0 0.0
.bss 124848 124848 0 0.0
.data 1864 1864 0 0.0
.text 852288 852288 0 0.0
esp32 all-clusters-app c3devkit (read only) 962000 962000 0 0.0
(read/write) 1393314 1393314 0 0.0
.dram0.bss 62048 62048 0 0.0
.dram0.data 14188 14188 0 0.0
.flash.rodata 198168 198168 0 0.0
.flash.text 962000 962000 0 0.0
.iram0.text 62016 62016 0 0.0
m5stack (read only) 1018099 1018099 0 0.0
(read/write) 461164 461164 0 0.0
.dram0.bss 67576 67576 0 0.0
.dram0.data 34016 34016 0 0.0
.flash.rodata 227736 227736 0 0.0
.flash.text 1012715 1012715 0 0.0
.iram0.text 123107 123107 0 0.0
k32w light k32w061+release (read/write) 701104 701104 0 0.0
.bss 77656 77656 0 0.0
.data 1868 1868 0 0.0
.text 615780 615780 0 0.0
lock k32w061+release (read/write) 700964 700964 0 0.0
.bss 77624 77624 0 0.0
.data 1908 1908 0 0.0
.text 615632 615632 0 0.0
linux chip-tool-ipv6only arm64 (read only) 9769516 9769516 0 0.0
(read/write) 472689 472689 0 0.0
.bss 40609 40609 0 0.0
.data 1128 1128 0 0.0
.data.rel.ro 371824 371824 0 0.0
.dynamic 560 560 0 0.0
.got 55328 55328 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 493844 493844 0 0.0
.text 8224308 8224308 0 0.0
thermostat-no-ble arm64 (read only) 2224068 2224676 608 0.0
(read/write) 146369 146385 16 0.0
.bss 62273 62289 16 0.0
.data 1024 1024 0 0.0
.data.rel.ro 75728 75728 0 0.0
.dynamic 560 560 0 0.0
.got 4352 4352 0 0.0
.init 24 24 0 0.0
.init_array 360 360 0 0.0
.rodata 138148 138468 320 0.2
.text 1865856 1866144 288 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2353460 2353460 0 0.0
.bss 184652 184652 0 0.0
.data 5752 5752 0 0.0
.text 1316060 1316060 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1155919 1155919 0 0.0
bss 146716 146716 0 0.0
rodata 145968 145968 0 0.0
text 788068 788068 0 0.0
p6 all-clusters-app default (read/write) 2492968 2492968 0 0.0
.bss 118072 118072 0 0.0
.data 2632 2632 0 0.0
.text 1451232 1451232 0 0.0
light-app default (read/write) 2396352 2396352 0 0.0
.bss 111544 111544 0 0.0
.data 2488 2488 0 0.0
.text 1354616 1354616 0 0.0
lock-app default (read/write) 2359912 2359912 0 0.0
.bss 111288 111288 0 0.0
.data 2448 2448 0 0.0
.text 1318176 1318176 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 895834 895834 0 0.0
bss 87424 87424 0 0.0
noinit 37160 37160 0 0.0
text 633394 633394 0 0.0

@tcarmelveilleux tcarmelveilleux merged commit 193b4d0 into project-chip:master Mar 20, 2022
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this pull request Apr 14, 2022
* Fix Linux example discriminators to default to 3840

- Previous change lost the "default" discriminator people
  were using for a long time (3840) from when all-clusters-app
  is run without specifying the discriminator argument.
- All users should always specify a non-default discriminator,
  but bringing back the "default" temporarily to avoid
  issues in test procedures.
- Cert tests and proper usage of testing scripts always explicitly
  set discriminator, so the issue does not arise in CI

Testing done:

- Running all-clusters-app with no args uses 3840
- Cert tests pass

* Restyled by clang-format

Co-authored-by: Restyled.io <[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.

4 participants