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

Make sure to queue m5stack actions to the Matter thread as needed. #15066

Merged
merged 1 commit into from
Feb 11, 2022
Merged

Make sure to queue m5stack actions to the Matter thread as needed. #15066

merged 1 commit into from
Feb 11, 2022

Conversation

bzbarsky-apple
Copy link
Contributor

Fixes #15065

Problem

Touching Matter stuff on the wrong thread.

Change overview

Use the right thread.

Testing

Used the buttons, logs showed post-attribute-change callback now running on the right thread.

@github-actions
Copy link

github-actions bot commented Feb 11, 2022 •
edited
Loading

PR #15066: Size comparison from 681e9b9 to ee7a746

Increases (2 builds for esp32)
platform target config section 681e9b9 ee7a746 change % change
esp32 all-clusters-app c3devkit (read only) 943892 943970 78 0.0
(read/write) 1399738 1399802 64 0.0
.flash.rodata 198576 198640 64 0.0
.flash.text 943892 943970 78 0.0
m5stack (read only) 993659 993787 128 0.0
(read/write) 465008 465080 72 0.0
.flash.rodata 225672 225744 72 0.0
.flash.text 988275 988403 128 0.0
Full report (43 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 681e9b9 ee7a746 change % change
cyw30739 light cyw930739m2evb_01 (read/write) 607074 607074 0 0.0
.app_xip_area 508552 508552 0 0.0
.bss 81220 81220 0 0.0
.data 644 644 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 563778 563778 0 0.0
.app_xip_area 466824 466824 0 0.0
.bss 79692 79692 0 0.0
.data 608 608 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor cyw930739m2evb_01 (read/write) 584586 584586 0 0.0
.app_xip_area 478376 478376 0 0.0
.bss 88620 88620 0 0.0
.data 552 552 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 907632 907632 0 0.0
(read/write) 128976 128976 0 0.0
.bss 126944 126944 0 0.0
.data 2032 2032 0 0.0
.text 907624 907624 0 0.0
BRD4161A+rpc (read only) 895092 895092 0 0.0
(read/write) 145636 145636 0 0.0
.bss 143504 143504 0 0.0
.data 2132 2132 0 0.0
.text 895084 895084 0 0.0
window-app BRD4161A (read only) 842712 842712 0 0.0
(read/write) 127040 127040 0 0.0
.bss 125136 125136 0 0.0
.data 1900 1900 0 0.0
.text 842704 842704 0 0.0
esp32 all-clusters-app c3devkit (read only) 943892 943970 78 0.0
(read/write) 1399738 1399802 64 0.0
.dram0.bss 68000 68000 0 0.0
.dram0.data 14268 14268 0 0.0
.flash.rodata 198576 198640 64 0.0
.flash.text 943892 943970 78 0.0
.iram0.text 62056 62056 0 0.0
m5stack (read only) 993659 993787 128 0.0
(read/write) 465008 465080 72 0.0
.dram0.bss 73144 73144 0 0.0
.dram0.data 34064 34064 0 0.0
.flash.rodata 225672 225744 72 0.0
.flash.text 988275 988403 128 0.0
.iram0.text 123399 123399 0 0.0
k32w light k32w061+release (read/write) 684248 684248 0 0.0
.bss 78248 78248 0 0.0
.data 1904 1904 0 0.0
.text 598296 598296 0 0.0
lock k32w061+release (read/write) 689088 689088 0 0.0
.bss 78640 78640 0 0.0
.data 1948 1948 0 0.0
.text 602700 602700 0 0.0
linux all-clusters-app debug (read only) 2346265 2346265 0 0.0
(read/write) 141600 141600 0 0.0
.bss 56128 56128 0 0.0
.data 1296 1296 0 0.0
.data.rel.ro 78488 78488 0 0.0
.dynamic 592 592 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 920 920 0 0.0
.rodata 205253 205253 0 0.0
.text 1978786 1978786 0 0.0
bridge-app debug+rpc (read only) 1713925 1713925 0 0.0
(read/write) 85768 85768 0 0.0
.bss 40528 40528 0 0.0
.data 2034 2034 0 0.0
.data.rel.ro 38056 38056 0 0.0
.dynamic 592 592 0 0.0
.got 3976 3976 0 0.0
.init 27 27 0 0.0
.init_array 560 560 0 0.0
.rodata 141036 141036 0 0.0
.text 1456389 1456389 0 0.0
chip-tool debug (read only) 8241253 8241253 0 0.0
(read/write) 233816 233816 0 0.0
.bss 33416 33416 0 0.0
.data 1184 1184 0 0.0
.data.rel.ro 193176 193176 0 0.0
.dynamic 608 608 0 0.0
.got 4784 4784 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 456789 456789 0 0.0
.text 7300341 7300341 0 0.0
chip-tool-ipv6only arm64 (read only) 8064028 8064028 0 0.0
(read/write) 341185 341185 0 0.0
.bss 51681 51681 0 0.0
.data 1216 1216 0 0.0
.data.rel.ro 235544 235544 0 0.0
.dynamic 560 560 0 0.0
.got 48944 48944 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 433684 433684 0 0.0
.text 6901044 6901044 0 0.0
door-lock-app debug (read only) 1919081 1919081 0 0.0
(read/write) 112088 112088 0 0.0
.bss 43600 43600 0 0.0
.data 1010 1010 0 0.0
.data.rel.ro 62064 62064 0 0.0
.dynamic 592 592 0 0.0
.got 4112 4112 0 0.0
.init 27 27 0 0.0
.init_array 672 672 0 0.0
.rodata 172434 172434 0 0.0
.text 1598178 1598178 0 0.0
lighting-app debug+rpc (read only) 2043385 2043385 0 0.0
(read/write) 117368 117368 0 0.0
.bss 44576 44576 0 0.0
.data 1464 1464 0 0.0
.data.rel.ro 65832 65832 0 0.0
.dynamic 608 608 0 0.0
.got 4144 4144 0 0.0
.init 27 27 0 0.0
.init_array 720 720 0 0.0
.rodata 165937 165937 0 0.0
.text 1722658 1722658 0 0.0
ota-provider-app debug (read only) 1857841 1857841 0 0.0
(read/write) 107768 107768 0 0.0
.bss 43456 43456 0 0.0
.data 1224 1224 0 0.0
.data.rel.ro 57432 57432 0 0.0
.dynamic 608 608 0 0.0
.got 4368 4368 0 0.0
.init 27 27 0 0.0
.init_array 624 624 0 0.0
.rodata 157406 157406 0 0.0
.text 1549890 1549890 0 0.0
ota-requestor-app debug (read only) 1854209 1854209 0 0.0
(read/write) 108984 108984 0 0.0
.bss 43840 43840 0 0.0
.data 1192 1192 0 0.0
.data.rel.ro 58504 58504 0 0.0
.dynamic 592 592 0 0.0
.got 4176 4176 0 0.0
.init 27 27 0 0.0
.init_array 624 624 0 0.0
.rodata 151695 151695 0 0.0
.text 1553666 1553666 0 0.0
shell debug (read only) 2320025 2320025 0 0.0
(read/write) 144496 144496 0 0.0
.bss 64928 64928 0 0.0
.data 832 832 0 0.0
.data.rel.ro 73040 73040 0 0.0
.dynamic 592 592 0 0.0
.got 4168 4168 0 0.0
.init 27 27 0 0.0
.init_array 904 904 0 0.0
.rodata 206258 206258 0 0.0
.text 1957346 1957346 0 0.0
thermostat-no-ble arm64 (read only) 2140076 2140076 0 0.0
(read/write) 142209 142209 0 0.0
.bss 59105 59105 0 0.0
.data 1032 1032 0 0.0
.data.rel.ro 74960 74960 0 0.0
.dynamic 560 560 0 0.0
.got 4144 4144 0 0.0
.init 24 24 0 0.0
.init_array 328 328 0 0.0
.rodata 132460 132460 0 0.0
.text 1789504 1789504 0 0.0
tv-app debug (read only) 2515921 2515921 0 0.0
(read/write) 140960 140960 0 0.0
.bss 58752 58752 0 0.0
.data 3200 3200 0 0.0
.data.rel.ro 72984 72984 0 0.0
.dynamic 592 592 0 0.0
.got 4528 4528 0 0.0
.init 27 27 0 0.0
.init_array 888 888 0 0.0
.rodata 197292 197292 0 0.0
.text 2145714 2145714 0 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2396832 2396832 0 0.0
.bss 190068 190068 0 0.0
.data 5320 5320 0 0.0
.text 1359432 1359432 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2358456 2358456 0 0.0
.bss 182680 182680 0 0.0
.data 5624 5624 0 0.0
.text 1321056 1321056 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2321984 2321984 0 0.0
.bss 182568 182568 0 0.0
.data 5600 5600 0 0.0
.text 1284584 1284584 0 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1139648 1139648 0 0.0
.bss 11756 11756 0 0.0
.data 4368 4368 0 0.0
.text 103032 103032 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2308052 2308052 0 0.0
.bss 179820 179820 0 0.0
.data 5424 5424 0 0.0
.text 1270624 1270624 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 1015947 1015947 0 0.0
bss 123004 123004 0 0.0
rodata 119528 119528 0 0.0
text 694520 694520 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 984779 984779 0 0.0
bss 120192 120192 0 0.0
rodata 110896 110896 0 0.0
text 674132 674132 0 0.0
nrf52840dongle_nrf52840 (read/write) 1031775 1031775 0 0.0
bss 124392 124392 0 0.0
rodata 118392 118392 0 0.0
text 699108 699108 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 922690 922690 0 0.0
bss 119568 119568 0 0.0
rodata 112772 112772 0 0.0
text 609556 609556 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 945775 945775 0 0.0
bss 121376 121376 0 0.0
rodata 108276 108276 0 0.0
text 637520 637520 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 853250 853250 0 0.0
bss 117968 117968 0 0.0
rodata 101444 101444 0 0.0
text 553328 553328 0 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 527595 527595 0 0.0
bss 53632 53632 0 0.0
rodata 49976 49976 0 0.0
text 361016 361016 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 945187 945187 0 0.0
bss 121096 121096 0 0.0
rodata 107632 107632 0 0.0
text 637780 637780 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 940535 940535 0 0.0
bss 121100 121100 0 0.0
rodata 106924 106924 0 0.0
text 633816 633816 0 0.0
shell nrf52840dk_nrf52840 (read/write) 806207 806207 0 0.0
bss 112376 112376 0 0.0
rodata 78948 78948 0 0.0
text 537304 537304 0 0.0
p6 all-clusters-app default (read/write) 2482288 2482288 0 0.0
.bss 119688 119688 0 0.0
.data 2672 2672 0 0.0
.text 1440552 1440552 0 0.0
light-app default (read/write) 2388616 2388616 0 0.0
.bss 109336 109336 0 0.0
.data 2520 2520 0 0.0
.text 1346880 1346880 0 0.0
lock-app default (read/write) 2352072 2352072 0 0.0
.bss 109056 109056 0 0.0
.data 2480 2480 0 0.0
.text 1310336 1310336 0 0.0
qpg lighting-app qpg6105+debug (read only) 593688 593688 0 0.0
(read/write) 146936 146936 0 0.0
.bss 90520 90520 0 0.0
.data 1108 1108 0 0.0
.text 588368 588368 0 0.0
lock-app qpg6105+debug (read only) 559440 559440 0 0.0
(read/write) 146936 146936 0 0.0
.bss 90504 90504 0 0.0
.data 1060 1060 0 0.0
.text 554120 554120 0 0.0
persistent-storage-app qpg6105+debug (read only) 99520 99520 0 0.0
(read/write) 146940 146940 0 0.0
.bss 24004 24004 0 0.0
.data 176 176 0 0.0
.text 94200 94200 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 874846 874846 0 0.0
bss 90328 90328 0 0.0
noinit 37160 37160 0 0.0
text 613496 613496 0 0.0

@bzbarsky-apple bzbarsky-apple merged commit 379d144 into project-chip:master Feb 11, 2022
@bzbarsky-apple bzbarsky-apple deleted the fix-m5stack-threading branch February 11, 2022 15:52
wqx6
wqx6 reviewed Feb 14, 2022
View reviewed changes
examples/all-clusters-app/esp32/main/CHIPDeviceManager.cpp
@@ -99,6 +99,13 @@ CHIP_ERROR CHIPDeviceManager::Init(CHIPDeviceManagerCallbacks * cb)
void MatterPostAttributeChangeCallback(const chip::app::ConcreteAttributePath & path, uint8_t mask, uint8_t type, uint16_t size,
uint8_t * value)
{
TaskHandle_t task = xTaskGetCurrentTaskHandle();
const char * name = pcTaskGetName(task);
if (!strcmp(name, "CHIP"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be if(strcmp(name, "CHIP"))? I got the error logs when I ran all-clusters-app on esp32. Looks like this check is incorrect.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Looks like :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Er, yes, this is backwards. Will create PR to fix as soon as ToT is not failing CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#15187

mspang pushed a commit to mspang/connectedhomeip that referenced this pull request Feb 16, 2022
@bzbarsky-apple @mspang
Make sure to queue m5stack actions to the Matter thread as needed. (p…
ef1aee2 
…roject-chip#15066)

Fixes project-chip#15065

(cherry picked from commit 379d144)
mspang added a commit that referenced this pull request Feb 16, 2022
@mspang @bzbarsky-apple
Make sure to queue m5stack actions to the Matter thread as needed. (#…
f3f994e 
…15066) (#15234)

Fixes #15065

(cherry picked from commit 379d144)

Co-authored-by: Boris Zbarsky <[email protected]>
@dhrishi dhrishi mentioned this pull request Feb 16, 2022
Closed
jamesluo11 pushed a commit to jamesluo11/connectedhomeip that referenced this pull request Apr 26, 2022
@bzbarsky-apple @fuxingguo16
Make sure to queue m5stack actions to the Matter thread as needed. (p…
7e2ed21 
…roject-chip#15066)

Fixes project-chip#15065
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wqx6 wqx6 wqx6 left review comments

@dhrishi dhrishi dhrishi approved these changes

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

@mlepage-google mlepage-google Awaiting requested review from mlepage-google

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

@austinh0 austinh0 Awaiting requested review from austinh0

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

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

@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

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@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

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@lzgrablic02 lzgrablic02 Awaiting requested review from lzgrablic02

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@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

@vijs vijs Awaiting requested review from vijs

@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
examples review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Using buttons on m5stack all-clusters-app calls into Matter stack in a thread-unsafe way
4 participants
@bzbarsky-apple @dhrishi @wqx6 @vivien-apple