Skip to content

Commit

Permalink
Separate System::Layer and Timer into per-implementation files.
Browse files Browse the repository at this point in the history
#### Problem

Various implementations of System::Layer are provided by `#if` sections,
which prevents mock versions for testing and out-of-tree platform
implementations.

#### Change overview

Split platform `#if` sections of `System::Layer` and `System::Timer`
into separate `WatchableEventManager` implementations.

This change does for timers what PR #6561 did for socket I/O events.
The intent is that the public `WatchableEventManager` interface here
will become pure virtual methods of `System::Layer`, and the various
versions of `WatchableEventManager` will become its implementations.

`System::Timer` remains as an `Object`-pool based utility class for
implementing the Timer API.

- Make `Timer` list not LwIP-specific, mutex it, and use it in preference
  to iterating through the entire pool.
- Move `IsEarlier` from `Timer` to `Clock`.
- Fix dual definitions of `Clock::Platform` functions on Linux.

#### Testing

Relying on CI in general, since no externally visible functionality
should have changed. Sanity-checked the libevent implementation (not in
CI) using chip-tool.
  • Loading branch information
kpschoedel committed Aug 4, 2021
1 parent 4f3fa5f commit 148b67d
Show file tree
Hide file tree
Showing 37 changed files with 1,249 additions and 1,076 deletions.
12 changes: 3 additions & 9 deletions examples/chip-tool/commands/common/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -422,16 +422,10 @@ static void OnResponseTimeout(chip::System::Layer *, void *, CHIP_ERROR)

CHIP_ERROR Command::ScheduleWaitForResponse(uint16_t seconds)
{
chip::System::Timer * timer = nullptr;

CHIP_ERROR err = chip::DeviceLayer::SystemLayer.NewTimer(timer);
if (err == CHIP_NO_ERROR)
{
timer->Start(seconds * 1000, OnResponseTimeout, this);
}
else
CHIP_ERROR err = chip::DeviceLayer::SystemLayer.StartTimer(seconds * 1000, OnResponseTimeout, this);
if (err != CHIP_NO_ERROR)
{
ChipLogError(chipTool, "Failed to allocate timer");
ChipLogError(chipTool, "Failed to allocate timer %" CHIP_ERROR_FORMAT, chip::ChipError::FormatError(err));
}
return err;
}
Expand Down
24 changes: 9 additions & 15 deletions examples/minimal-mdns/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -327,22 +327,16 @@ int main(int argc, char ** args)

BroadcastPacket(&mdnsServer);

System::Timer * timer = nullptr;

if (DeviceLayer::SystemLayer.NewTimer(timer) == CHIP_NO_ERROR)
{
timer->Start(
gOptions.runtimeMs,
[](System::Layer *, void *, CHIP_ERROR err) {
DeviceLayer::PlatformMgr().StopEventLoopTask();
DeviceLayer::PlatformMgr().Shutdown();
},
nullptr);
}
else
CHIP_ERROR err = DeviceLayer::SystemLayer.StartTimer(
gOptions.runtimeMs,
[](System::Layer *, void *, CHIP_ERROR err) {
DeviceLayer::PlatformMgr().StopEventLoopTask();
DeviceLayer::PlatformMgr().Shutdown();
},
nullptr);
if (err != CHIP_NO_ERROR)
{

printf("Failed to create the shutdown timer. Kill with ^C.\n");
printf("Failed to create the shutdown timer. Kill with ^C. %" CHIP_ERROR_FORMAT "\n", ChipError::FormatError(err));
}

DeviceLayer::PlatformMgr().RunEventLoop();
Expand Down
5 changes: 0 additions & 5 deletions src/ble/BleLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,6 @@ class DLL_EXPORT BleLayer
CHIP_ERROR CloseAllBleConnections();
CHIP_ERROR CloseBleConnection(BLE_CONNECTION_OBJECT connObj);

CHIP_ERROR ScheduleWork(chip::System::Layer::TimerCompleteFunct aComplete, void * aAppState)
{
return mSystemLayer->ScheduleWork(aComplete, aAppState);
}

/**< Platform interface functions:
* Calling conventions:
Expand Down
4 changes: 4 additions & 0 deletions src/darwin/Framework/CHIP.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,8 @@
"CHIP_DEVICE_LAYER_TARGET_NRF5=0",
"CHIP_DEVICE_LAYER_TARGET_EFR32=0",
"CHIP_SYSTEM_CONFIG_USE_SOCKETS=1",
"CHIP_SYSTEM_CONFIG_POSIX_LOCKING=0",
"CHIP_SYSTEM_CONFIG_NO_LOCKING=1",
"$(inherited)",
);
HEADER_SEARCH_PATHS = (
Expand Down Expand Up @@ -734,6 +736,8 @@
"CHIP_DEVICE_LAYER_TARGET_NRF5=0",
"CHIP_DEVICE_LAYER_TARGET_EFR32=0",
"CHIP_SYSTEM_CONFIG_USE_SOCKETS=1",
"CHIP_SYSTEM_CONFIG_POSIX_LOCKING=0",
"CHIP_SYSTEM_CONFIG_NO_LOCKING=1",
"$(inherited)",
);
HEADER_SEARCH_PATHS = (
Expand Down
11 changes: 3 additions & 8 deletions src/include/platform/PlatformManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <system/SystemLayer.h>

namespace chip {

namespace DeviceLayer {

class PlatformManagerImpl;
Expand All @@ -36,6 +37,7 @@ class ConfigurationManagerImpl;
class TraitManager;
class ThreadStackManagerImpl;
class TimeSyncManager;

namespace Internal {
class DeviceControlServer;
class FabricProvisioningServer;
Expand Down Expand Up @@ -174,14 +176,7 @@ class PlatformManager
friend class Internal::GenericThreadStackManagerImpl_OpenThread_LwIP;
template <class>
friend class Internal::GenericConfigurationManagerImpl;
// Parentheses used to fix clang parsing issue with these declarations
friend ::CHIP_ERROR(::chip::System::Platform::Eventing::PostEvent)(::chip::System::Layer & aLayer,
::chip::System::Object & aTarget,
::chip::System::EventType aType, uintptr_t aArgument);
friend ::CHIP_ERROR(::chip::System::Platform::Eventing::DispatchEvents)(::chip::System::Layer & aLayer);
friend ::CHIP_ERROR(::chip::System::Platform::Eventing::DispatchEvent)(::chip::System::Layer & aLayer,
::chip::System::Event aEvent);
friend ::CHIP_ERROR(::chip::System::Platform::Eventing::StartTimer)(::chip::System::Layer & aLayer, uint32_t aMilliseconds);
friend class System::PlatformEventing;

/*
* PostEvent can be called safely on any thread without locking the stack.
Expand Down
4 changes: 2 additions & 2 deletions src/include/platform/internal/GenericPlatformManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,8 @@ void GenericPlatformManagerImpl<ImplClass>::DispatchEventToSystemLayer(const Chi
CHIP_ERROR err = CHIP_NO_ERROR;

// Invoke the System Layer's event handler function.
err = SystemLayer.HandleEvent(*event->ChipSystemLayerEvent.Target, event->ChipSystemLayerEvent.Type,
event->ChipSystemLayerEvent.Argument);
err = SystemLayer.WatchableEvents().HandleEvent(*event->ChipSystemLayerEvent.Target, event->ChipSystemLayerEvent.Type,
event->ChipSystemLayerEvent.Argument);
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Error handling CHIP System Layer event (type %d): %s", event->Type, ErrorStr(err));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ void GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_RunEventLoop(void)

// Call into the system layer to dispatch the callback functions for all timers
// that have expired.
err = SystemLayer.HandlePlatformTimer();
err = SystemLayer.WatchableEvents().HandlePlatformTimer();
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Error handling CHIP timers: %s", ErrorStr(err));
Expand Down
4 changes: 2 additions & 2 deletions src/inet/IPEndPointBasis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -661,8 +661,8 @@ IPPacketInfo * IPEndPointBasis::GetPacketInfo(const System::PacketBufferHandle &
CHIP_ERROR IPEndPointBasis::PostPacketBufferEvent(chip::System::Layer & aLayer, System::Object & aTarget,
System::EventType aEventType, System::PacketBufferHandle && aBuffer)
{
const CHIP_ERROR error =
aLayer.PostEvent(aTarget, aEventType, (uintptr_t) System::LwIPPacketBufferView::UnsafeGetLwIPpbuf(aBuffer));
const CHIP_ERROR error = aLayer.WatchableEvents().PostEvent(
aTarget, aEventType, (uintptr_t) System::LwIPPacketBufferView::UnsafeGetLwIPpbuf(aBuffer));
if (error == CHIP_NO_ERROR)
{
// If PostEvent() succeeded, it has ownership of the buffer, so we need to release it (without freeing it).
Expand Down
2 changes: 1 addition & 1 deletion src/inet/InetLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ CHIP_ERROR InetLayer::Init(chip::System::Layer & aSystemLayer, void * aContext)
err = InitQueueLimiter();
SuccessOrExit(err);

mSystemLayer->AddEventHandlerDelegate(sInetEventHandlerDelegate);
mSystemLayer->WatchableEvents().AddEventHandlerDelegate(sInetEventHandlerDelegate);
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP

State = kState_Initialized;
Expand Down
2 changes: 1 addition & 1 deletion src/inet/RawEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ CHIP_ERROR RawEndPoint::Bind(IPAddressType addrType, const IPAddress & addr, Int
ReturnErrorOnFailure(IPEndPointBasis::Bind(addrType, addr, 0, intfId));

#if CHIP_SYSTEM_CONFIG_USE_DISPATCH
dispatch_queue_t dispatchQueue = SystemLayer().GetDispatchQueue();
dispatch_queue_t dispatchQueue = SystemLayer().WatchableEvents().GetDispatchQueue();
if (dispatchQueue != nullptr)
{
unsigned long fd = static_cast<unsigned long>(mSocket.GetFD());
Expand Down
18 changes: 10 additions & 8 deletions src/inet/TCPEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ CHIP_ERROR TCPEndPoint::Bind(IPAddressType addrType, const IPAddress & addr, uin
}

#if CHIP_SYSTEM_CONFIG_USE_DISPATCH
dispatch_queue_t dispatchQueue = SystemLayer().GetDispatchQueue();
dispatch_queue_t dispatchQueue = SystemLayer().WatchableEvents().GetDispatchQueue();
if (dispatchQueue != nullptr)
{
unsigned long fd = static_cast<unsigned long>(mSocket.GetFD());
Expand Down Expand Up @@ -2204,8 +2204,8 @@ err_t TCPEndPoint::LwIPHandleConnectComplete(void * arg, struct tcp_pcb * tpcb,

// Post callback to HandleConnectComplete.
conErr = chip::System::MapErrorLwIP(lwipErr);
if (lSystemLayer.PostEvent(*ep, kInetEvent_TCPConnectComplete, static_cast<uintptr_t>(ChipError::AsInteger(conErr))) !=
CHIP_NO_ERROR)
if (lSystemLayer.WatchableEvents().PostEvent(*ep, kInetEvent_TCPConnectComplete,
static_cast<uintptr_t>(ChipError::AsInteger(conErr))) != CHIP_NO_ERROR)
res = ERR_ABRT;
}
else
Expand Down Expand Up @@ -2270,7 +2270,8 @@ err_t TCPEndPoint::LwIPHandleIncomingConnection(void * arg, struct tcp_pcb * tpc
tcp_err(tpcb, LwIPHandleError);

// Post a callback to the HandleConnectionReceived() function, passing it the new end point.
if (lSystemLayer.PostEvent(*listenEP, kInetEvent_TCPConnectionReceived, (uintptr_t) conEP) != CHIP_NO_ERROR)
if (lSystemLayer.WatchableEvents().PostEvent(*listenEP, kInetEvent_TCPConnectionReceived, (uintptr_t) conEP) !=
CHIP_NO_ERROR)
{
err = CHIP_ERROR_CONNECTION_ABORTED;
conEP->Release(); // for the Retain() above
Expand All @@ -2280,7 +2281,8 @@ err_t TCPEndPoint::LwIPHandleIncomingConnection(void * arg, struct tcp_pcb * tpc

// Otherwise, there was an error accepting the connection, so post a callback to the HandleError function.
else
lSystemLayer.PostEvent(*listenEP, kInetEvent_TCPError, static_cast<uintptr_t>(ChipError::AsInteger(err)));
lSystemLayer.WatchableEvents().PostEvent(*listenEP, kInetEvent_TCPError,
static_cast<uintptr_t>(ChipError::AsInteger(err)));
}
else
err = CHIP_ERROR_CONNECTION_ABORTED;
Expand All @@ -2306,7 +2308,7 @@ err_t TCPEndPoint::LwIPHandleDataReceived(void * arg, struct tcp_pcb * tpcb, str
chip::System::Layer & lSystemLayer = ep->SystemLayer();

// Post callback to HandleDataReceived.
if (lSystemLayer.PostEvent(*ep, kInetEvent_TCPDataReceived, (uintptr_t) p) != CHIP_NO_ERROR)
if (lSystemLayer.WatchableEvents().PostEvent(*ep, kInetEvent_TCPDataReceived, (uintptr_t) p) != CHIP_NO_ERROR)
res = ERR_ABRT;
}
else
Expand All @@ -2328,7 +2330,7 @@ err_t TCPEndPoint::LwIPHandleDataSent(void * arg, struct tcp_pcb * tpcb, u16_t l
chip::System::Layer & lSystemLayer = ep->SystemLayer();

// Post callback to HandleDataReceived.
if (lSystemLayer.PostEvent(*ep, kInetEvent_TCPDataSent, (uintptr_t) len) != CHIP_NO_ERROR)
if (lSystemLayer.WatchableEvents().PostEvent(*ep, kInetEvent_TCPDataSent, (uintptr_t) len) != CHIP_NO_ERROR)
res = ERR_ABRT;
}
else
Expand Down Expand Up @@ -2357,7 +2359,7 @@ void TCPEndPoint::LwIPHandleError(void * arg, err_t lwipErr)

// Post callback to HandleError.
CHIP_ERROR err = chip::System::MapErrorLwIP(lwipErr);
lSystemLayer.PostEvent(*ep, kInetEvent_TCPError, static_cast<uintptr_t>(ChipError::AsInteger(err)));
lSystemLayer.WatchableEvents().PostEvent(*ep, kInetEvent_TCPError, static_cast<uintptr_t>(ChipError::AsInteger(err)));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/inet/UDPEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ CHIP_ERROR UDPEndPoint::Bind(IPAddressType addrType, const IPAddress & addr, uin
}

#if CHIP_SYSTEM_CONFIG_USE_DISPATCH
dispatch_queue_t dispatchQueue = SystemLayer().GetDispatchQueue();
dispatch_queue_t dispatchQueue = SystemLayer().WatchableEvents().GetDispatchQueue();
if (dispatchQueue != nullptr)
{
unsigned long fd = static_cast<unsigned long>(mSocket.GetFD());
Expand Down
4 changes: 2 additions & 2 deletions src/inet/tests/TestInetCommonPosix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ void ServiceEvents(struct ::timeval & aSleepTime)
{
if (sRemainingSystemLayerEventDelay == 0)
{
gSystemLayer.DispatchEvents();
gSystemLayer.WatchableEvents().DispatchEvents();
sRemainingSystemLayerEventDelay = gNetworkOptions.EventDelay;
}
else
Expand All @@ -478,7 +478,7 @@ void ServiceEvents(struct ::timeval & aSleepTime)
// TODO: Currently timers are delayed by aSleepTime above. A improved solution would have a mechanism to reduce
// aSleepTime according to the next timer.

gSystemLayer.HandlePlatformTimer();
gSystemLayer.WatchableEvents().HandlePlatformTimer();
}
}
#if CHIP_TARGET_STYLE_UNIX
Expand Down
1 change: 0 additions & 1 deletion src/platform/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ if (chip_device_platform != "none") {
"LockTracker.cpp",
"PersistedStorage.cpp",
"SystemEventSupport.cpp",
"SystemTimerSupport.cpp",
"TestIdentity.cpp",
]

Expand Down
2 changes: 1 addition & 1 deletion src/platform/Darwin/PlatformManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack()
err = Internal::GenericPlatformManagerImpl<PlatformManagerImpl>::_InitChipStack();
SuccessOrExit(err);

SystemLayer.SetDispatchQueue(GetWorkQueue());
SystemLayer.WatchableEvents().SetDispatchQueue(GetWorkQueue());

exit:
return err;
Expand Down
4 changes: 1 addition & 3 deletions src/platform/Linux/SystemPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ struct ChipDeviceEvent;
#define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 1
#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0
#define CHIP_SYSTEM_CONFIG_NO_LOCKING 0
#define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME 0

#define CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS 1
#define CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME 1

// ========== Platform-specific Configuration Overrides =========

Expand Down
16 changes: 9 additions & 7 deletions src/platform/SystemEventSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,11 @@

namespace chip {
namespace System {
namespace Platform {
namespace Eventing {

using namespace ::chip::DeviceLayer;

CHIP_ERROR PostEvent(System::Layer & aLayer, System::Object & aTarget, System::EventType aType, uintptr_t aArgument)
CHIP_ERROR PlatformEventing::PostEvent(System::Layer & aLayer, System::Object & aTarget, System::EventType aType,
uintptr_t aArgument)
{
ChipDeviceEvent event;
event.Type = DeviceEventType::kChipSystemLayerEvent;
Expand All @@ -47,21 +46,24 @@ CHIP_ERROR PostEvent(System::Layer & aLayer, System::Object & aTarget, System::E
return CHIP_NO_ERROR;
}

CHIP_ERROR DispatchEvents(System::Layer & aLayer)
CHIP_ERROR PlatformEventing::DispatchEvents(System::Layer & aLayer)
{
PlatformMgr().RunEventLoop();

return CHIP_NO_ERROR;
}

CHIP_ERROR DispatchEvent(System::Layer & aLayer, const ChipDeviceEvent * aEvent)
CHIP_ERROR PlatformEventing::DispatchEvent(System::Layer & aLayer, const ChipDeviceEvent * aEvent)
{
PlatformMgr().DispatchEvent(aEvent);

return CHIP_NO_ERROR;
}

} // namespace Eventing
} // namespace Platform
CHIP_ERROR PlatformEventing::StartTimer(System::Layer & aLayer, uint32_t aMilliseconds)
{
return PlatformMgr().StartChipTimer(aMilliseconds);
}

} // namespace System
} // namespace chip
45 changes: 0 additions & 45 deletions src/platform/SystemTimerSupport.cpp

This file was deleted.

Loading

0 comments on commit 148b67d

Please sign in to comment.