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

[NetworkCommissioning] Send Response Command instead of status code for network commissioning cluster #12010

Merged
merged 1 commit into from
Nov 19, 2021
Merged

[NetworkCommissioning] Send Response Command instead of status code for network commissioning cluster #12010

merged 1 commit into from
Nov 19, 2021

Conversation

erjiaqing
Copy link
Contributor

@erjiaqing erjiaqing commented Nov 19, 2021 •
edited
Loading

Problem

Network Commissioning cluster commands are sending status codes instead of cluster commands, this should be fixed

Fixes #12006

Change overview

  • Use cluster object to encode command response

Testing

  • Updated Cirque test case to verify the response.

@erjiaqing
Copy link
Contributor Author

@vivien-apple Can you verify if CHIPTool still works fine? IIRC it just ignored the response and always call default success callback. Thanks.

@github-actions
Copy link

github-actions bot commented Nov 19, 2021 •
edited
Loading

PR #12010: Size comparison from ebadd33 to 4d21e3d

Increases above 0.2%:

platform target config section ebadd33 4d21e3d change % change
linux bridge-app debug+rpc .text 1111845 1114997 3152 0.3
ota-provider-app debug (read only) 1268729 1271993 3264 0.3
.text 1057186 1060354 3168 0.3
ota-requestor-app debug .text 1129522 1132690 3168 0.3
Increases (30 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section ebadd33 4d21e3d change % change
efr32 lighting-app BRD4161A (read only) 749960 750424 464 0.1
.text 749952 750416 464 0.1
BRD4161A+rpc (read only) 737484 737948 464 0.1
.text 737476 737940 464 0.1
lock-app BRD4161A (read only) 727016 727480 464 0.1
.text 727008 727472 464 0.1
window-app BRD4161A (read only) 728000 728464 464 0.1
.text 727992 728456 464 0.1
esp32 all-clusters-app c3devkit (read only) 828538 828870 332 0.0
.flash.text 828538 828870 332 0.0
m5stack (read only) 899595 899875 280 0.0
.flash.text 894211 894491 280 0.0
k32w lighting-app k32w061+se05x+release (read/write) 706872 707240 368 0.1
.text 621996 622364 368 0.1
lock-app k32w061+debug (read/write) 598312 598680 368 0.1
.text 522956 523324 368 0.1
shell k32w061+debug (read/write) 663656 664024 368 0.1
.text 577204 577572 368 0.1
linux all-clusters-app debug (read only) 1726153 1729401 3248 0.2
.rodata 136661 136757 96 0.1
.text 1455538 1458690 3152 0.2
bridge-app debug+rpc (read only) 1321181 1324429 3248 0.2
.rodata 111932 112028 96 0.1
.text 1111845 1114997 3152 0.3
lighting-app debug+rpc (read only) 1590289 1593521 3232 0.2
.rodata 129553 129617 64 0.0
.text 1325730 1328898 3168 0.2
ota-provider-app debug (read only) 1268729 1271993 3264 0.3
.rodata 113951 114047 96 0.1
.text 1057186 1060354 3168 0.3
ota-requestor-app debug (read only) 1354465 1357729 3264 0.2
.rodata 124968 125064 96 0.1
.text 1129522 1132690 3168 0.3
tv-app debug (read only) 1872785 1876049 3264 0.2
.rodata 157893 157989 96 0.1
.text 1569762 1572930 3168 0.2
mbed all-clusters-app CY8CPROTO_062_4343W+release (read/write) 2287616 2288000 384 0.0
.text 1250216 1250600 384 0.0
lighting-app CY8CPROTO_062_4343W+release (read/write) 2273552 2273936 384 0.0
.text 1236152 1236536 384 0.0
lock-app CY8CPROTO_062_4343W+release (read/write) 2249456 2249840 384 0.0
.text 1212056 1212440 384 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 864595 864963 368 0.0
text 582604 582960 356 0.1
nrf52840dk_nrf52840+rpc (read/write) 826995 827363 368 0.0
text 556772 557128 356 0.1
nrf5340dk_nrf5340_cpuapp (read/write) 789650 790002 352 0.0
text 512076 512432 356 0.1
lock-app nrf52840dk_nrf52840 (read/write) 840027 840379 352 0.0
text 562696 563056 360 0.1
nrf5340dk_nrf5340_cpuapp (read/write) 765330 765682 352 0.0
text 492260 492620 360 0.1
pump-app nrf52840dk_nrf52840 (read/write) 846135 846487 352 0.0
text 566880 567236 356 0.1
pump-controller-app nrf52840dk_nrf52840 (read/write) 839727 840079 352 0.0
text 562316 562676 360 0.1
p6 all-clusters-app default (read/write) 2297968 2298416 448 0.0
.text 1256232 1256680 448 0.0
lock-app default (read/write) 2215456 2215904 448 0.0
.text 1173720 1174168 448 0.0
qpg lighting-app qpg6100+debug (read only) 493688 494072 384 0.1
.text 488368 488752 384 0.1
lock-app qpg6100+debug (read only) 468652 469036 384 0.1
.text 463332 463716 384 0.1
telink lighting-app tlsr9518adk80d (read/write) 768010 768346 336 0.0
text 533194 533530 336 0.1
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section ebadd33 4d21e3d change % change
efr32 lighting-app BRD4161A (read only) 749960 750424 464 0.1
(read/write) 115540 115540 0 0.0
.bss 113740 113740 0 0.0
.data 1800 1800 0 0.0
.text 749952 750416 464 0.1
BRD4161A+rpc (read only) 737484 737948 464 0.1
(read/write) 132168 132168 0 0.0
.bss 130244 130244 0 0.0
.data 1924 1924 0 0.0
.text 737476 737940 464 0.1
lock-app BRD4161A (read only) 727016 727480 464 0.1
(read/write) 113324 113324 0 0.0
.bss 111564 111564 0 0.0
.data 1756 1756 0 0.0
.text 727008 727472 464 0.1
window-app BRD4161A (read only) 728000 728464 464 0.1
(read/write) 113644 113644 0 0.0
.bss 111884 111884 0 0.0
.data 1760 1760 0 0.0
.text 727992 728456 464 0.1
esp32 all-clusters-app c3devkit (read only) 828538 828870 332 0.0
(read/write) 1220178 1220178 0 0.0
.dram0.bss 56688 56688 0 0.0
.dram0.data 14084 14084 0 0.0
.flash.rodata 164152 164152 0 0.0
.flash.text 828538 828870 332 0.0
.iram0.text 61394 61394 0 0.0
m5stack (read only) 899595 899875 280 0.0
(read/write) 419492 419492 0 0.0
.dram0.bss 62040 62040 0 0.0
.dram0.data 34056 34056 0 0.0
.flash.rodata 192116 192116 0 0.0
.flash.text 894211 894491 280 0.0
.iram0.text 122943 122943 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 706872 707240 368 0.1
.bss 77164 77164 0 0.0
.data 1912 1912 0 0.0
.text 621996 622364 368 0.1
lock-app k32w061+debug (read/write) 598312 598680 368 0.1
.bss 67676 67676 0 0.0
.data 1880 1880 0 0.0
.text 522956 523324 368 0.1
shell k32w061+debug (read/write) 663656 664024 368 0.1
.bss 78804 78804 0 0.0
.data 1848 1848 0 0.0
.text 577204 577572 368 0.1
linux all-clusters-app debug (read only) 1726153 1729401 3248 0.2
(read/write) 126816 126816 0 0.0
.bss 57392 57392 0 0.0
.data 1042 1042 0 0.0
.data.rel.ro 63088 63088 0 0.0
.dynamic 592 592 0 0.0
.got 4112 4112 0 0.0
.init 27 27 0 0.0
.init_array 552 552 0 0.0
.rodata 136661 136757 96 0.1
.text 1455538 1458690 3152 0.2
bridge-app debug+rpc (read only) 1321181 1324429 3248 0.2
(read/write) 77008 77008 0 0.0
.bss 42096 42096 0 0.0
.data 1568 1568 0 0.0
.data.rel.ro 28352 28352 0 0.0
.dynamic 592 592 0 0.0
.got 3984 3984 0 0.0
.init 27 27 0 0.0
.init_array 408 408 0 0.0
.rodata 111932 112028 96 0.1
.text 1111845 1114997 3152 0.3
chip-tool debug (read only) 4986061 4986061 0 0.0
(read/write) 166888 166888 0 0.0
.bss 40456 40456 0 0.0
.data 2272 2272 0 0.0
.data.rel.ro 118672 118672 0 0.0
.dynamic 592 592 0 0.0
.got 4416 4416 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 268458 268458 0 0.0
.text 4407093 4407093 0 0.0
lighting-app debug+rpc (read only) 1590289 1593521 3232 0.2
(read/write) 110224 110224 0 0.0
.bss 47760 47760 0 0.0
.data 1234 1234 0 0.0
.data.rel.ro 55920 55920 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 536 536 0 0.0
.rodata 129553 129617 64 0.0
.text 1325730 1328898 3168 0.2
ota-provider-app debug (read only) 1268729 1271993 3264 0.3
(read/write) 75656 75656 0 0.0
.bss 44704 44704 0 0.0
.data 784 784 0 0.0
.data.rel.ro 25080 25080 0 0.0
.dynamic 592 592 0 0.0
.got 4016 4016 0 0.0
.init 27 27 0 0.0
.init_array 448 448 0 0.0
.rodata 113951 114047 96 0.1
.text 1057186 1060354 3168 0.3
ota-requestor-app debug (read only) 1354465 1357729 3264 0.2
(read/write) 79392 79392 0 0.0
.bss 47168 47168 0 0.0
.data 848 848 0 0.0
.data.rel.ro 26280 26280 0 0.0
.dynamic 592 592 0 0.0
.got 3992 3992 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 124968 125064 96 0.1
.text 1129522 1132690 3168 0.3
shell debug (read only) 798801 798801 0 0.0
(read/write) 66440 66440 0 0.0
.bss 23944 23944 0 0.0
.data 242 242 0 0.0
.data.rel.ro 37752 37752 0 0.0
.dynamic 592 592 0 0.0
.got 3528 3528 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 78703 78703 0 0.0
.text 616082 616082 0 0.0
tv-app debug (read only) 1872785 1876049 3264 0.2
(read/write) 318968 318968 0 0.0
.bss 250376 250376 0 0.0
.data 2768 2768 0 0.0
.data.rel.ro 60144 60144 0 0.0
.dynamic 592 592 0 0.0
.got 4432 4432 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 157893 157989 96 0.1
.text 1569762 1572930 3168 0.2
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2287616 2288000 384 0.0
.bss 179284 179284 0 0.0
.data 5216 5216 0 0.0
.heap 851944 851944 0 0.0
.text 1250216 1250600 384 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2273552 2273936 384 0.0
.bss 172148 172148 0 0.0
.data 5576 5576 0 0.0
.heap 858720 858720 0 0.0
.text 1236152 1236536 384 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2249456 2249840 384 0.0
.bss 171036 171036 0 0.0
.data 5568 5568 0 0.0
.heap 859840 859840 0 0.0
.text 1212056 1212440 384 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1139744 1139744 0 0.0
.bss 11752 11752 0 0.0
.data 4368 4368 0 0.0
.heap 1020328 1020328 0 0.0
.text 103128 103128 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2050480 2050480 0 0.0
.bss 156224 156224 0 0.0
.data 4968 4968 0 0.0
.heap 875256 875256 0 0.0
.text 1013080 1013080 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 864595 864963 368 0.0
bss 110560 110560 0 0.0
rodata 95828 95828 0 0.0
text 582604 582960 356 0.1
nrf52840dk_nrf52840+rpc (read/write) 826995 827363 368 0.0
bss 106916 106916 0 0.0
rodata 87012 87012 0 0.0
text 556772 557128 356 0.1
nrf5340dk_nrf5340_cpuapp (read/write) 789650 790002 352 0.0
bss 111936 111936 0 0.0
rodata 91084 91084 0 0.0
text 512076 512432 356 0.1
lock-app nrf52840dk_nrf52840 (read/write) 840027 840379 352 0.0
bss 109584 109584 0 0.0
rodata 92300 92300 0 0.0
text 562696 563056 360 0.1
nrf5340dk_nrf5340_cpuapp (read/write) 765330 765682 352 0.0
bss 110996 110996 0 0.0
rodata 87604 87604 0 0.0
text 492260 492620 360 0.1
pigweed-app nrf52840dk_nrf52840 (read/write) 497327 497327 0 0.0
bss 51824 51824 0 0.0
rodata 45780 45780 0 0.0
text 339436 339436 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 846135 846487 352 0.0
bss 109724 109724 0 0.0
rodata 94008 94008 0 0.0
text 566880 567236 356 0.1
pump-controller-app nrf52840dk_nrf52840 (read/write) 839727 840079 352 0.0
bss 109620 109620 0 0.0
rodata 92256 92256 0 0.0
text 562316 562676 360 0.1
shell nrf52840dk_nrf52840 (read/write) 777971 777971 0 0.0
bss 109068 109068 0 0.0
rodata 73064 73064 0 0.0
text 521236 521236 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 693026 693026 0 0.0
bss 110052 110052 0 0.0
rodata 67708 67708 0 0.0
text 441848 441848 0 0.0
p6 all-clusters-app default (read/write) 2297968 2298416 448 0.0
.bss 112272 112272 0 0.0
.data 2520 2520 0 0.0
.heap 918552 918552 0 0.0
.text 1256232 1256680 448 0.0
lock-app default (read/write) 2215456 2215904 448 0.0
.bss 100896 100896 0 0.0
.data 2400 2400 0 0.0
.heap 930048 930048 0 0.0
.text 1173720 1174168 448 0.0
qpg lighting-app qpg6100+debug (read only) 493688 494072 384 0.1
(read/write) 114144 114144 0 0.0
.bss 50208 50208 0 0.0
.data 1008 1008 0 0.0
.text 488368 488752 384 0.1
lock-app qpg6100+debug (read only) 468652 469036 384 0.1
(read/write) 114140 114140 0 0.0
.bss 49152 49152 0 0.0
.data 964 964 0 0.0
.text 463332 463716 384 0.1
persistent-storage-app qpg6100+debug (read only) 105408 105408 0 0.0
(read/write) 114142 114142 0 0.0
.bss 8978 8978 0 0.0
.data 272 272 0 0.0
.text 100088 100088 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 768010 768346 336 0.0
bss 79072 79072 0 0.0
noinit 37160 37160 0 0.0
text 533194 533530 336 0.1

@vivien-apple
Copy link
Contributor

@vivien-apple Can you verify if CHIPTool still works fine? IIRC it just ignored the response and always call default success callback. Thanks.

I would assume that it just works fine looking at https://github.com/project-chip/connectedhomeip/blob/master/examples/chip-tool/commands/pairing/PairingCommand.cpp#L367 since the errorCode is just ignored as state in the comment.

One can verify that by running ./out/debug/standalone/chip-tool pairing ble-wifi SSID PASSWORD 0 20202021 3840 but I do not have an up to date accessory handy so I can't test "right now" but early next week...

@vivien-apple
Copy link
Contributor

vivien-apple commented Nov 19, 2021 •
edited
Loading

@vivien-apple Can you verify if CHIPTool still works fine? IIRC it just ignored the response and always call default success callback. Thanks.

I would assume that it just works fine looking at https://github.com/project-chip/connectedhomeip/blob/master/examples/chip-tool/commands/pairing/PairingCommand.cpp#L367 since the errorCode is just ignored as state in the comment.

One can verify that by running ./out/debug/standalone/chip-tool pairing ble-wifi SSID PASSWORD 0 20202021 3840 but I do not have an up to date accessory handy so I can't test "right now" but early next week...

I applied the patch locally and it just works.

I also tried to uncomment https://github.com/project-chip/connectedhomeip/blob/master/examples/chip-tool/commands/pairing/PairingCommand.cpp#L367 and https://github.com/project-chip/connectedhomeip/blob/master/examples/chip-tool/commands/pairing/PairingCommand.cpp#L400 and use an very long SSID just to trigger an error and it does work too

@tcarmelveilleux tcarmelveilleux merged commit dcca930 into project-chip:master Nov 19, 2021
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

@vivien-apple vivien-apple vivien-apple approved these changes

@yunhanw-google yunhanw-google yunhanw-google approved these changes

@msandstedt msandstedt msandstedt approved these changes

@tcarmelveilleux tcarmelveilleux tcarmelveilleux approved these changes

@andy31415 andy31415 Awaiting requested review from andy31415

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

@balducci-apple balducci-apple Awaiting requested review from balducci-apple

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

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

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chulspro chulspro Awaiting requested review from chulspro

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

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

@harimau-qirex harimau-qirex Awaiting requested review from harimau-qirex

@hawk248 hawk248 Awaiting requested review from hawk248

@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

@kghost kghost Awaiting requested review from kghost

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@LuDuda LuDuda Awaiting requested review from LuDuda

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@pan-apple pan-apple Awaiting requested review from pan-apple

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

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

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tecimovic tecimovic Awaiting requested review from tecimovic

@turon turon Awaiting requested review from turon

@wbschiller wbschiller Awaiting requested review from wbschiller

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

Assignees
No one assigned
Labels
app controller review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AddWifiNetwork should emit AddWiFiNetworkResponse
6 participants
@erjiaqing @vivien-apple @tcarmelveilleux @msandstedt @yunhanw-google @Damian-Nordic