Separate System::Layer and Timer into per-implementation files.

#### 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 project-chip#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.
kpschoedel · Aug 4, 2021 · 617f9c1 · 617f9c1
1 parent 5e898ca
commit 617f9c1
Show file tree

Hide file tree

Showing 37 changed files with 1,249 additions and 1,076 deletions.
diff --git a/examples/chip-tool/commands/common/Command.cpp b/examples/chip-tool/commands/common/Command.cpp
@@ -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;
 }

diff --git a/examples/minimal-mdns/client.cpp b/examples/minimal-mdns/client.cpp
@@ -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();

diff --git a/src/ble/BleLayer.h b/src/ble/BleLayer.h
@@ -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:

diff --git a/src/darwin/Framework/CHIP.xcodeproj/project.pbxproj b/src/darwin/Framework/CHIP.xcodeproj/project.pbxproj
@@ -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 = (
@@ -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 = (

diff --git a/src/include/platform/PlatformManager.h b/src/include/platform/PlatformManager.h
@@ -28,6 +28,7 @@
 #include <system/SystemLayer.h>
 
 namespace chip {
+
 namespace DeviceLayer {
 
 class PlatformManagerImpl;
@@ -36,6 +37,7 @@ class ConfigurationManagerImpl;
 class TraitManager;
 class ThreadStackManagerImpl;
 class TimeSyncManager;
+
 namespace Internal {
 class DeviceControlServer;
 class FabricProvisioningServer;
@@ -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.

diff --git a/src/include/platform/internal/GenericPlatformManagerImpl.cpp b/src/include/platform/internal/GenericPlatformManagerImpl.cpp
@@ -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));

diff --git a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.cpp b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.cpp
@@ -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));

diff --git a/src/inet/IPEndPointBasis.cpp b/src/inet/IPEndPointBasis.cpp
@@ -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).

diff --git a/src/inet/InetLayer.cpp b/src/inet/InetLayer.cpp
@@ -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;

diff --git a/src/inet/RawEndPoint.cpp b/src/inet/RawEndPoint.cpp
@@ -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());

diff --git a/src/inet/TCPEndPoint.cpp b/src/inet/TCPEndPoint.cpp
@@ -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());
@@ -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
@@ -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
@@ -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;
@@ -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
@@ -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
@@ -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)));
     }
 }
 

diff --git a/src/inet/UDPEndPoint.cpp b/src/inet/UDPEndPoint.cpp
@@ -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());

diff --git a/src/inet/tests/TestInetCommonPosix.cpp b/src/inet/tests/TestInetCommonPosix.cpp
@@ -469,7 +469,7 @@ void ServiceEvents(struct ::timeval & aSleepTime)
         {
             if (sRemainingSystemLayerEventDelay == 0)
             {
-                gSystemLayer.DispatchEvents();
+                gSystemLayer.WatchableEvents().DispatchEvents();
                 sRemainingSystemLayerEventDelay = gNetworkOptions.EventDelay;
             }
             else
@@ -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

diff --git a/src/platform/BUILD.gn b/src/platform/BUILD.gn
@@ -254,7 +254,6 @@ if (chip_device_platform != "none") {
       "LockTracker.cpp",
       "PersistedStorage.cpp",
       "SystemEventSupport.cpp",
-      "SystemTimerSupport.cpp",
       "TestIdentity.cpp",
     ]
 

diff --git a/src/platform/Darwin/PlatformManagerImpl.cpp b/src/platform/Darwin/PlatformManagerImpl.cpp
@@ -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;

diff --git a/src/platform/Linux/SystemPlatformConfig.h b/src/platform/Linux/SystemPlatformConfig.h
@@ -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 =========
 

diff --git a/src/platform/SystemEventSupport.cpp b/src/platform/SystemEventSupport.cpp
@@ -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;
@@ -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
diff --git a/src/platform/SystemTimerSupport.cpp b/src/platform/SystemTimerSupport.cpp