Virtualize System::Layer interfaces #9366

kpschoedel · 2021-08-31T14:08:03Z

Problem

Issue #7715 Virtualize System and Inet interfaces

Change overview

Establishes an abstract System::Layer class hierarchy, described below.
Converts the transitional WatchableEventManager classes into
corresponding LayerImpl classes.
Move LwIPEventHandlerDelegate into LayerLwIP.
Remove GetClock(), which was only used in one file.
(System::Clock is currently fully static, but might benefit from its
own virtualization, e.g. for test timeouts.)
For the moment, CHIP_SYSTEM_CONFIG_USE_DISPATCH is still included
by #if on LayerImplSelect. With some changes to Inet, Dispatch can
probably have its own much simpler implementation of LayerSockets.

The class hierarchy is:

Layer: Core timer methods.
- LayerLwIP: Adds methods specific to builds using
  CHIP_SYSTEM_CONFIG_USING_LWIP.
  - LayerImplLwIP: Concrete implementation of LayerLwIP.
    (This is currently used by all LwIP-based platforms.)
- LayerSockets: Adds I/O event methods specific to builds using
  CHIP_SYSTEM_CONFIG_USING_SOCKETS.
  - LayerSocketsLoop: Adds methods for event-loop-based
    implementations.
    - LayerImplSelect: Concrete implementation of LayerSocketLoop,
      using select(). (This is currenly used by all sockets-based
      platforms.)
    - LayerImplLibevent: Concrete implementation of
      LayerSocketLoop, using libevent. (This is currenly a
      demonstration of substitutability, not used by any platform.)

Testing

CI; no changes to functionality. Tests updated where necessary to create
concrete LayerImpl objects.

#### Problem Issue project-chip#7715 _Virtualize System and Inet interfaces_ #### Change overview - Establishes an abstract `System::Layer` class hierarchy, described below. - Converts the transitional `WatchableEventManager` classes into corresponding `LayerImpl` classes. - Move `LwIPEventHandlerDelegate` into `LayerLwIP`. - Remove `GetClock()`, which was only used in one file. (`System::Clock` is currently fully static, but might benefit from its own virtualization, e.g. for test timeouts.) - For the moment, `CHIP_SYSTEM_CONFIG_USE_DISPATCH` is still included by `#if` on `LayerImplSelect`. With some changes to Inet, Dispatch can probably have its own much simpler implementation of `LayerSockets`. The class hierarchy is: - `Layer`: Core timer methods. - `LayerLwIP`: Adds methods specific to builds using `CHIP_SYSTEM_CONFIG_USING_LWIP`. - `LayerImplLwIP`: Concrete implementation of `LayerLwIP`. (This is currently used by all LwIP-based platforms.) - `LayerSockets`: Adds I/O event methods specific to builds using `CHIP_SYSTEM_CONFIG_USING_SOCKETS`. - `LayerSocketsLoop`: Adds methods for event-loop-based implementations. - `LayerImplSelect`: Concrete implementation of `LayerSocketLoop`, using `select()`. (This is currenly used by all sockets-based platforms.) - `LayerImplLibevent`: Concrete implementation of `LayerSocketLoop`, using `libevent`. (This is currenly a demonstration of substitutability, not used by any platform.) #### Testing CI; no changes to functionality. Tests updated where necessary to create concrete `LayerImpl` objects.

kpschoedel · 2021-08-31T14:53:05Z

~~A merge conflict with #9329 is expected.~~ Merged

src/system/SystemPacketBuffer.h

…3-abstract

github-actions · 2021-09-02T20:46:52Z

Size increase report for "gn_qpg-example-build" from 53dd583

File	Section	File	VM
chip-qpg6100-lighting-example.out	.text	84	84

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_str,0,227
.text,84,84
.shstrtab,0,-2
.debug_frame,0,-4
.debug_aranges,0,-48
.symtab,0,-80
[Unmapped],0,-84
.debug_ranges,0,-152
.strtab,0,-238
.debug_loc,0,-633
.debug_abbrev,0,-10552
.debug_line,0,-22565
.debug_info,0,-219105

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'

github-actions · 2021-09-02T21:36:55Z

Size increase report for "esp32-example-build" from 53dd583

File	Section	File	VM
chip-temperature-measurement-app.elf	.flash.rodata	52	52
chip-temperature-measurement-app.elf	.dram0.bss	0	-8
chip-temperature-measurement-app.elf	.flash.text	-68	-68
chip-bridge-app.elf	.flash.rodata	52	52
chip-bridge-app.elf	.flash.text	4	4
chip-bridge-app.elf	.dram0.bss	0	-8
chip-shell.elf	.flash.rodata	52	52
chip-shell.elf	[2 Others]	8	-8
chip-shell.elf	.flash.text	-16	-16
chip-lock-app.elf	.flash.rodata	52	52
chip-lock-app.elf	.dram0.bss	0	-8
chip-lock-app.elf	.flash.text	-72	-72
chip-all-clusters-app.elf	.flash.rodata	48	48
chip-all-clusters-app.elf	.dram0.bss	0	-8
chip-all-clusters-app.elf	.dram0.heap_start	0	-8
chip-all-clusters-app.elf	.flash.text	-12	-12

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_loc,0,287
.shstrtab,0,277
[ELF Section Headers],0,240
.debug_str,0,231
.debug_ranges,0,64
.flash.rodata,52,52
.xt.prop._ZN4chip6System13LayerImplLwIPD0Ev,0,36
.xt.prop._ZN4chip6System13LayerImplLwIPD2Ev,0,36
.xt.prop._ZNK4chip6System13LayerImplLwIP13IsInitializedEv,0,36
.symtab,0,32
[Unmapped],0,16
.xt.prop._ZTVN4chip6System13LayerImplLwIPE,0,12
.dram0.bss,-8,0
.debug_aranges,0,-48
.flash.text,-68,-68
.debug_frame,0,-88
.strtab,0,-289
.debug_abbrev,0,-15933
.debug_line,0,-32891
.debug_info,0,-341850

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

sections,vmsize,filesize
.debug_info,0,-8

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

sections,vmsize,filesize
.shstrtab,0,277
[ELF Section Headers],0,240
.debug_str,0,229
.debug_loc,0,206
.debug_ranges,0,64
.flash.rodata,52,52
.xt.prop._ZN4chip6System13LayerImplLwIPD0Ev,0,36
.xt.prop._ZN4chip6System13LayerImplLwIPD2Ev,0,36
.xt.prop._ZNK4chip6System13LayerImplLwIP13IsInitializedEv,0,36
.symtab,0,32
.xt.prop._ZTVN4chip6System13LayerImplLwIPE,0,12
.flash.text,4,4
.dram0.bss,-8,0
.debug_aranges,0,-48
[Unmapped],0,-56
.debug_frame,0,-88
.strtab,0,-289
.debug_abbrev,0,-16802
.debug_line,0,-34493
.debug_info,0,-359156

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

sections,vmsize,filesize
.shstrtab,0,277
.debug_str,0,240
[ELF Section Headers],0,240
.debug_loc,0,99
.debug_ranges,0,64
.flash.rodata,52,52
.xt.prop._ZN4chip6System13LayerImplLwIPD0Ev,0,36
.xt.prop._ZN4chip6System13LayerImplLwIPD2Ev,0,36
.xt.prop._ZNK4chip6System13LayerImplLwIP13IsInitializedEv,0,36
.symtab,0,32
.xt.prop._ZN4chip11DeviceLayer8Internal26GenericPlatformManagerImplINS0_19PlatformManagerImplEE26DispatchEventToDeviceLayerEPKNS0_15ChipDeviceEventE,0,12
.xt.prop._ZTVN4chip6System13LayerImplLwIPE,0,12
[2 Others],-8,8
.flash.text,-16,-16
[Unmapped],0,-36
.debug_aranges,0,-48
.debug_frame,0,-88
.strtab,0,-289
.debug_abbrev,0,-9281
.debug_line,0,-15792
.debug_info,0,-213846

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

sections,vmsize,filesize
.debug_abbrev,0,-286
.debug_line,0,-698
.debug_str,0,-5330
.debug_info,0,-11034

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

sections,vmsize,filesize
.shstrtab,0,277
[ELF Section Headers],0,240
.debug_str,0,222
.debug_ranges,0,64
.debug_loc,0,52
.flash.rodata,52,52
.xt.prop._ZN4chip6System13LayerImplLwIPD0Ev,0,36
.xt.prop._ZN4chip6System13LayerImplLwIPD2Ev,0,36
.xt.prop._ZNK4chip6System13LayerImplLwIP13IsInitializedEv,0,36
.symtab,0,32
[Unmapped],0,20
.xt.prop._ZTVN4chip6System13LayerImplLwIPE,0,12
.dram0.bss,-8,0
.debug_aranges,0,-48
.flash.text,-72,-72
.debug_frame,0,-88
.strtab,0,-289
.debug_abbrev,0,-15568
.debug_line,0,-34318
.debug_info,0,-399372

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

sections,vmsize,filesize
.debug_str,0,222
.flash.rodata,48,48
.debug_ranges,0,16
.riscv.attributes,0,-1
.shstrtab,0,-2
.dram0.bss,-8,0
.dram0.heap_start,-8,0
.flash.text,-12,-12
.symtab,0,-16
.debug_frame,0,-20
[Unmapped],0,-36
.debug_aranges,0,-48
.debug_loc,0,-151
.strtab,0,-186
.debug_abbrev,0,-19167
.debug_line,0,-38589
.debug_info,0,-468706

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

sections,vmsize,filesize
.debug_info,0,-8

github-actions · 2021-09-02T21:37:24Z

Size increase report for "nrfconnect-example-build" from 53dd583

File	Section	File	VM
chip-lock.elf	text	208	208
chip-lock.elf	rodata	96	96
chip-lock.elf	[LOAD #3 [RW]]	0	24
chip-lock.elf	bss	0	-24
chip-shell.elf	text	276	276
chip-shell.elf	rodata	96	96
chip-shell.elf	[LOAD #3 [RW]]	0	17
chip-shell.elf	device_handles	-4	-4
chip-shell.elf	bss	0	-17

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_str,0,680
text,208,208
rodata,96,96
.strtab,0,90
.symtab,0,80
[LOAD #3 [RW]],24,0
.shstrtab,0,2
bss,-24,0
.debug_aranges,0,-72
.debug_frame,0,-104
.debug_ranges,0,-176
.debug_loc,0,-1136
.debug_abbrev,0,-12719
.debug_line,0,-20385
.debug_info,0,-533176

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

sections,vmsize,filesize
.debug_str,0,680
text,276,276
.strtab,0,215
.symtab,0,144
rodata,96,96
[LOAD #3 [RW]],17,0
.shstrtab,0,1
device_handles,-4,-4
bss,-17,0
.debug_aranges,0,-72
.debug_frame,0,-104
.debug_ranges,0,-176
.debug_loc,0,-772
.debug_abbrev,0,-7255
.debug_line,0,-11497
.debug_info,0,-270332

src/inet/RawEndPoint.cpp

src/include/platform/internal/GenericPlatformManagerImpl.cpp

#### Problem 3a50501 project-chip#9366 caused a crash. In `AddEventHandlerDelegate()`, `lDelegate` was accidentally a stack variable instead of a reference to the caller-supplied storage. This affects all platforms using LwIP. #### Change overview Add one small but important `&`. Fixes project-chip#9563 _ESP32&ESP32C3 crash on system layer_ #### Testing all-clusters-app on M5Stack

#### Problem 3a50501 #9366 caused a crash. In `AddEventHandlerDelegate()`, `lDelegate` was accidentally a stack variable instead of a reference to the caller-supplied storage. This affects all platforms using LwIP. #### Change overview Add one small but important `&`. Fixes #9563 _ESP32&ESP32C3 crash on system layer_ #### Testing all-clusters-app on M5Stack

* Virtualize System::Layer interfaces. #### Problem Issue project-chip#7715 _Virtualize System and Inet interfaces_ #### Change overview - Establishes an abstract `System::Layer` class hierarchy, described below. - Converts the transitional `WatchableEventManager` classes into corresponding `LayerImpl` classes. - Move `LwIPEventHandlerDelegate` into `LayerLwIP`. - Remove `GetClock()`, which was only used in one file. (`System::Clock` is currently fully static, but might benefit from its own virtualization, e.g. for test timeouts.) - For the moment, `CHIP_SYSTEM_CONFIG_USE_DISPATCH` is still included by `#if` on `LayerImplSelect`. With some changes to Inet, Dispatch can probably have its own much simpler implementation of `LayerSockets`. The class hierarchy is: - `Layer`: Core timer methods. - `LayerLwIP`: Adds methods specific to builds using `CHIP_SYSTEM_CONFIG_USING_LWIP`. - `LayerImplLwIP`: Concrete implementation of `LayerLwIP`. (This is currently used by all LwIP-based platforms.) - `LayerSockets`: Adds I/O event methods specific to builds using `CHIP_SYSTEM_CONFIG_USING_SOCKETS`. - `LayerSocketsLoop`: Adds methods for event-loop-based implementations. - `LayerImplSelect`: Concrete implementation of `LayerSocketLoop`, using `select()`. (This is currenly used by all sockets-based platforms.) - `LayerImplLibevent`: Concrete implementation of `LayerSocketLoop`, using `libevent`. (This is currenly a demonstration of substitutability, not used by any platform.) #### Testing CI; no changes to functionality. Tests updated where necessary to create concrete `LayerImpl` objects. * remove debug code from SystemPacketBuffer.h

#### Problem 3a50501 project-chip#9366 caused a crash. In `AddEventHandlerDelegate()`, `lDelegate` was accidentally a stack variable instead of a reference to the caller-supplied storage. This affects all platforms using LwIP. #### Change overview Add one small but important `&`. Fixes project-chip#9563 _ESP32&ESP32C3 crash on system layer_ #### Testing all-clusters-app on M5Stack

boring-cyborg bot added app controller examples inet linux platform system labels Aug 31, 2021

pullapprove bot requested review from andy31415, chrisdecenzo, jelderton, jmartinez-silabs, LuDuda, mspang, saurabhst, turon, vivien-apple, wehale and woody-apple August 31, 2021 14:08

pullapprove bot added the review - pending label Aug 31, 2021

woody-apple added the SDK Approved label Aug 31, 2021

woody-apple approved these changes Aug 31, 2021

View reviewed changes

andy31415 reviewed Sep 2, 2021

View reviewed changes

src/system/SystemPacketBuffer.h Outdated Show resolved Hide resolved

andy31415 approved these changes Sep 2, 2021

View reviewed changes

kpschoedel added 3 commits September 2, 2021 11:14

Merge remote-tracking branch 'origin/master' into x7715-system-layer-…

15ef3a7

…3-abstract

remove debug code from SystemPacketBuffer.h

c84e3c8

Merge remote-tracking branch 'origin/master' into x7715-system-layer-…

adb3c00

…3-abstract

kpschoedel force-pushed the x7715-system-layer-3-abstract branch from ee5f8aa to adb3c00 Compare September 2, 2021 20:26

mspang reviewed Sep 3, 2021

View reviewed changes

src/inet/RawEndPoint.cpp Outdated Show resolved Hide resolved

jmartinez-silabs approved these changes Sep 7, 2021

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels Sep 7, 2021

andy31415 reviewed Sep 7, 2021

View reviewed changes

src/include/platform/internal/GenericPlatformManagerImpl.cpp Show resolved Hide resolved

andy31415 added the technical review - DONE label Sep 8, 2021

andy31415 merged commit 3a50501 into project-chip:master Sep 8, 2021

kpschoedel deleted the x7715-system-layer-3-abstract branch September 8, 2021 19:27

gjc13 mentioned this pull request Sep 9, 2021

fix TestDevice CI error #9560

Merged

wqx6 mentioned this pull request Sep 9, 2021

ESP32&ESP32C3 crash on system layer #9563

Closed

kpschoedel mentioned this pull request Sep 9, 2021

Fix EventHandlerDelegate crash on LwIP platforms #9569

Merged

cecille mentioned this pull request Sep 9, 2021

Crash on M5 #9540

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Virtualize System::Layer interfaces #9366

Virtualize System::Layer interfaces #9366

kpschoedel commented Aug 31, 2021

kpschoedel commented Aug 31, 2021 •

edited

Loading

github-actions bot commented Sep 2, 2021

github-actions bot commented Sep 2, 2021

github-actions bot commented Sep 2, 2021

Virtualize System::Layer interfaces #9366

Virtualize System::Layer interfaces #9366

Conversation

kpschoedel commented Aug 31, 2021

Problem

Change overview

Testing

kpschoedel commented Aug 31, 2021 • edited Loading

github-actions bot commented Sep 2, 2021

github-actions bot commented Sep 2, 2021

github-actions bot commented Sep 2, 2021

kpschoedel commented Aug 31, 2021 •

edited

Loading