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

Revisions to System::Layer #8267

Merged
merged 4 commits into from
Jul 15, 2021

Conversation

kpschoedel
Copy link
Contributor

Problem

System::Layer has redundancy in event configuration,
and a confusingly-named grab-bag namespace System::Platform::Layer

This is a standalone fragment of work toward event/threading
refactoring, issue #7725.

Change overview

  • Merge CHIP_SYSTEM_CONFIG_LWIP_EVENT_OBJECT_TYPE with
    CHIP_SYSTEM_CONFIG_EVENT_OBJECT_TYPE, and
    CHIP_SYSTEM_CONFIG_LWIP_EVENT_TYPE with
    CHIP_SYSTEM_CONFIG_EVENT_TYPE, since they serve the same purpose
    in LwIP and non-LwIP implementations respectively.
  • Remove some dead configuration code.
  • Move event functions from the System::Platform::Layer namespace
    (not the same kind of ‘layer’ as System::Layer) to a specific
    System::Platform::EventSupport namespace.
  • Move (Will|Did)(Init|Shutdown) functions from the
    System::Platform::Layer namespace up to System::Platform.

Testing

No functional changes, only renaming and pruning.

#### Problem

`System::Layer` has redundancy in event configuration,
and a confusingly-named grab-bag namespace `System::Platform::Layer`

This is a standalone fragment of work toward event/threading
refactoring, issue project-chip#7725.

#### Change overview

- Merge `CHIP_SYSTEM_CONFIG_LWIP_EVENT_OBJECT_TYPE` with
  `CHIP_SYSTEM_CONFIG_EVENT_OBJECT_TYPE`, and
  `CHIP_SYSTEM_CONFIG_LWIP_EVENT_TYPE` with
  `CHIP_SYSTEM_CONFIG_EVENT_TYPE`, since they serve the same purpose
  in LwIP and non-LwIP implementations respectively.
- Remove some dead configuration code.
- Move event functions from the `System::Platform::Layer` namespace
  (not the same kind of ‘layer’ as `System::Layer`) to a specific
  `System::Platform::EventSupport` namespace.
- Move (Will|Did)(Init|Shutdown) functions from the
  `System::Platform::Layer` namespace up to `System::Platform`.

#### Testing

No functional changes, only renaming and pruning.
@boring-cyborg boring-cyborg bot added the app label Jul 10, 2021

namespace Platform {
namespace Layer {
namespace EventSupport {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: do we need "Support" in the name here? I generally claim that support/helper/core/util and similar naming do not add significant readability. Could we get around with just "Eventing" or "Events" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will rename in the next PR.

@github-actions
Copy link

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

File Section File VM
chip-all-clusters-app.elf .flash.text -16 -16
chip-lock-app.elf .flash.text 64 64
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
.debug_loc,0,-3
.debug_abbrev,0,-14
.debug_info,0,-39

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

sections,vmsize,filesize
.debug_loc,0,-6
.debug_line,0,-258
.debug_abbrev,0,-1375
.debug_str,0,-4866
.debug_info,0,-10739

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

sections,vmsize,filesize
.debug_str,0,20
.strtab,0,16
.debug_loc,0,-19
.debug_line,0,-1584
.debug_abbrev,0,-7764
.debug_info,0,-67273

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

sections,vmsize,filesize
.xt.prop._ZN4chip6System5Mutex6UnlockEv,0,108
.xt.lit._ZN4chip11DeviceLayer8Internal35GenericPlatformManagerImpl_FreeRTOSINS0_19PlatformManagerImplEE14_LockChipStackEv,0,48
.xt.lit._ZN4chip11DeviceLayer8Internal35GenericPlatformManagerImpl_FreeRTOSINS0_19PlatformManagerImplEE15_StartChipTimerEj,0,48
.xt.lit._ZN4chip11DeviceLayer8Internal35GenericPlatformManagerImpl_FreeRTOSINS0_19PlatformManagerImplEE16_UnlockChipStackEv,0,48
.debug_str,0,23
.strtab,0,16
[Unmapped],0,16
.xt.prop._ZN4chip11DeviceLayer8Internal35GenericPlatformManagerImpl_FreeRTOSINS0_19PlatformManagerImplEE14_LockChipStackEv,0,12
.xt.prop._ZN4chip11DeviceLayer8Internal35GenericPlatformManagerImpl_FreeRTOSINS0_19PlatformManagerImplEE15_StartChipTimerEj,0,12
.xt.prop._ZN4chip11DeviceLayer8Internal35GenericPlatformManagerImpl_FreeRTOSINS0_19PlatformManagerImplEE16_UnlockChipStackEv,0,12
.flash.text,-16,-16
.debug_loc,0,-29
.xt.prop._ZN4chip11DeviceLayer8Internal26GenericPlatformManagerImplINS0_19PlatformManagerImplEE14_InitChipStackEv,0,-40
.xt.lit._ZN4chip11DeviceLayer8Internal35GenericPlatformManagerImpl_FreeRTOSINS0_19PlatformManagerImplEE10_PostEventEPKNS0_15ChipDeviceEventE,0,-80
.xt.lit._ZN4chip6System5Mutex6UnlockEv,0,-128
.debug_line,0,-4575
.debug_abbrev,0,-20509
.debug_info,0,-193982

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

sections,vmsize,filesize
.flash.text,64,64
.debug_str,0,22
.strtab,0,16
.debug_loc,0,10
[Unmapped],0,-64
.debug_line,0,-86
.debug_abbrev,0,-881
.debug_info,0,-8977

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

sections,vmsize,filesize
.debug_str,0,20
.strtab,0,16
.debug_loc,0,-37
.debug_line,0,-4124
.debug_abbrev,0,-18698
.debug_info,0,-174441


@bzbarsky-apple bzbarsky-apple merged commit fd6e541 into project-chip:master Jul 15, 2021
@kpschoedel kpschoedel deleted the system-event-1 branch July 15, 2021 13:23
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this pull request Jul 15, 2021
#### Problem

#### Change overview

- Removed `SystemLayer.mPlatformData`, which was never set or used,
  along with associated functions.

- Removed `SystemLayer.mContext`, which was set but never used,
  along with associated function arguments.

- Removed `CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_XTOR_FUNCTIONS`
  and associated functions, which were never used.

- Rearrangements to group LwIP-specific code together.

- project-chip#8267 review followup: rename namespace EventSupport to Eventing

#### Testing

Successful builds should confirm unused code is unused.
Sanity check using chip-tool.
bzbarsky-apple pushed a commit that referenced this pull request Jul 16, 2021
#### Problem

#### Change overview

- Removed `SystemLayer.mPlatformData`, which was never set or used,
  along with associated functions.

- Removed `SystemLayer.mContext`, which was set but never used,
  along with associated function arguments.

- Removed `CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_XTOR_FUNCTIONS`
  and associated functions, which were never used.

- Rearrangements to group LwIP-specific code together.

- #8267 review followup: rename namespace EventSupport to Eventing

#### Testing

Successful builds should confirm unused code is unused.
Sanity check using chip-tool.
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* Revisions to System::Layer

#### Problem

`System::Layer` has redundancy in event configuration,
and a confusingly-named grab-bag namespace `System::Platform::Layer`

This is a standalone fragment of work toward event/threading
refactoring, issue project-chip#7725.

#### Change overview

- Merge `CHIP_SYSTEM_CONFIG_LWIP_EVENT_OBJECT_TYPE` with
  `CHIP_SYSTEM_CONFIG_EVENT_OBJECT_TYPE`, and
  `CHIP_SYSTEM_CONFIG_LWIP_EVENT_TYPE` with
  `CHIP_SYSTEM_CONFIG_EVENT_TYPE`, since they serve the same purpose
  in LwIP and non-LwIP implementations respectively.
- Remove some dead configuration code.
- Move event functions from the `System::Platform::Layer` namespace
  (not the same kind of ‘layer’ as `System::Layer`) to a specific
  `System::Platform::EventSupport` namespace.
- Move (Will|Did)(Init|Shutdown) functions from the
  `System::Platform::Layer` namespace up to `System::Platform`.

#### Testing

No functional changes, only renaming and pruning.

* restyle

* zap regen
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
#### Problem

#### Change overview

- Removed `SystemLayer.mPlatformData`, which was never set or used,
  along with associated functions.

- Removed `SystemLayer.mContext`, which was set but never used,
  along with associated function arguments.

- Removed `CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_XTOR_FUNCTIONS`
  and associated functions, which were never used.

- Rearrangements to group LwIP-specific code together.

- project-chip#8267 review followup: rename namespace EventSupport to Eventing

#### Testing

Successful builds should confirm unused code is unused.
Sanity check using chip-tool.
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.

5 participants