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

[msg] Reorder header bits to match spec. #7861

Merged
merged 2 commits into from
Jun 28, 2021

Conversation

turon
Copy link
Contributor

@turon turon commented Jun 24, 2021

Problem

The SDK message header flags do not match the specification.

Change overview

In order to write tools that dissect the Matter message format, this PR attempts to align the SDK implementation with the latest specification of the message flags and security flags fields. This PR is a minimal change set. Follow on tasks include:

  • encode / decode paths should handle message and security flags as separate bytes rather than as a combined, little-endian uint16
  • combine kEncryptionType and kEncryptedMessage -- currently trivial attempts to do so fail PASE and MRP unit tests. The EncryptionType field is not at the proper specification location, and the kEncryptedMessage flag is a correct yet simplified view of the field.
  • clean up encode/decode code paths so header processing is simplified and done in one place in all cases.

Fixes #7772. Fixes #7773. Fixes #7774.

Testing

The existing CI does a good job of validating encode / decode paths for the header flags.

@bzbarsky-apple
Copy link
Contributor

/rebase

@github-actions
Copy link

Size increase report for "gn_qpg6100-example-build" from ed677f0

File Section File VM
chip-qpg6100-lighting-example.out .text -16 -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
.debug_loc,0,252
.debug_str,0,16
[Unmapped],0,16
.debug_ranges,0,-16
.text,-16,-16
.debug_abbrev,0,-18
.debug_frame,0,-20
.debug_line,0,-35
.debug_info,0,-1831

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 ed677f0

File Section File VM
chip-shell.elf text -16 -16
chip-lock.elf text -16 -16
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
.debug_loc,0,233
.debug_str,0,15
.debug_ranges,0,8
.debug_frame,0,-4
.debug_line,0,-12
text,-16,-16
.debug_abbrev,0,-23
.debug_info,0,-281

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

sections,vmsize,filesize
.debug_loc,0,297
.debug_str,0,15
.debug_ranges,0,8
.debug_frame,0,-4
.debug_line,0,-12
text,-16,-16
.debug_abbrev,0,-23
.debug_info,0,-1777


@github-actions
Copy link

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

File Section File VM
chip-all-clusters-app.elf .flash.text -12 -12
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize

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

sections,vmsize,filesize
.debug_loc,0,54
.debug_str,0,14
.shstrtab,0,-2
.strtab,0,-10
.flash.text,-12,-12
.symtab,0,-16
.debug_abbrev,0,-18
.debug_ranges,0,-24
.debug_line,0,-26
.debug_info,0,-2192


@woody-apple
Copy link
Contributor

@mspang mspang merged commit a89a079 into project-chip:master Jun 28, 2021
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* [msg] Reorder header bits to match spec.

* Restyled by clang-format

Co-authored-by: Restyled.io <commits@restyled.io>
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.

[msg] MX Flag always set [msg] D Flag should be DSIZ field [msg] S Flag using wrong bit.
8 participants