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

[clusters] Fix scheduled actions in clusters #10001

Merged
merged 1 commit into from
Sep 30, 2021

Conversation

Damian-Nordic
Copy link
Contributor

Problem

PR #9469 changed the implementation of EventControlHandler function from af-event.cpp, but only updated Color Control Cluster code to use the new approach. As a result, all other clusters using scheduled actions, such as LevelControl.MoveToLevel, got broken.

Change overview

Make sure both methods of initializing cluster events are supported until all clusters are aligned one way or another.
Also, add a cirque test for LevelControl.MoveToLevel.

Testing

Tested by Cirque and using nRF Connect lighting-app.

@Damian-Nordic
Copy link
Contributor Author

FYI @mkardous-silabs

PR project-chip#9469 changed the implementation of
EventControlHandler function from af-event.cpp, but only
updated Color Control Cluster code to use the new approach.
As a result, all other clusters using scheduled actions,
such as LevelControl.MoveToLevel, got broken.

Make sure both methods of initializing cluster events are
supported until all clusters are aligned one way or another.
Also, add a cirque test for LevelControl.MoveToLevel.
@mkardous-silabs
Copy link
Contributor

Might be worth while to update level-control to use the updated version EventControlHandler instead adding back the old code that use depracted event management through generated code that isn't generated anymore.

@Damian-Nordic
Copy link
Contributor Author

@mkardous-silabs Yes, but there are more clusters that need to be updated, so I would prefer to fix the regression and later start transitioning subsequent clusters.

@mkardous-silabs
Copy link
Contributor

@Damian-Nordic Fair enough, but if you have the time, it seems like an opportune moment to update the level-control cluster at the same time to the updated implementation.

@github-actions
Copy link

Size increase report for "gn_qpg-example-build" from 3e86fbb

File Section File VM
chip-qpg6100-lighting-example.out .text 404 404
chip-qpg6100-lighting-example.out .data 16 16
chip-qpg6100-lighting-example.out .heap 0 -16
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
.text,404,404
.debug_loc,0,175
.symtab,0,160
.debug_info,0,141
.strtab,0,127
.debug_line,0,45
.debug_abbrev,0,30
.data,16,16
.debug_str,0,1
.shstrtab,0,1
.heap,-16,0
[Unmapped],0,-404

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'


@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 3e86fbb

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

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize

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

sections,vmsize,filesize
.debug_loc,0,144
.debug_info,0,140
.symtab,0,48
.debug_line,0,28
[LOAD #3 [RW]],28,0
text,16,16
.strtab,0,11
.debug_abbrev,0,8
bss,4,0
.shstrtab,0,1


@github-actions
Copy link

Size increase report for "esp32-example-build" from 3e86fbb

File Section File VM
chip-all-clusters-app.elf .flash.text 2232 2232
chip-all-clusters-app.elf .flash.rodata 608 608
chip-all-clusters-app.elf .dram0.data 64 64
chip-all-clusters-app.elf .dram0.bss 0 32
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
.flash.text,2232,2232
.strtab,0,1550
.flash.rodata,608,608
.symtab,0,464
.debug_info,0,135
.debug_loc,0,93
.dram0.data,64,64
.debug_line,0,56
.dram0.bss,32,0
.debug_abbrev,0,12
.shstrtab,0,-2
[Unmapped],0,-2904


@woody-apple
Copy link
Contributor

@woody-apple woody-apple merged commit 213e483 into project-chip:master Sep 30, 2021
andy31415 pushed a commit that referenced this pull request Oct 7, 2021
PR #9469 changed the implementation of
EventControlHandler function from af-event.cpp, but only
updated Color Control Cluster code to use the new approach.
As a result, all other clusters using scheduled actions,
such as LevelControl.MoveToLevel, got broken.

Make sure both methods of initializing cluster events are
supported until all clusters are aligned one way or another.
Also, add a cirque test for LevelControl.MoveToLevel.
@andy31415
Copy link
Contributor

Cherrypicked with request/approval from Chaitanya

jimlyall-q pushed a commit to jimlyall-q/connectedhomeip that referenced this pull request Oct 11, 2021
PR project-chip#9469 changed the implementation of
EventControlHandler function from af-event.cpp, but only
updated Color Control Cluster code to use the new approach.
As a result, all other clusters using scheduled actions,
such as LevelControl.MoveToLevel, got broken.

Make sure both methods of initializing cluster events are
supported until all clusters are aligned one way or another.
Also, add a cirque test for LevelControl.MoveToLevel.
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.

6 participants