Improve error checking in the command encoder. #1913

bzbarsky-apple · 2020-07-29T20:40:08Z

There are actually two changes here:

Improve the error checking so that we correctly check that the
buffer actually has enough space when writing a multibyte value.
Introduce a way to ask the encoder how much space it needs to
encode a given APS frame.

Fixes #1644

Problem

The error checking in the command encoder doesn't work correctly when encoding multibyte values

Summary of Changes

Fix the error checking to correctly check that we have enough space.

BroderickCarlin · 2020-07-30T06:14:25Z

src/app/chip-zcl-zpro/command-encoder/encoder.c

-    {
-        return size;
-    }
+#define TRY_WRITE(dataItem)                                                                                                        \


Do we have a generic method for interfacing with buffers? Having proper methods for interfacing with buffers may be a better solution than a locally defined macro

I haven't found one so far, but if we do, I'm happy to use it.

Note that this stuff is all still throwaway code pending whatever our real encoder will be; I really needed the "measure the size" aspect now, and adding it this way was simpler than keeping the existing structure of the code and adding lots of null-checks on buffer...

Also note that there is more work to be done here, e.g. to fix #1632

something that works on top of PacketBuffer would be welcome. something that has a "measure" form would be ideal.

I've just added BufBound, which might help here

We'd need to switch this file to C++ to use that. Which might be fine as long as we keep the entrypoint APIs extern C.... I can rebase on top of your change once it lands as desired, and make that modification.

ah, yeah. I was thinking that we probably want to just make all these files C++ and fix any void* stuff

Yeah, #1629 tracks the void* bits.

#2073 now tracks using BufBound here.

There are actually two changes here: 1) Improve the error checking so that we correctly check that the buffer actually has enough space when writing a multibyte value. 2) Introduce a way to ask the encoder how much space it needs to encode a given APS frame. Fixes project-chip#1644

github-actions · 2020-07-31T20:44:09Z

Size increase report for "gn_nrf-example-build"

File	Section	File	VM

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

github-actions · 2020-07-31T20:44:14Z

Size increase report for "gn_linux-example-build"

File	Section	File	VM

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

github-actions · 2020-07-31T20:50:23Z

Size increase report for "nrf-example-build"

File	Section	File	VM

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-nrf52840-lock-example.out and ./pull_artifact/chip-nrf52840-lock-example.out:

sections,vmsize,filesize

github-actions · 2020-07-31T20:50:30Z

Size increase report for "nrfconnect-example-build"

File	Section	File	VM

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize

github-actions · 2020-07-31T20:50:35Z

Size increase report for "linux-example-build"

File	Section	File	VM
chip-standalone-demo.out	.text	48	48
chip-standalone-demo.out	.eh_frame	40	40
chip-standalone-demo.out	.eh_frame_hdr	8	8

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-standalone-demo.out and ./pull_artifact/chip-standalone-demo.out:

sections,vmsize,filesize
.debug_info,0,700
.debug_line,0,540
.debug_str,0,524
.debug_loc,0,410
.debug_ranges,0,80
.debug_abbrev,0,76
.debug_macro,0,70
.text,48,48
.eh_frame,40,40
.symtab,0,24
.strtab,0,17
.debug_aranges,0,16
.eh_frame_hdr,8,8
[Unmapped],0,-97

github-actions · 2020-07-31T20:50:42Z

Size increase report for "esp32-example-build"

File	Section	File	VM

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-wifi-echo.elf and ./pull_artifact/chip-wifi-echo.elf:

sections,vmsize,filesize

woody-apple · 2020-08-03T21:00:24Z

@jelderton @andy31415 ?

…dio after creating a solution Merge in WMN_TOOLS/matter from feature/solutions_seq_templates to silabs_slc_1.3 Squashed commit of the following: commit 806440fda12d0a727f41efe549ebfa771be9619a Author: Sarthak Shaha <[email protected]> Date: Tue May 28 15:06:53 2024 -0400 rebase commit 97b7b05729b7aa7cf2ffd3f5c108d1829433aab9 Author: Sarthak Shaha <[email protected]> Date: Tue May 28 13:47:10 2024 -0400 fix to show project.slcp first in studio after creating a solution

bzbarsky-apple requested review from andy31415, anush-apple, BroderickCarlin, chrisdecenzo, hawk248, jelderton, robszewczyk, saurabhst and woody-apple as code owners July 29, 2020 20:40

boring-cyborg bot added the app label Jul 29, 2020

restyled-io bot mentioned this pull request Jul 29, 2020

Restyle Improve error checking in the command encoder. #1914

Closed

bzbarsky-apple force-pushed the encoder-fix-error-checking branch from f8a9f8c to 81d9fbe Compare July 29, 2020 20:46

bzbarsky-apple requested a review from bhaskar-apple July 30, 2020 03:51

bzbarsky-apple self-assigned this Jul 30, 2020

bzbarsky-apple requested a review from rwalker-apple July 30, 2020 04:38

bzbarsky-apple force-pushed the encoder-fix-error-checking branch from 81d9fbe to 7565683 Compare July 30, 2020 04:41

BroderickCarlin reviewed Jul 30, 2020

View reviewed changes

bzbarsky-apple mentioned this pull request Jul 30, 2020

Implement DefaultResponse commands #1915

Merged

bzbarsky-apple force-pushed the encoder-fix-error-checking branch from 7565683 to fa5dcc1 Compare July 30, 2020 18:13

bzbarsky-apple force-pushed the encoder-fix-error-checking branch from fa5dcc1 to 8ef4108 Compare July 31, 2020 20:37

rwalker-apple approved these changes Jul 31, 2020

View reviewed changes

BroderickCarlin approved these changes Aug 3, 2020

View reviewed changes

bhaskar-apple approved these changes Aug 3, 2020

View reviewed changes

woody-apple approved these changes Aug 3, 2020

View reviewed changes

jelderton approved these changes Aug 3, 2020

View reviewed changes

woody-apple merged commit 8e03d28 into project-chip:master Aug 3, 2020

bzbarsky-apple deleted the encoder-fix-error-checking branch August 4, 2020 02:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve error checking in the command encoder. #1913

Improve error checking in the command encoder. #1913

bzbarsky-apple commented Jul 29, 2020

BroderickCarlin Jul 30, 2020

bzbarsky-apple Jul 30, 2020

rwalker-apple Jul 30, 2020

rwalker-apple Jul 31, 2020

bzbarsky-apple Jul 31, 2020

rwalker-apple Aug 3, 2020

bzbarsky-apple Aug 4, 2020

bzbarsky-apple Aug 11, 2020

github-actions bot commented Jul 31, 2020

github-actions bot commented Jul 31, 2020

github-actions bot commented Jul 31, 2020

github-actions bot commented Jul 31, 2020

github-actions bot commented Jul 31, 2020

github-actions bot commented Jul 31, 2020

woody-apple commented Aug 3, 2020

Improve error checking in the command encoder. #1913

Improve error checking in the command encoder. #1913

Conversation

bzbarsky-apple commented Jul 29, 2020

Problem

Summary of Changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 31, 2020

github-actions bot commented Jul 31, 2020

github-actions bot commented Jul 31, 2020

github-actions bot commented Jul 31, 2020

github-actions bot commented Jul 31, 2020

github-actions bot commented Jul 31, 2020

woody-apple commented Aug 3, 2020