Add and Enable IM subscription support #9510

yunhanw-google · 2021-09-07T16:56:47Z

Problem

Follow IM spec chapter 5 and IM encoding spec 7.3 and 7.4 to achieve
subscription functionality.
https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/master/src/data_model/Interaction-Model.adoc#5-subscribe-interaction
https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/master/src/data_model/Encoding-Specification.adoc#subscriberequestmessage
#7978 #7979

Change overview

Establishing distinct subscriptions and tracking their respective configurations from multiple clients simultaneously.
Sharing of reporting logic between reads and subscriptions to maximize re-use.
Multiple paths per subscription
Automatic liveness assessment on client and server and termination of subscription accordingly.
Eventing support in addition to attribute reporting.
Spec compliance to the encoding specification with TLV-based payloads.
Preservation of the application level APIs for client-side reporting, but with a swap of the internals to use the new machinery
Update templates to use interaction model APIs for sending subscribe
requests.
Update DeviceControllerInteractionModelDelegate to handle report
messages from subscription.
Update ChipCallback Mgr to bridge subscribe responses to existing
callbacks, and handle TLV message just as read responses.

Testing

Update TestSuite for basic Subscription protocol tests, test routing
looks like follows:
Send subscribe request, wait for response.
Wait for a few seconds, and execute "kick" commands.
The test is expected to receive the same number of Reports as the
number of kick commands.
The test will fail when not receiving enough report data from server
in global timeout.
Update python script test to (in cirque) include a similar test
routine as test suite.
Add unit test for positive and negative subscribe test
Add integrated cirque test for subscribe

andy31415 · 2021-09-07T17:01:37Z

Marked discussion required - this is one of the proposals for subscription support (based on read extension to support subscriptions)

woody-apple · 2021-09-08T16:28:56Z

Per discussion today this is now SDK Approved, and we should proceed with this path forward. Closing the alternative proposal here: #9466

src/app/InteractionModelEngine.cpp

src/app/ReadClient.cpp

--Extend IM ReadHandler with subscribe capability, and update the corresponding reporting engine, which can process multiple subscriber, process subscribe request and generate subscribe response. --Hook setDirty API to ember so that ember can generate the change set and send it out via IM report.

yunhanw-google · 2021-09-13T17:31:31Z

@vivien-apple @erjiaqing has updated the test, any further concern, thanks

vivien-apple · 2021-09-13T17:57:38Z

Here is how I think we should write the test:

name: Subscribe Tests - OnOff Cluster

 config:
     cluster: "On/Off"
     endpoint: 1

 tests:
     - label: "Set OnOff Attribute to false"
       command: "Off"

     - label: "Subscribe OnOff Attribute"
       command: "subscribeAttribute"
       attribute: "OnOff"
       minInterval: 2
       maxInterval: 10
       response:
            value: false

     - label: "Set OnOff Attribute to true"
       command: "On"

     - label: "Wait for OnOff attribute report: True"
       command: "waitForAttributeReport"
       attribute: "OnOff"
       response:
           value: true

     - label: "Set OnOff Attribute to false"
       command: "Off"

     - label: "Wait for OnOff attribute report: False"
       command: "waitForAttributeReport"
       attribute: "OnOff"
       response:
           value: false

I have changed a little bit the syntax for "waitForAttributeReport" as it can probably be done without having to resort to touching the "simulated" cluster bits and that would let the command inherited cluster and endpoint

--Extends IM read client with subscribe capability and it can send subscribe request and process subscribe process and further maintain subscription with livess check. --Disable path filter for IM read/subscribe request. --Update templates to use interaction model APIs for sending subscribe requests. --Update DeviceControllerInteractionModelDelegate to handle report messages from subscription. --Update ChipCallback Mgr to bridge subscribe responses to existing callbacks, and handle TLV message just as read responses. --Update TestSuite for basic Subscription protocol tests, test routing looks like follows: Send subscribe request, wait for response. Wait for a few seconds, and execute "kick" commands. The test is expected to receive the same number of Reports as the number of kick commands. The test will fail when not receiving enough report data from server in global timeout. --Update python script test to (in cirque) include a similar test routine as test suite. --Add unit test for positive and negative subscribe test --Add integrated cirque test for subscribe

github-actions · 2021-09-13T18:33:44Z

Size increase report for "gn_qpg-example-build" from 92d3a28

File	Section	File	VM
chip-qpg6100-lighting-example.out	.text	3636	3636
chip-qpg6100-lighting-example.out	.bss	0	416
chip-qpg6100-lighting-example.out	.heap	0	-416

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

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'

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

sections,vmsize,filesize
.debug_info,0,87368
.debug_loc,0,18493
.debug_line,0,16853
.debug_str,0,5982
.debug_abbrev,0,5972
.text,3636,3636
.debug_ranges,0,2800
.strtab,0,2189
.debug_frame,0,1716
.symtab,0,1248
.debug_aranges,0,488
.bss,416,0
.shstrtab,0,-1
.heap,-416,0
[Unmapped],0,-3636

github-actions · 2021-09-13T19:07:49Z

Size increase report for "nrfconnect-example-build" from 92d3a28

File	Section	File	VM
chip-lock.elf	text	3296	3296
chip-lock.elf	rodata	432	432
chip-lock.elf	bss	0	416

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,90909
.debug_loc,0,18453
.debug_line,0,16275
.debug_abbrev,0,6304
.debug_str,0,5899
text,3296,3296
.debug_ranges,0,3072
.strtab,0,2189
.debug_frame,0,1688
.symtab,0,1264
.debug_aranges,0,472
rodata,432,432
bss,416,0
.shstrtab,0,-1

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

sections,vmsize,filesize

github-actions · 2021-09-13T19:24:16Z

Size increase report for "esp32-example-build" from 92d3a28

File	Section	File	VM
chip-lock-app.elf	.flash.text	3528	3528
chip-lock-app.elf	.flash.rodata	1000	1000
chip-lock-app.elf	.dram0.bss	0	424
chip-all-clusters-app.elf	.flash.text	3844	3844
chip-all-clusters-app.elf	.flash.rodata	1016	1016
chip-all-clusters-app.elf	.dram0.bss	0	416
chip-shell.elf	.flash.text	48	48
chip-temperature-measurement-app.elf	.flash.text	3392	3392
chip-temperature-measurement-app.elf	.flash.rodata	560	560
chip-temperature-measurement-app.elf	.dram0.bss	0	424
chip-ipv6only-app.elf	.flash.text	172	172
chip-bridge-app.elf	.flash.text	3596	3596
chip-bridge-app.elf	.flash.rodata	1000	1000
chip-bridge-app.elf	.dram0.bss	0	424

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,154385
.debug_line,0,26802
.debug_loc,0,14140
.debug_abbrev,0,7891
.debug_str,0,5934
.flash.text,3528,3528
.debug_ranges,0,2464
.strtab,0,2296
.debug_frame,0,1376
.flash.rodata,1000,1000
.symtab,0,560
.debug_aranges,0,496
.dram0.bss,424,0
.xt.prop._ZNK4chip3app11ClusterInfo25IsAttributePathSupersetOfERKS1_,0,216
.shstrtab,0,68
[ELF Section Headers],0,40
[6 Others],0,20
.xt.prop._ZN4chip3app9EventList7BuilderC5Ev,0,12
.xt.prop._ZN4chip3app18CommandDataElement7BuilderC5Ev,0,-12
.xt.prop._ZN4chip6System27TLVPacketBufferBackingStoreD5Ev,0,-12
[Unmapped],0,-432

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

sections,vmsize,filesize
.debug_info,0,142717
.debug_line,0,21136
.debug_loc,0,13070
.debug_abbrev,0,7757
.debug_str,0,5947
.flash.text,3844,3844
.debug_ranges,0,2640
.strtab,0,2295
.debug_frame,0,1916
.flash.rodata,1016,1016
.symtab,0,544
.debug_aranges,0,496
.dram0.bss,416,0
.riscv.attributes,0,1
.shstrtab,0,1
[Unmapped],0,-764

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

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

sections,vmsize,filesize
.flash.text,48,48
[Unmapped],0,-48

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
.debug_info,0,97037
.debug_line,0,25924
.debug_loc,0,13946
.debug_abbrev,0,7005
.debug_str,0,5936
.flash.text,3392,3392
.debug_ranges,0,2376
.strtab,0,2213
.debug_frame,0,1376
.flash.rodata,560,560
.symtab,0,528
.debug_aranges,0,496
.dram0.bss,424,0
[Unmapped],0,144
.xt.prop._ZN4chip3app9EventList7BuilderC5Ev,0,12
.xt.lit._ZN4chip3app9EventList7BuilderC5Ev,0,8
[1 Others],0,-1
.xt.lit._ZN4chip3app18CommandDataElement7BuilderC5Ev,0,-8
.xt.lit._ZN4chip6System27TLVPacketBufferBackingStoreD5Ev,0,-8
.xt.prop._ZN4chip3app18CommandDataElement7BuilderC5Ev,0,-12
.xt.prop._ZN4chip6System27TLVPacketBufferBackingStoreD5Ev,0,-12

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize
[Unmapped],0,3924
.flash.text,172,172

Comparing ./master_artifact/chip-bridge-app.elf and ./pull_artifact/chip-bridge-app.elf:

sections,vmsize,filesize
.debug_info,0,97651
.debug_line,0,26662
.debug_loc,0,14384
.debug_abbrev,0,7009
.debug_str,0,5946
.flash.text,3596,3596
.debug_ranges,0,2464
.strtab,0,2285
.debug_frame,0,1376
.flash.rodata,1000,1000
.symtab,0,560
.debug_aranges,0,496
.dram0.bss,424,0
.xt.prop._ZNK4chip3app11ClusterInfo25IsAttributePathSupersetOfERKS1_,0,216
.shstrtab,0,67
[ELF Section Headers],0,40
.xt.prop._ZN4chip3app9EventList7BuilderC5Ev,0,12
.xt.prop._ZN4chip3app18CommandDataElement7BuilderC5Ev,0,-12
.xt.prop._ZN4chip6System27TLVPacketBufferBackingStoreD5Ev,0,-12
[Unmapped],0,-500

hubTab · 2021-09-15T15:09:21Z

src/app/ReadClient.h

    bool IsAwaitingInitialReport() const { return mState == ClientState::AwaitingInitialReport; }
+    bool IsAwaitingSubscribeResponse() const { return mState == ClientState::AwaitingSubscribeResponse; }
+
    CHIP_ERROR GenerateEventPathList(EventPathList::Builder & aEventPathListBuilder, EventPathParams * apEventPathParamsList,
                                     size_t aEventPathParamsListSize);
    CHIP_ERROR GenerateAttributePathList(AttributePathList::Builder & aAttributeathListBuilder,


SmartThings is very excited to start integrating Subscription Interactions, and this PR is an excellent path forward.
In fact, we have started already using the interactions, by using the Read Interaction, which is based on lib/connectedhomeip/src/app/tests. However, we run into a couple of issues with the ReadClient class:

Currently ReadClient.h has GenerateAttributePathList(), ProcessReportData() and the destructor (virtual ~ReadClient()) marked as private, making them inaccessible for use outside the class. To get past this hurdle, we have declared them as public so that our application can use the ReadClient class.

I suggest setting them up as public, so that the SDK consumer can use the ReadClient class, in a similar way the TestReadInteraction (as it is set as a friend class, but this would not be a path moving forward for custom code). Or if this is not preferable, we are open to alternatives that you can suggest so Read Interaction development can continue.

… remove some dead code introduced in project-chip#9510

… remove some dead code introduced in #9510 (#9913) * Rename ConfigureAttribute to SubscribeAttribute, remove isAnalog, and remove some dead code introduced in #9510 * Update generated content

yunhanw-google requested review from andy31415, msandstedt, erjiaqing, mrjerryjohns, yufengwangca, woody-apple, pan-apple, bzbarsky-apple, vivien-apple and jmartinez-silabs September 7, 2021 16:56

boring-cyborg bot added app examples labels Sep 7, 2021

pullapprove bot requested review from chrisdecenzo, jelderton and LuDuda September 7, 2021 16:57

pullapprove bot added the review - pending label Sep 7, 2021

andy31415 added the SDK Discussion Required label Sep 7, 2021

yunhanw-google force-pushed the feature/add_subscribe_client_handler7 branch 6 times, most recently from bc5c5fe to a2c5b8f Compare September 8, 2021 15:26

woody-apple added SDK Approved and removed SDK Discussion Required labels Sep 8, 2021

woody-apple mentioned this pull request Sep 8, 2021

Provide destination and endpoint while requesting for reports #9466

Closed

yunhanw-google force-pushed the feature/add_subscribe_client_handler7 branch from a2c5b8f to 7e0f053 Compare September 8, 2021 16:53

andy31415 approved these changes Sep 8, 2021

View reviewed changes

src/app/InteractionModelEngine.cpp Outdated Show resolved Hide resolved

src/app/InteractionModelEngine.cpp Outdated Show resolved Hide resolved

src/app/ReadClient.cpp Outdated Show resolved Hide resolved

pullapprove bot added the review - approved label Sep 10, 2021

yunhanw-google force-pushed the feature/add_subscribe_client_handler7 branch 3 times, most recently from 16e7e2d to 987a2d3 Compare September 10, 2021 22:10

msandstedt added the technical review - DONE label Sep 13, 2021

erjiaqing and others added 2 commits September 13, 2021 10:07

[testsuite] Refactor test suite simulated cluster command

b285218

yunhanw-google force-pushed the feature/add_subscribe_client_handler7 branch from 987a2d3 to 5319438 Compare September 13, 2021 17:23

yunhanw-google mentioned this pull request Sep 13, 2021

check if the attribute is a List to set up the right filter. #9643

Closed

yunhanw-google requested a review from vivien-apple September 13, 2021 17:31

yunhanw-google and others added 2 commits September 13, 2021 11:08

Update test cases, add darwin test

df4f5e9

yunhanw-google force-pushed the feature/add_subscribe_client_handler7 branch from 5319438 to df4f5e9 Compare September 13, 2021 18:13

run codegen

74eb32c

mspang merged commit ec5f5dc into project-chip:master Sep 13, 2021

erjiaqing mentioned this pull request Sep 14, 2021

[im/test] Fix interaction model subscription test #9677

Merged

vivien-apple mentioned this pull request Sep 14, 2021

[YAML] Update chip-tool tests template to generates less redundant code #9641

Merged

yunhanw-google mentioned this pull request Sep 15, 2021

Disable the path filter for IM read. #9644

Closed

hubTab reviewed Sep 15, 2021

View reviewed changes

vivien-apple added a commit to vivien-apple/connectedhomeip-1 that referenced this pull request Sep 23, 2021

Rename ConfigureAttribute to SubscribeAttribute, remove isAnalog, and…

96f9182

… remove some dead code introduced in project-chip#9510

vivien-apple mentioned this pull request Sep 23, 2021

Rename ConfigureAttribute to SubscribeAttribute, remove isAnalog, and remove some dead code introduced in #9510 #9913

Merged

vivien-apple added a commit to vivien-apple/connectedhomeip-1 that referenced this pull request Sep 23, 2021

Rename ConfigureAttribute to SubscribeAttribute, remove isAnalog, and…

14475e0

… remove some dead code introduced in project-chip#9510

msandstedt mentioned this pull request Sep 28, 2021

Call DRBG_get_bytes() For Subscription Id Generation. #9995

Merged

vivien-apple added a commit to vivien-apple/connectedhomeip-1 that referenced this pull request Sep 30, 2021

Rename ConfigureAttribute to SubscribeAttribute, remove isAnalog, and…

8a1090a

… remove some dead code introduced in project-chip#9510

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add and Enable IM subscription support #9510

Add and Enable IM subscription support #9510

yunhanw-google commented Sep 7, 2021 •

edited

Loading

andy31415 commented Sep 7, 2021

woody-apple commented Sep 8, 2021

yunhanw-google commented Sep 13, 2021

vivien-apple commented Sep 13, 2021

github-actions bot commented Sep 13, 2021

github-actions bot commented Sep 13, 2021

github-actions bot commented Sep 13, 2021

hubTab Sep 15, 2021 •

edited

Loading

Add and Enable IM subscription support #9510

Add and Enable IM subscription support #9510

Conversation

yunhanw-google commented Sep 7, 2021 • edited Loading

Problem

Change overview

Testing

andy31415 commented Sep 7, 2021

woody-apple commented Sep 8, 2021

yunhanw-google commented Sep 13, 2021

vivien-apple commented Sep 13, 2021

github-actions bot commented Sep 13, 2021

github-actions bot commented Sep 13, 2021

github-actions bot commented Sep 13, 2021

hubTab Sep 15, 2021 • edited Loading

Choose a reason for hiding this comment

yunhanw-google commented Sep 7, 2021 •

edited

Loading

hubTab Sep 15, 2021 •

edited

Loading