-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Virtualize System::Layer interfaces #9366
Merged
andy31415
merged 4 commits into
project-chip:master
from
kpschoedel:x7715-system-layer-3-abstract
Sep 8, 2021
Merged
Virtualize System::Layer interfaces #9366
andy31415
merged 4 commits into
project-chip:master
from
kpschoedel:x7715-system-layer-3-abstract
Sep 8, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
#### 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.
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
woody-apple
approved these changes
Aug 31, 2021
|
andy31415
reviewed
Sep 2, 2021
andy31415
approved these changes
Sep 2, 2021
kpschoedel
force-pushed
the
x7715-system-layer-3-abstract
branch
from
September 2, 2021 20:26
ee5f8aa
to
adb3c00
Compare
Size increase report for "gn_qpg-example-build" from 53dd583
Full report output
|
Size increase report for "esp32-example-build" from 53dd583
Full report output
|
Size increase report for "nrfconnect-example-build" from 53dd583
Full report output
|
mspang
reviewed
Sep 3, 2021
jmartinez-silabs
approved these changes
Sep 7, 2021
andy31415
reviewed
Sep 7, 2021
kpschoedel
added a commit
to kpschoedel/connectedhomeip
that referenced
this pull request
Sep 9, 2021
#### 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
andy31415
pushed a commit
that referenced
this pull request
Sep 9, 2021
#### 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
Closed
kpschoedel
added a commit
to kpschoedel/connectedhomeip
that referenced
this pull request
Sep 9, 2021
* 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
kpschoedel
added a commit
to kpschoedel/connectedhomeip
that referenced
this pull request
Sep 9, 2021
#### 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Issue #7715 Virtualize System and Inet interfaces
Change overview
System::Layer
class hierarchy, described below.WatchableEventManager
classes intocorresponding
LayerImpl
classes.LwIPEventHandlerDelegate
intoLayerLwIP
.GetClock()
, which was only used in one file.(
System::Clock
is currently fully static, but might benefit from itsown virtualization, e.g. for test timeouts.)
CHIP_SYSTEM_CONFIG_USE_DISPATCH
is still includedby
#if
onLayerImplSelect
. With some changes to Inet, Dispatch canprobably have its own much simpler implementation of
LayerSockets
.The class hierarchy is:
Layer
: Core timer methods.LayerLwIP
: Adds methods specific to builds usingCHIP_SYSTEM_CONFIG_USING_LWIP
.LayerImplLwIP
: Concrete implementation ofLayerLwIP
.(This is currently used by all LwIP-based platforms.)
LayerSockets
: Adds I/O event methods specific to builds usingCHIP_SYSTEM_CONFIG_USING_SOCKETS
.LayerSocketsLoop
: Adds methods for event-loop-basedimplementations.
LayerImplSelect
: Concrete implementation ofLayerSocketLoop
,using
select()
. (This is currenly used by all sockets-basedplatforms.)
LayerImplLibevent
: Concrete implementation ofLayerSocketLoop
, usinglibevent
. (This is currenly ademonstration of substitutability, not used by any platform.)
Testing
CI; no changes to functionality. Tests updated where necessary to create
concrete
LayerImpl
objects.