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

Fix operational credentials fabric count attributes #9303

Merged
merged 7 commits into from
Aug 31, 2021

Conversation

pan-apple
Copy link
Contributor

Problem

The SupportedFabrics and CommissionedFabrics attributes of operational credentials cluster are always 0, when accessed via the read attribute command.

Change overview

The attribute values were not getting updated. The code to write SupportedFabrics was missing. CommissionedFabrics value was not written due to FabricID check.

Testing

Added a new cluster test to read and verify these two values.

@pan-apple
Copy link
Contributor Author

@andy31415 , @Damian-Nordic , @jepenven-silabs do you have any review feedback?

Copy link
Contributor

@tcarmelveilleux tcarmelveilleux left a comment

Choose a reason for hiding this comment

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

  • Test must be written to avoid assumption which is a configuration option of the project rather than a well-known constant.

src/app/tests/suites/OperationalCredentialsCluster.yaml Outdated Show resolved Hide resolved
@pan-apple
Copy link
Contributor Author

rebased.. trying to re-trigger CI.

@pan-apple pan-apple force-pushed the fix-opcreds-attributes branch from 68ddc93 to 1e5e61b Compare August 30, 2021 20:09
@github-actions
Copy link

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

File Section File VM
chip-qpg6100-lighting-example.out .text 100 100
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
.text,100,100
.strtab,0,82
.debug_info,0,78
.symtab,0,32
.debug_line,0,24
.shstrtab,0,2
.debug_str,0,-1
.debug_loc,0,-57
[Unmapped],0,-100


@github-actions
Copy link

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

File Section File VM
chip-all-clusters-app.elf .flash.rodata 72 72
chip-all-clusters-app.elf .flash.text 44 44
chip-lock-app.elf .flash.text 92 92
chip-lock-app.elf .flash.rodata 64 64
chip-temperature-measurement-app.elf .flash.rodata 72 72
chip-temperature-measurement-app.elf .flash.text 12 12
chip-bridge-app.elf .flash.rodata 72 72
chip-bridge-app.elf .flash.text 24 24
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize

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

sections,vmsize,filesize
.strtab,0,82
.flash.rodata,72,72
.flash.text,44,44
.debug_info,0,28
.debug_line,0,26
.symtab,0,16
.riscv.attributes,0,-2
.shstrtab,0,-2
.debug_loc,0,-12
[Unmapped],0,-116

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

sections,vmsize,filesize
.flash.text,92,92
.debug_info,0,91
.strtab,0,82
.flash.rodata,64,64
.debug_loc,0,21
.symtab,0,16
.debug_line,0,9
.debug_str,0,-1
.shstrtab,0,-2
.debug_ranges,0,-16
[Unmapped],0,-156

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

sections,vmsize,filesize
.debug_info,0,90
.strtab,0,82
.flash.rodata,72,72
.symtab,0,16
.flash.text,12,12
.shstrtab,0,2
.debug_str,0,1
.debug_line,0,-1
.debug_loc,0,-2
.debug_ranges,0,-16
[Unmapped],0,-84

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

sections,vmsize,filesize

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

sections,vmsize,filesize

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

sections,vmsize,filesize

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

sections,vmsize,filesize
.debug_info,0,90
.strtab,0,82
.flash.rodata,72,72
.flash.text,24,24
.debug_loc,0,22
.symtab,0,16
.debug_line,0,9
.debug_str,0,-1
.shstrtab,0,-2
.debug_ranges,0,-16
[Unmapped],0,-96


@github-actions
Copy link

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

File Section File VM
chip-lock.elf rodata 64 68
chip-lock.elf text 40 40
chip-lock.elf device_handles -8 -8
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
.strtab,0,82
.debug_info,0,78
rodata,68,64
text,40,40
.symtab,0,32
.debug_line,0,26
.shstrtab,0,2
device_handles,-8,-8
.debug_loc,0,-32


@pan-apple
Copy link
Contributor Author

@andy31415 , @Damian-Nordic , @jepenven-silabs do you have any review feedback?

@woody-apple
Copy link
Contributor

@saurabhst ?

@woody-apple
Copy link
Contributor

@Damian-Nordic @hawk248 ?

@woody-apple
Copy link
Contributor

@andy31415 ?

@andy31415 andy31415 merged commit dfb4fda into project-chip:master Aug 31, 2021
@andy31415
Copy link
Contributor

TE5 cherrypick failed due to changes in src/darwin/Framework/CHIP/templates/clusters-tests.zapt:

#9215 and #9287 changed the same file

andy31415 pushed a commit that referenced this pull request Aug 31, 2021
* Fix operational credentials fabric count attributes

* make the PR independet of compressed fabric ID feature

* add test to Darwin tests as well

* remove invalid check for vendor id

* Update unit tests to check for limits

* update number of min fabrics in the test

* remove upper limit check in SupportedFabrics
@andy31415
Copy link
Contributor

I did a manual merge of this - the change seemed to be only the test list.

@pan-apple pan-apple deleted the fix-opcreds-attributes branch August 31, 2021 21:54
hank820 pushed a commit to hank820/connectedhomeip that referenced this pull request Sep 1, 2021
* Fix operational credentials fabric count attributes

* make the PR independet of compressed fabric ID feature

* add test to Darwin tests as well

* remove invalid check for vendor id

* Update unit tests to check for limits

* update number of min fabrics in the test

* remove upper limit check in SupportedFabrics
hank820 pushed a commit to hank820/connectedhomeip that referenced this pull request Sep 1, 2021
* Fix operational credentials fabric count attributes

* make the PR independet of compressed fabric ID feature

* add test to Darwin tests as well

* remove invalid check for vendor id

* Update unit tests to check for limits

* update number of min fabrics in the test

* remove upper limit check in SupportedFabrics
hank820 pushed a commit to hank820/connectedhomeip that referenced this pull request Sep 1, 2021
* Fix operational credentials fabric count attributes

* make the PR independet of compressed fabric ID feature

* add test to Darwin tests as well

* remove invalid check for vendor id

* Update unit tests to check for limits

* update number of min fabrics in the test

* remove upper limit check in SupportedFabrics
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* Fix operational credentials fabric count attributes

* make the PR independet of compressed fabric ID feature

* add test to Darwin tests as well

* remove invalid check for vendor id

* Update unit tests to check for limits

* update number of min fabrics in the test

* remove upper limit check in SupportedFabrics
mkardous-silabs pushed a commit to mkardous-silabs/connectedhomeip that referenced this pull request Sep 24, 2021
* Fix operational credentials fabric count attributes

* make the PR independet of compressed fabric ID feature

* add test to Darwin tests as well

* remove invalid check for vendor id

* Update unit tests to check for limits

* update number of min fabrics in the test

* remove upper limit check in SupportedFabrics
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