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

[Darwin] Implement multiple read and subscribe #25840

Conversation

joonhaengHeo
Copy link
Contributor

Fixes #25424

  1. Implement multiple read attribute or event.
  2. Implement multiple subscribe attribute or event.

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.

This is very very close! Thank you for moving the code (made it much easier to review) and adding the tests!

The only real issue here that still needs to get sorted out is the memory leak on error in the read function.

src/darwin/Framework/CHIP/MTRBaseDevice.mm Outdated Show resolved Hide resolved
src/darwin/Framework/CHIP/MTRBaseDevice.mm Show resolved Hide resolved
src/darwin/Framework/CHIP/MTRBaseDevice.mm Show resolved Hide resolved
src/darwin/Framework/CHIP/MTRBaseDevice.mm Outdated Show resolved Hide resolved
src/darwin/Framework/CHIP/MTRBaseDevice.mm Outdated Show resolved Hide resolved
@github-actions
Copy link

PR #25840: Size comparison from 76eaffc to 3619342

Increases (1 build for cc32xx)
platform target config section 76eaffc 3619342 change % change
cc32xx lock CC3235SF_LAUNCHXL .debug_info 20326060 20328478 2418 0.0
.debug_line 2687403 2687417 14 0.0
.debug_loc 2838361 2838391 30 0.0
.debug_str 3042379 3042559 180 0.0
Full report (1 build for cc32xx)
platform target config section 76eaffc 3619342 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 643081 643081 0 0.0
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 933224 933224 0 0.0
.debug_aranges 87760 87760 0 0.0
.debug_frame 302028 302028 0 0.0
.debug_info 20326060 20328478 2418 0.0
.debug_line 2687403 2687417 14 0.0
.debug_loc 2838361 2838391 30 0.0
.debug_ranges 288040 288040 0 0.0
.debug_str 3042379 3042559 180 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 104393 104393 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 377626 377626 0 0.0
.symtab 256832 256832 0 0.0
.text 536568 536568 0 0.0

src/darwin/Framework/CHIP/MTRBaseDevice.mm Outdated Show resolved Hide resolved
src/darwin/Framework/CHIP/MTRBaseDevice.mm Outdated Show resolved Hide resolved
src/darwin/Framework/CHIP/MTRBaseDevice.mm Outdated Show resolved Hide resolved
src/darwin/Framework/CHIP/MTRBaseDevice.mm Outdated Show resolved Hide resolved
src/darwin/Framework/CHIP/MTRBaseDevice.mm Show resolved Hide resolved
src/darwin/Framework/CHIP/MTRBaseDevice.mm Outdated Show resolved Hide resolved
src/darwin/Framework/CHIP/MTRBaseDevice.mm Outdated Show resolved Hide resolved
@joonhaengHeo joonhaengHeo force-pushed the implement_darwin_multiple_read_subscribe branch from f676acb to d493f31 Compare April 18, 2023 00:22
@github-actions
Copy link

PR #25840: Size comparison from 8809801 to d493f31

Decreases (2 builds for nrfconnect, qpg)
platform target config section 8809801 d493f31 change % change
nrfconnect all-clusters-app nrf52840dk_nrf52840 text 804836 804832 -4 -0.0
qpg lighting-app qpg6105+debug (read/write) 1166568 1166560 -8 -0.0
.text 613668 613660 -8 -0.0
Full report (6 builds for mbed, nrfconnect, qpg)
platform target config section 8809801 d493f31 change % change
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2466336 2466336 0 0.0
.bss 215960 215960 0 0.0
.data 5888 5888 0 0.0
.text 1428980 1428980 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read only) 4 4 0 0.0
(read/write) 1173892 1173892 0 0.0
bss 155557 155557 0 0.0
rodata 132740 132740 0 0.0
text 804836 804832 -4 -0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read only) 4 4 0 0.0
(read/write) 1119140 1119140 0 0.0
bss 154713 154713 0 0.0
rodata 109556 109556 0 0.0
text 774228 774228 0 0.0
all-clusters-app nrf7002dk_nrf5340_cpuapp (read only) 4 4 0 0.0
(read/write) 1433952 1433952 0 0.0
bss 135297 135297 0 0.0
rodata 228784 228784 0 0.0
text 775788 775788 0 0.0
qpg lighting-app qpg6105+debug (read/write) 1166568 1166560 -8 -0.0
.bss 99308 99308 0 0.0
.data 856 856 0 0.0
.text 613668 613660 -8 -0.0
lock-app qpg6105+debug (read/write) 1136688 1136688 0 0.0
.bss 94452 94452 0 0.0
.data 860 860 0 0.0
.text 583784 583784 0 0.0

@bzbarsky-apple
Copy link
Contributor

Fast-tracking platform-specific change with platform owner review.

@github-actions
Copy link

PR #25840: Size comparison from 8809801 to 995d77f

Full report (3 builds for mbed, qpg)
platform target config section 8809801 995d77f change % change
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2466336 2466336 0 0.0
.bss 215960 215960 0 0.0
.data 5888 5888 0 0.0
.text 1428980 1428980 0 0.0
qpg lighting-app qpg6105+debug (read/write) 1166568 1166568 0 0.0
.bss 99308 99308 0 0.0
.data 856 856 0 0.0
.text 613668 613668 0 0.0
lock-app qpg6105+debug (read/write) 1136688 1136688 0 0.0
.bss 94452 94452 0 0.0
.data 860 860 0 0.0
.text 583784 583784 0 0.0

@bzbarsky-apple bzbarsky-apple merged commit 4ccbbb7 into project-chip:master Apr 18, 2023
@bzbarsky-apple
Copy link
Contributor

@joonhaengHeo Thank you for the fix, and your patience!

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.

How to read multiple attribute in darwin platform
2 participants