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

Expand use of type-safe BitFlags and enum class. #5319

Merged
merged 1 commit into from
Mar 11, 2021

Conversation

kpschoedel
Copy link
Contributor

@kpschoedel kpschoedel commented Mar 11, 2021

Problem

  • BitFlags.h has relatively unsafe deprecated functions.
  • Some bit flags use enum rather than type-safe enum class.

Summary of Changes

  • Removed deprecated SetFlags(), ClearFlags(), and GetFlags(),
    replacing uses with BitFlags.

  • BitFlags.h changes:

    • Default to use std::underlying_type rather than requiring it as a template parameter.
    • Added BitFlags operations to reduce uses of type-unsafe Raw() and SetRaw():
      • Multi-argument constructors.
      • operator FlagsEnum().
      • ClearAll().
      • HasAny() and HasAll(), following the pattern of the existing HasOnly().
      • Binary &.
  • Converted various bit flag types from enum to enum class:

    • BLEEndPoint::ConnectionStateFlags
    • BLEEndPoint::TimerStateFlags
    • BLEManagerImpl::Flags
    • Command::CommandPathFlags
    • Encoding::HexFlags
    • GenericConfigurationManagerImpl::Flags
    • GenericConnectivityManagerImpl_Thread::Flags
    • GenericConnectivityManagerImpl_WiFi::ConnectivityFlags
    • bdx::RangeControlFlags
    • bdx::StatusCode
    • bdx::TransferControlFlags
    • bdx::TransferRole

Reland of 4b8d558 (#5232) due to a logic error in */BLEManagerImpl.cpp and a large
accidental deletion in K32W/BLEManagerImpl.cpp.

- `BitFlags.h` has relatively unsafe deprecated functions.
- Some bit flags use `enum` rather than type-safe `enum class`.

#### Summary of Changes

- Removed deprecated `SetFlags()`, `ClearFlags()`, and `GetFlags()`,
  replacing uses with `BitFlags`.

- BitFlags.h changes:
  - Default to use `std::underlying_type` rather than requiring it as a template parameter.
  - Added `BitFlags` operations to reduce uses of type-unsafe `Raw()` and `SetRaw()`:
    - Multi-argument constructors.
    - `operator FlagsEnum()`.
    - `ClearAll()`.
    - `HasAny()` and `HasAll()`, following the pattern of the existing `HasOnly()`.
    - Binary `&`.

- Converted various bit flag types from `enum` to `enum class`:
  - BLEEndPoint::ConnectionStateFlags
  - BLEEndPoint::TimerStateFlags
  - BLEManagerImpl::Flags
  - Command::CommandPathFlags
  - Encoding::HexFlags
  - GenericConfigurationManagerImpl::Flags
  - GenericConnectivityManagerImpl_Thread::Flags
  - GenericConnectivityManagerImpl_WiFi::ConnectivityFlags
  - bdx::RangeControlFlags
  - bdx::StatusCode
  - bdx::TransferControlFlags
  - bdx::TransferRole

Reland of 4b8d558 (project-chip#5232) to a logic error in `*/BLEManagerImpl.cpp` and a large
accidental deletion in `K32W/BLEManagerImpl.cpp`.
@kpschoedel
Copy link
Contributor Author

Thanks to @mspang for catching this.

@github-actions
Copy link

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

File Section File VM
chip-qpg6100-lock-example.out .text 64 64
chip-qpg6100-lock-example.out .data 8 8
chip-qpg6100-lock-example.out .heap 0 -8
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,46063
.debug_str,0,10027
.debug_abbrev,0,1168
.debug_line,0,306
.symtab,0,192
.debug_frame,0,80
.text,64,64
.debug_ranges,0,48
.debug_aranges,0,32
.strtab,0,30
.data,8,8
.shstrtab,0,2
.heap,-8,0
[Unmapped],0,-64
.debug_loc,0,-1152


@github-actions
Copy link

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

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

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,50967
.debug_str,0,9713
.debug_abbrev,0,1855
.debug_line,0,711
.debug_ranges,0,256
.xt.prop._ZN4chip11DeviceLayer8Internal31GenericConfigurationManagerImplINS0_24ConfigurationManagerImplEE5_InitEv,0,116
.flash.text,24,24
.flash.rodata,4,4
[Unmapped],0,-4
.symtab,0,-16
.xt.lit._ZN4chip11DeviceLayer8Internal31GenericConfigurationManagerImplINS0_24ConfigurationManagerImplEE5_InitEv,0,-40
.xt.prop._ZN4chip11DeviceLayer8Internal31GenericConfigurationManagerImplINS0_24ConfigurationManagerImplEE12_GetVendorIdERt,0,-40
.xt.prop._ZN4chip7SetFlagIhNS_11DeviceLayer8Internal31GenericConfigurationManagerImplINS1_24ConfigurationManagerImplEEUt_EEEvRT_T0_b,0,-100
.strtab,0,-109
.shstrtab,0,-135
.debug_loc,0,-422

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

sections,vmsize,filesize


@github-actions
Copy link

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

File Section File VM
chip-shell.elf text 76 76
chip-shell.elf init_array 4 4
chip-lock.elf text 76 76
chip-lock.elf init_array 4 4
chip-lighting.elf text 76 76
chip-lighting.elf init_array 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
.debug_info,0,23879
.debug_str,0,8863
.debug_abbrev,0,724
.debug_line,0,601
.debug_ranges,0,480
text,76,76
.debug_loc,0,69
.symtab,0,32
init_array,4,4
.shstrtab,0,1
.debug_frame,0,-8
.strtab,0,-109

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

sections,vmsize,filesize
.debug_info,0,47474
.debug_str,0,9712
.debug_abbrev,0,1164
.debug_line,0,682
.debug_ranges,0,480
.debug_loc,0,80
text,76,76
.symtab,0,32
init_array,4,4
.shstrtab,0,1
.debug_frame,0,-8
.strtab,0,-109

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

sections,vmsize,filesize
.debug_info,0,49034
.debug_str,0,9712
.debug_abbrev,0,1176
.debug_line,0,698
.debug_ranges,0,480
.debug_loc,0,84
text,76,76
.symtab,0,32
init_array,4,4
.shstrtab,0,1
.debug_frame,0,-8
.strtab,0,-109


@bzbarsky-apple bzbarsky-apple merged commit 8ea423a into project-chip:master Mar 11, 2021
@kpschoedel kpschoedel deleted the bitflags-reland branch March 12, 2021 15:28
Damian-Nordic pushed a commit to Damian-Nordic/connectedhomeip that referenced this pull request Mar 18, 2021
- `BitFlags.h` has relatively unsafe deprecated functions.
- Some bit flags use `enum` rather than type-safe `enum class`.

#### Summary of Changes

- Removed deprecated `SetFlags()`, `ClearFlags()`, and `GetFlags()`,
  replacing uses with `BitFlags`.

- BitFlags.h changes:
  - Default to use `std::underlying_type` rather than requiring it as a template parameter.
  - Added `BitFlags` operations to reduce uses of type-unsafe `Raw()` and `SetRaw()`:
    - Multi-argument constructors.
    - `operator FlagsEnum()`.
    - `ClearAll()`.
    - `HasAny()` and `HasAll()`, following the pattern of the existing `HasOnly()`.
    - Binary `&`.

- Converted various bit flag types from `enum` to `enum class`:
  - BLEEndPoint::ConnectionStateFlags
  - BLEEndPoint::TimerStateFlags
  - BLEManagerImpl::Flags
  - Command::CommandPathFlags
  - Encoding::HexFlags
  - GenericConfigurationManagerImpl::Flags
  - GenericConnectivityManagerImpl_Thread::Flags
  - GenericConnectivityManagerImpl_WiFi::ConnectivityFlags
  - bdx::RangeControlFlags
  - bdx::StatusCode
  - bdx::TransferControlFlags
  - bdx::TransferRole

Reland of 4b8d558 (project-chip#5232) to a logic error in `*/BLEManagerImpl.cpp` and a large
accidental deletion in `K32W/BLEManagerImpl.cpp`.
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this pull request Apr 1, 2021
#### Problem

TI cc13x2_26x2 does not build.
Followup from 8ea423a (PR project-chip#5319)

#### Summary of Changes

Convert `BLEManagerImpl` `mFlags` to use `enum class` and `BitFlags`.
mspang pushed a commit that referenced this pull request Apr 1, 2021
#### Problem

TI cc13x2_26x2 does not build.
Followup from 8ea423a (PR #5319)

#### Summary of Changes

Convert `BLEManagerImpl` `mFlags` to use `enum class` and `BitFlags`.
jimlyall-q pushed a commit to jimlyall-q/connectedhomeip that referenced this pull request Apr 13, 2021
#### Problem

TI cc13x2_26x2 does not build.
Followup from 8ea423a (PR project-chip#5319)

#### Summary of Changes

Convert `BLEManagerImpl` `mFlags` to use `enum class` and `BitFlags`.
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.

4 participants