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

[Descriptor]: Support read attribute with complex type via AttributeAccessInterface for descriptor cluster #10374

Merged
merged 3 commits into from
Oct 12, 2021
Merged

[Descriptor]: Support read attribute with complex type via AttributeAccessInterface for descriptor cluster #10374

merged 3 commits into from
Oct 12, 2021

Conversation

yufengwangca
Copy link
Contributor

@yufengwangca yufengwangca commented Oct 11, 2021 •
edited
Loading

Problem

What is being fixed? Examples:

  • Currently, our SDK does not support read attribute with complex type such as list well. For descriptor cluster, right now we are putting these lists in the attr store. But we want to stop doing that and just create them on-the-fly using AttributeAccessInterface when read.

Change overview

Support read attribute with complex type via AttributeAccessInterface for descriptor cluster.

Testing

How was this tested? (at least one bullet point required)

  • The client side change is still in progress, the server side change is temperately disabled by flag CHIP_CLUSTER_CONFIG_ENABLE_COMPLEX_ATTRIBUTE_READ. We will enable this flag until the client-side bits get done.
  • The test is conducted with test client side change.
chip-device-ctrl > 
chip-device-ctrl > zclread Descriptor PartsList 12344321 0 0
[1633915768.491543][429879:429887] CHIP:IN: Prepared encrypted message 0x23269a0 to 0x0000000000BC5C01 of type 0x2 and protocolId (0, 1) on exchange 7561i with MessageCounter:2.
[1633915768.491639][429879:429887] CHIP:IN: Sending encrypted msg 0x23269a0 with MessageCounter:2 to 0x0000000000BC5C01 at monotonic time: 196182278 msec
[1633915768.728309][429879:429887] CHIP:EM: Received message of type 0x5 with protocolId (0, 1) and MessageCounter:2 on exchange 7561i
[1633915768.729738][429879:429887] CHIP:ZCL: ReadAttributesResponse:
[1633915768.729788][429879:429887] CHIP:ZCL:   ClusterId: 0x0000_001D
[1633915768.729816][429879:429887] CHIP:ZCL:   attributeId: 0x0000_0003
[1633915768.729837][429879:429887] CHIP:ZCL:   status: Success                (0x0000)
[1633915768.729856][429879:429887] CHIP:ZCL:   attribute TLV Type: 0x16
[1633915768.730152][429879:429887] CHIP:IN: Prepared encrypted message 0x23269a0 to 0x0000000000BC5C01 of type 0x1 and protocolId (0, 1) on exchange 7561i with MessageCounter:3.
[1633915768.730233][429879:429887] CHIP:IN: Sending encrypted msg 0x23269a0 with MessageCounter:3 to 0x0000000000BC5C01 at monotonic time: 196182516 msec
AttributeReadResult(path=AttributePath(nodeId=12344321, endpointId=0, clusterId=29, attributeId=3), status=0, value=[0, 1, 2])
chip-device-ctrl > [1633915768.737316][429879:429887] CHIP:EM: Received message of type 0x10 with protocolId (0, 0) and MessageCounter:3 on exchange 7561i


chip-device-ctrl > zclread Descriptor ServerList 12344321 0 0
[1633915868.486598][429879:429887] CHIP:IN: Prepared encrypted message 0x23269a0 to 0x0000000000BC5C01 of type 0x2 and protocolId (0, 1) on exchange 7562i with MessageCounter:4.
[1633915868.486698][429879:429887] CHIP:IN: Sending encrypted msg 0x23269a0 with MessageCounter:4 to 0x0000000000BC5C01 at monotonic time: 196282273 msec
[1633915868.505656][429879:429887] CHIP:EM: Received message of type 0x5 with protocolId (0, 1) and MessageCounter:4 on exchange 7562i
[1633915868.506061][429879:429887] CHIP:ZCL: ReadAttributesResponse:
[1633915868.506074][429879:429887] CHIP:ZCL:   ClusterId: 0x0000_001D
[1633915868.506083][429879:429887] CHIP:ZCL:   attributeId: 0x0000_0001
[1633915868.506088][429879:429887] CHIP:ZCL:   status: Success                (0x0000)
[1633915868.506094][429879:429887] CHIP:ZCL:   attribute TLV Type: 0x16
[1633915868.506181][429879:429887] CHIP:IN: Prepared encrypted message 0x23269a0 to 0x0000000000BC5C01 of type 0x1 and protocolId (0, 1) on exchange 7562i with MessageCounter:5.
[1633915868.506203][429879:429887] CHIP:IN: Sending encrypted msg 0x23269a0 with MessageCounter:5 to 0x0000000000BC5C01 at monotonic time: 196282292 msec
AttributeReadResult(path=AttributePath(nodeId=12344321, endpointId=0, clusterId=29, attributeId=1), status=0, value=[29, 40, 48, 49, 50, 51, 52, 53, 54, 55, 60, 62])
chip-device-ctrl > [1633915868.515807][429879:429887] CHIP:EM: Received message of type 0x10 with protocolId (0, 0) and MessageCounter:5 on exchange 7562i

chip-device-ctrl > zclread Descriptor ClientList 12344321 0 0
[1633915885.193521][429879:429887] CHIP:IN: Prepared encrypted message 0x23269a0 to 0x0000000000BC5C01 of type 0x2 and protocolId (0, 1) on exchange 7563i with MessageCounter:6.
[1633915885.193555][429879:429887] CHIP:IN: Sending encrypted msg 0x23269a0 with MessageCounter:6 to 0x0000000000BC5C01 at monotonic time: 196298980 msec
[1633915885.463107][429879:429887] CHIP:EM: Received message of type 0x5 with protocolId (0, 1) and MessageCounter:6 on exchange 7563i
[1633915885.463915][429879:429887] CHIP:ZCL: ReadAttributesResponse:
[1633915885.463964][429879:429887] CHIP:ZCL:   ClusterId: 0x0000_001D
[1633915885.463993][429879:429887] CHIP:ZCL:   attributeId: 0x0000_0002
[1633915885.464012][429879:429887] CHIP:ZCL:   status: Success                (0x0000)
[1633915885.464042][429879:429887] CHIP:ZCL:   attribute TLV Type: 0x16
[1633915885.464304][429879:429887] CHIP:IN: Prepared encrypted message 0x23269a0 to 0x0000000000BC5C01 of type 0x1 and protocolId (0, 1) on exchange 7563i with MessageCounter:7.
[1633915885.464371][429879:429887] CHIP:IN: Sending encrypted msg 0x23269a0 with MessageCounter:7 to 0x0000000000BC5C01 at monotonic time: 196299251 msec
AttributeReadResult(path=AttributePath(nodeId=12344321, endpointId=0, clusterId=29, attributeId=2), status=0, value=[])
chip-device-ctrl > [1633915885.477337][429879:429887] CHIP:EM: Received message of type 0x10 with protocolId (0, 0) and MessageCounter:7 on exchange 7563i

chip-device-ctrl > zclread Descriptor DeviceList 12344321 0 0
[1633915924.126135][429879:429887] CHIP:IN: Prepared encrypted message 0x23269a0 to 0x0000000000BC5C01 of type 0x2 and protocolId (0, 1) on exchange 7564i with MessageCounter:8.
[1633915924.126244][429879:429887] CHIP:IN: Sending encrypted msg 0x23269a0 with MessageCounter:8 to 0x0000000000BC5C01 at monotonic time: 196337912 msec
[1633915924.204321][429879:429887] CHIP:EM: Received message of type 0x5 with protocolId (0, 1) and MessageCounter:8 on exchange 7564i
[1633915924.205390][429879:429887] CHIP:ZCL: ReadAttributesResponse:
[1633915924.205441][429879:429887] CHIP:ZCL:   ClusterId: 0x0000_001D
[1633915924.205480][429879:429887] CHIP:ZCL:   attributeId: 0x0000_0000
[1633915924.205500][429879:429887] CHIP:ZCL:   status: Success                (0x0000)
[1633915924.205526][429879:429887] CHIP:ZCL:   attribute TLV Type: 0x16
[1633915924.205803][429879:429887] CHIP:IN: Prepared encrypted message 0x23269a0 to 0x0000000000BC5C01 of type 0x1 and protocolId (0, 1) on exchange 7564i with MessageCounter:9.
[1633915924.205872][429879:429887] CHIP:IN: Sending encrypted msg 0x23269a0 with MessageCounter:9 to 0x0000000000BC5C01 at monotonic time: 196337992 msec
AttributeReadResult(path=AttributePath(nodeId=12344321, endpointId=0, clusterId=29, attributeId=0), status=0, value=[{0: 0, 1: 1}])
chip-device-ctrl > [1633915924.264006][429879:429887] CHIP:EM: Received message of type 0x10 with protocolId (0, 0) and MessageCounter:9 on exchange 7564i

bzbarsky-apple
bzbarsky-apple requested changes Oct 11, 2021
View reviewed changes
examples/lighting-app/lighting-common/lighting-app.zap Show resolved Hide resolved
src/app/clusters/descriptor/descriptor.cpp Outdated Show resolved Hide resolved
src/app/clusters/descriptor/descriptor.cpp Outdated Show resolved Hide resolved
src/app/clusters/descriptor/descriptor.cpp Show resolved Hide resolved
src/app/clusters/descriptor/descriptor.cpp Show resolved Hide resolved
src/app/clusters/descriptor/descriptor.cpp Outdated Show resolved Hide resolved
src/app/clusters/descriptor/descriptor.cpp Show resolved Hide resolved
bzbarsky-apple
bzbarsky-apple reviewed Oct 11, 2021
View reviewed changes
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Oh, one other issue: with this change, reporting is broken. The old setup has the emberAfPluginDescriptorServerInitCallback call in emberAfEndpointEnableDisable which updates the stored values for the new topology. But in the new setup, that's not going to do anything once we stop writing to the attr store from the init callback. Instead, we need something to prod the reporting machinery when the set of enabled endpoints changes.

It's probably ok to have a followup for this so we can unblock the non-reporting use cases....

src/app/clusters/descriptor/descriptor.cpp Show resolved Hide resolved
@boring-cyborg boring-cyborg bot added the darwin label Oct 12, 2021
bzbarsky-apple
bzbarsky-apple reviewed Oct 12, 2021
View reviewed changes
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 looks great, but the submodule changes should not be in this PR.

src/app/clusters/descriptor/descriptor.cpp Show resolved Hide resolved
src/app/clusters/descriptor/descriptor.cpp Show resolved Hide resolved
src/app/clusters/descriptor/descriptor.cpp Outdated Show resolved Hide resolved
src/app/tests/suites/TestDescriptorCluster.yaml Show resolved Hide resolved
@github-actions
Copy link

Size increase report for "gn_qpg-example-build" from add5a11

File Section File VM
chip-qpg6100-lighting-example.out .text 2660 2660
chip-qpg6100-lighting-example.out .bss 0 1032
chip-qpg6100-lighting-example.out .data 4 4
chip-qpg6100-lighting-example.out .heap 0 -1040
Full report output 
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
.debug_info,0,86404
.debug_line,0,6594
.debug_loc,0,4151
.debug_abbrev,0,4067
.text,2660,2660
.debug_str,0,2392
.debug_ranges,0,1496
.bss,1032,0
.strtab,0,854
.symtab,0,848
.debug_frame,0,380
.debug_aranges,0,120
[ELF Program Headers],0,32
.data,4,4
.shstrtab,0,-2
.heap,-1040,0
[Unmapped],0,-2692

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'

@pullapprove pullapprove bot requested a review from wbschiller October 12, 2021 22:08
@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from add5a11

File Section File VM
chip-lock.elf text 1420 1420
chip-lock.elf rodata 1208 1208
chip-lock.elf bss 0 1052
chip-lock.elf init_array 4 4
chip-lock.elf [LOAD #3 [RW]] 0 -28
Full report output 
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.debug_info,0,88299
.debug_line,0,6201
.debug_abbrev,0,4240
.debug_loc,0,4190
.debug_str,0,2378
.debug_ranges,0,1496
text,1420,1420
rodata,1208,1208
bss,1052,0
.strtab,0,854
.symtab,0,832
.debug_frame,0,380
.debug_aranges,0,120
init_array,4,4
.shstrtab,0,2
[LOAD #3 [RW]],-28,0

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize

@github-actions
Copy link

Size increase report for "esp32-example-build" from add5a11

File Section File VM
chip-all-clusters-app.elf .flash.text 608 608
chip-all-clusters-app.elf .flash.rodata 24 24
chip-all-clusters-app.elf .dram0.bss 0 16
Full report output 
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_info,0,27657
.debug_str,0,1866
.debug_loc,0,1828
.debug_line,0,1717
.debug_ranges,0,1152
.debug_abbrev,0,1075
.flash.text,608,608
.strtab,0,546
.debug_frame,0,152
.symtab,0,144
.debug_aranges,0,40
.flash.rodata,24,24
.dram0.bss,16,0
.shstrtab,0,2
.riscv.attributes,0,1
[Unmapped],0,-632

@yufengwangca yufengwangca merged commit b7a6cb3 into project-chip:master Oct 12, 2021
@yufengwangca yufengwangca deleted the pr/cluster/descriptor branch October 14, 2021 04:24
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this pull request Oct 15, 2021
@bzbarsky-apple
Fix build failures due to merge conflicts.
b964dd6 
project-chip#10397 modified
APIs that were being used in new places at the same time in
project-chip#10459,
project-chip#10362,
project-chip#10464, and
project-chip#10374, so now the
tree does not compile.
@bzbarsky-apple bzbarsky-apple mentioned this pull request Oct 15, 2021
Merged
andy31415 pushed a commit that referenced this pull request Oct 15, 2021
@bzbarsky-apple
Fix build failures due to merge conflicts. (#10572)
efc17de 
#10397 modified
APIs that were being used in new places at the same time in
#10459,
#10362,
#10464, and
#10374, so now the
tree does not compile.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrjerryjohns mrjerryjohns mrjerryjohns left review comments

@jmartinez-silabs jmartinez-silabs jmartinez-silabs approved these changes

@jmeg-sfy jmeg-sfy jmeg-sfy approved these changes

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

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

@balducci-apple balducci-apple Awaiting requested review from balducci-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

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

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

@hawk248 hawk248 Awaiting requested review from hawk248

@holbrookt holbrookt Awaiting requested review from holbrookt

@jelderton jelderton Awaiting requested review from jelderton

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

@kghost kghost Awaiting requested review from kghost

@LuDuda LuDuda Awaiting requested review from LuDuda

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@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

@shana-apple shana-apple Awaiting requested review from shana-apple

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@turon turon Awaiting requested review from turon

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

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

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

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

@wbschiller wbschiller Awaiting requested review from wbschiller

Assignees
No one assigned
Labels
app darwin examples lib review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@yufengwangca @mrjerryjohns @yunhanw-google @woody-apple @bzbarsky-apple @jmartinez-silabs @jmeg-sfy