Use enum class for TLVElementType and TLVTagControl. #3157

bzbarsky-apple · 2020-10-08T21:12:07Z

This makes it easy to give them as specific size spec, which will
prevent issues with people adding out-of-range values and simplify
-Wconversion work.

Problem

TLVElementType and TLVTagControl are raw enums, not enum classes, so proving that casts to 8-bit ints are safe is not trivial.

Summary of Changes

Make them 8-bit-backed enum classes, so the compiler will enforce things for us.

rwalker-apple · 2020-10-09T04:59:54Z

src/lib/core/CHIPTLVData.hpp

@@ -105,41 +105,41 @@
 /*
 *  @brief Specifies an anonymous TLV element, which doesn't have any tag
 */
-#define CHIP_TLV_TAG_ANONYMOUS chip::TLV::kTLVTagControl_Anonymous
+#define CHIP_TLV_TAG_ANONYMOUS chip::TLV::TLVTagControl::Anonymous


this departs from C++ towards Haskell :-)

gerickson · 2020-10-12T15:08:06Z

src/lib/core/CHIPTLV.h

@@ -58,6 +58,16 @@ class PacketBuffer;
 namespace chip {
 namespace TLV {

+inline uint8_t operator|(TLVElementType lhs, TLVTagControl rhs)


Style nit: but strongly prefer white space around operator the operation | to improve readability.

can we make .clang-format do that? if not, we're gonna have trouble enforcing

Agreed. I can't seem to find anything in the clang-format documentation that indicates control over this specific behavior.

This makes it easy to give them as specific size spec, which will prevent issues with people adding out-of-range values and simplify -Wconversion work.

…end the command (project-chip#3151)

Some devices are configured to act as Thread router instead of sleepy-end device. This PR removes these incorrect or verbose comments.

…ject-chip#3175) * [nRF Connect] README updates on Docker for MacOS and NCS version * Restyled by prettier-markdown * Rephrase a sentence Co-authored-by: Restyled.io <commits@restyled.io>

…ude server init callbacks. (project-chip#3180) This allows the app to implement emberAfPluginOnOffClusterServerPostInitCallback to sync up the data model state with the state of the actual device when the data model initializes.

* Add a lot of pragma once changes. Now scripted! * update pragma once in setup payload * More pragma once * Pragma once within platform and more * pragma once in the crypto layer * pragma once in examples * Fix by restyle-diff

…-chip#3161)

github-actions · 2020-10-12T21:38:23Z

Size increase report for "esp32-example-build"

File	Section	File	VM
chip-wifi-echo.elf	.flash.text	16	16

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
.debug_info,0,458
.debug_loc,0,323
.debug_line,0,215
.debug_ranges,0,144
.debug_abbrev,0,24
.flash.text,16,16
[Unmapped],0,4
.debug_str,0,-508

github-actions · 2020-10-12T21:38:36Z

Size increase report for "nrfconnect-example-build"

File	Section	File	VM
chip-lock.elf	text	22	22
chip-lock.elf	[LOAD #1 [RWX]]	-6	-6
chip-lighting.elf	text	22	22
chip-lighting.elf	[LOAD #1 [RWX]]	-6	-6

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,539
.debug_loc,0,256
.debug_ranges,0,168
.debug_line,0,128
.debug_abbrev,0,24
text,22,22
.debug_frame,0,4
[Unmapped],0,-3
[LOAD #1 [RWX]],-6,-6
.debug_str,0,-508

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

sections,vmsize,filesize
.debug_info,0,539
.debug_loc,0,256
.debug_ranges,0,168
.debug_line,0,128
.debug_abbrev,0,24
text,22,22
[Unmapped],0,5
.debug_frame,0,4
[LOAD #1 [RWX]],-6,-6
.debug_str,0,-508

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

sections,vmsize,filesize

boring-cyborg bot added the lib label Oct 8, 2020

pullapprove bot requested review from andy31415, BroderickCarlin, chrisdecenzo, hawk248, jelderton, mspang, rwalker-apple and saurabhst October 8, 2020 21:14

rwalker-apple reviewed Oct 9, 2020

View reviewed changes

rwalker-apple approved these changes Oct 9, 2020

View reviewed changes

andy31415 approved these changes Oct 9, 2020

View reviewed changes

mspang approved these changes Oct 9, 2020

View reviewed changes

BroderickCarlin approved these changes Oct 12, 2020

View reviewed changes

gerickson reviewed Oct 12, 2020

View reviewed changes

gerickson approved these changes Oct 12, 2020

View reviewed changes

restyled-io bot mentioned this pull request Oct 12, 2020

Restyle Use enum class for TLVElementType and TLVTagControl. #3202

Closed

bzbarsky-apple and others added 9 commits October 12, 2020 14:21

Use enum class for TLVElementType and TLVTagControl.

08889a0

This makes it easy to give them as specific size spec, which will prevent issues with people adding out-of-range values and simplify -Wconversion work.

Check if the result of EncodeCommand is successful before trying to s…

e516b00

…end the command (project-chip#3151)

Remove verbose code comments (project-chip#3187)

b0722c2

Some devices are configured to act as Thread router instead of sleepy-end device. This PR removes these incorrect or verbose comments.

Remove {#mainpage} from the main README.md file (project-chip#3190)

fa60110

Remove Makefiles introduced by project-chip#3099 (project-chip#3191)

b9c8f5c

[nRF Connect] README updates on Docker for MacOS and NCS version (pro…

69c9aba

…ject-chip#3175) * [nRF Connect] README updates on Docker for MacOS and NCS version * Restyled by prettier-markdown * Rephrase a sentence Co-authored-by: Restyled.io <commits@restyled.io>

Use pragma once in more places (project-chip#3182)

b90ead3

* Add a lot of pragma once changes. Now scripted! * update pragma once in setup payload * More pragma once * Pragma once within platform and more * pragma once in the crypto layer * pragma once in examples * Fix by restyle-diff

[android] Use ATT Write Request instead of ATT Write Command (project…

660a4df

…-chip#3161)

rwalker-apple force-pushed the enum-class-tlv-tags branch from 70e5c75 to 660a4df Compare October 12, 2020 21:23

boring-cyborg bot added the examples label Oct 12, 2020

rwalker-apple merged commit aa1f11c into project-chip:master Oct 12, 2020

bzbarsky-apple deleted the enum-class-tlv-tags branch June 16, 2021 14:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use enum class for TLVElementType and TLVTagControl. #3157

Use enum class for TLVElementType and TLVTagControl. #3157

bzbarsky-apple commented Oct 8, 2020

rwalker-apple Oct 9, 2020

gerickson Oct 12, 2020

rwalker-apple Oct 12, 2020

gerickson Oct 12, 2020

github-actions bot commented Oct 12, 2020

github-actions bot commented Oct 12, 2020

Use enum class for TLVElementType and TLVTagControl. #3157

Use enum class for TLVElementType and TLVTagControl. #3157

Conversation

bzbarsky-apple commented Oct 8, 2020

Problem

Summary of Changes

rwalker-apple Oct 9, 2020

Choose a reason for hiding this comment

gerickson Oct 12, 2020

Choose a reason for hiding this comment

rwalker-apple Oct 12, 2020

Choose a reason for hiding this comment

gerickson Oct 12, 2020

Choose a reason for hiding this comment

github-actions bot commented Oct 12, 2020

github-actions bot commented Oct 12, 2020