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

Remove duplicate attribute path in IM Read #9389

Conversation

yunhanw-google
Copy link
Contributor

Problem

Need to remove duplicate path if read request has multiple overlapped paths.

Change overview

--Remove duplicate attribute paths if there are multiple overlapped path in read
attribute path for IM Read

Testing

--Add unit test for multiple overlapped path

Copy link
Contributor

@woody-apple woody-apple left a comment

Choose a reason for hiding this comment

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

I might be missing something, but I don't see tests sufficiently updated to verify this.

@yunhanw-google yunhanw-google force-pushed the feature/remove_duplicate_path_read branch from 6a43ddd to 76912f0 Compare September 1, 2021 18:39
@yunhanw-google yunhanw-google force-pushed the feature/remove_duplicate_path_read branch from 76912f0 to 562c0cf Compare September 1, 2021 18:42
@yunhanw-google
Copy link
Contributor Author

I might be missing something, but I don't see tests sufficiently updated to verify this.

Yep, just forgot to add the checker, thanks for pointing it out, with two duplicated path, I add one additional check to check if only one attribute path has processed.

@yunhanw-google yunhanw-google force-pushed the feature/remove_duplicate_path_read branch from 562c0cf to 68a9234 Compare September 1, 2021 19:04
src/app/ReadHandler.h Outdated Show resolved Hide resolved
src/app/ReadHandler.cpp Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Sep 1, 2021

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

File Section File VM
chip-qpg6100-lighting-example.out .text 176 176
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,3314
.debug_loc,0,1653
.debug_str,0,750
.debug_line,0,738
.text,176,176
.debug_ranges,0,152
.strtab,0,136
.debug_abbrev,0,65
.symtab,0,64
.debug_frame,0,60
.debug_aranges,0,16
[Unmapped],0,-176


@yunhanw-google yunhanw-google force-pushed the feature/remove_duplicate_path_read branch from d238bc6 to 81e48b6 Compare September 1, 2021 20:28
Summary of Changes:
--Remove duplicate attribute paths if there are multiple overlapped path in read
attribute path for IM Read
--Add unit test for multiple overlapped path

Restyled by clang-format

Restyled by clang-format
@yunhanw-google yunhanw-google force-pushed the feature/remove_duplicate_path_read branch from 81e48b6 to f044876 Compare September 1, 2021 21:23
@github-actions
Copy link

github-actions bot commented Sep 1, 2021

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

File Section File VM
chip-ipv6only-app.elf .flash.text -172 -172
chip-shell.elf .flash.text 48 48
chip-lock-app.elf .flash.text 264 264
chip-temperature-measurement-app.elf .flash.text 196 196
chip-all-clusters-app.elf .flash.text 208 208
chip-bridge-app.elf .flash.text 196 196
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
.flash.text,-172,-172
[Unmapped],0,-3924

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-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
.debug_info,0,3029
.debug_loc,0,1554
.debug_line,0,1499
.debug_str,0,853
.flash.text,264,264
.strtab,0,136
.xt.prop._ZNK4chip3app11ClusterInfo25IsAttributePathSupersetOfERKS1_,0,120
.shstrtab,0,68
.debug_abbrev,0,65
.debug_ranges,0,56
.debug_frame,0,48
.symtab,0,48
[ELF Section Headers],0,40
.debug_aranges,0,16
[Unmapped],0,-264

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

sections,vmsize,filesize
.debug_info,0,3010
.debug_loc,0,1659
.debug_line,0,1497
.debug_str,0,855
.flash.text,196,196
.strtab,0,136
.xt.prop._ZNK4chip3app11ClusterInfo25IsAttributePathSupersetOfERKS1_,0,120
.debug_abbrev,0,75
.shstrtab,0,68
.debug_ranges,0,56
.debug_frame,0,48
.symtab,0,48
[ELF Section Headers],0,40
.debug_aranges,0,16
[Unmapped],0,-196

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

sections,vmsize,filesize

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

sections,vmsize,filesize
.debug_info,0,2085
.debug_loc,0,1117
.debug_str,0,875
.debug_line,0,829
.flash.text,208,208
.strtab,0,136
.debug_abbrev,0,62
.debug_frame,0,56
.debug_ranges,0,56
.symtab,0,32
.debug_aranges,0,16
[Unmapped],0,-208

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

sections,vmsize,filesize
.debug_info,0,3016
.debug_loc,0,1727
.debug_line,0,1499
.debug_str,0,853
.flash.text,196,196
.strtab,0,136
.xt.prop._ZNK4chip3app11ClusterInfo25IsAttributePathSupersetOfERKS1_,0,120
.shstrtab,0,68
.debug_abbrev,0,65
.debug_ranges,0,56
.debug_frame,0,48
.symtab,0,48
[ELF Section Headers],0,40
.debug_aranges,0,16
[Unmapped],0,-196

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

sections,vmsize,filesize


@github-actions
Copy link

github-actions bot commented Sep 1, 2021

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

File Section File VM
chip-lock.elf text 180 180
chip-lock.elf device_handles -4 -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_info,0,3331
.debug_loc,0,1977
.debug_str,0,855
.debug_line,0,752
text,180,180
.debug_ranges,0,152
.strtab,0,136
.debug_abbrev,0,65
.symtab,0,64
.debug_frame,0,60
.debug_aranges,0,16
device_handles,-4,-4


@mspang mspang merged commit c054c9f into project-chip:master Sep 1, 2021
mkardous-silabs pushed a commit to mkardous-silabs/connectedhomeip that referenced this pull request Sep 24, 2021
Summary of Changes:
--Remove duplicate attribute paths if there are multiple overlapped path in read
attribute path for IM Read
--Add unit test for multiple overlapped path

Restyled by clang-format

Restyled by clang-format
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