Skip to content

Commit

Permalink
Virtualize System::Layer interfaces
Browse files Browse the repository at this point in the history
#### 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.)

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.
  • Loading branch information
kpschoedel committed Aug 30, 2021
1 parent fc87fb5 commit c8b38c5
Show file tree
Hide file tree
Showing 49 changed files with 695 additions and 786 deletions.
2 changes: 1 addition & 1 deletion examples/platform/nrfconnect/util/test/TestInetCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
using namespace chip;
using namespace chip::Inet;

System::Layer gSystemLayer;
System::LayerImpl gSystemLayer;

Inet::InetLayer gInet;

Expand Down
3 changes: 2 additions & 1 deletion src/app/tests/TestCommandInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include <support/ErrorStr.h>
#include <support/UnitTestRegistration.h>
#include <support/logging/CHIPLogging.h>
#include <system/SystemLayerImpl.h>
#include <system/SystemPacketBuffer.h>
#include <system/TLVPacketBufferBackingStore.h>
#include <transport/SecureSessionMgr.h>
Expand All @@ -48,7 +49,7 @@
#include <nlunit-test.h>

namespace chip {
static System::Layer gSystemLayer;
static System::LayerImpl gSystemLayer;
static SecureSessionMgr gSessionManager;
static Messaging::ExchangeManager gExchangeManager;
static TransportMgr<Transport::UDP> gTransportManager;
Expand Down
3 changes: 2 additions & 1 deletion src/app/tests/TestEventLogging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include <protocols/secure_channel/PASESession.h>
#include <support/ErrorStr.h>
#include <support/UnitTestRegistration.h>
#include <system/SystemLayerImpl.h>
#include <system/SystemPacketBuffer.h>
#include <system/TLVPacketBufferBackingStore.h>
#include <transport/SecureSessionMgr.h>
Expand All @@ -55,7 +56,7 @@ static const uint32_t kLivenessChangeEvent = 1;
static const chip::EndpointId kTestEndpointId = 2;
static const uint64_t kLivenessDeviceStatus = chip::TLV::ContextTag(1);
static chip::TransportMgr<chip::Transport::UDP> gTransportManager;
static chip::System::Layer gSystemLayer;
static chip::System::LayerImpl gSystemLayer;

static uint8_t gDebugEventBuffer[128];
static uint8_t gInfoEventBuffer[128];
Expand Down
3 changes: 2 additions & 1 deletion src/app/tests/TestInteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include <protocols/secure_channel/PASESession.h>
#include <support/ErrorStr.h>
#include <support/UnitTestRegistration.h>
#include <system/SystemLayerImpl.h>
#include <system/SystemPacketBuffer.h>
#include <system/TLVPacketBufferBackingStore.h>
#include <transport/SecureSessionMgr.h>
Expand All @@ -43,7 +44,7 @@
#include <nlunit-test.h>

namespace {
static chip::System::Layer gSystemLayer;
static chip::System::LayerImpl gSystemLayer;
static chip::SecureSessionMgr gSessionManager;
static chip::Messaging::ExchangeManager gExchangeManager;
static chip::secure_channel::MessageCounterManager gMessageCounterManager;
Expand Down
3 changes: 2 additions & 1 deletion src/app/tests/TestReportingEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <protocols/secure_channel/PASESession.h>
#include <support/ErrorStr.h>
#include <support/UnitTestRegistration.h>
#include <system/SystemLayerImpl.h>
#include <system/SystemPacketBuffer.h>
#include <system/TLVPacketBufferBackingStore.h>
#include <transport/SecureSessionMgr.h>
Expand All @@ -44,7 +45,7 @@
#include <nlunit-test.h>

namespace chip {
static System::Layer gSystemLayer;
static System::LayerImpl gSystemLayer;
static SecureSessionMgr gSessionManager;
static Messaging::ExchangeManager gExchangeManager;
static TransportMgr<Transport::UDP> gTransportManager;
Expand Down
16 changes: 8 additions & 8 deletions src/controller/java/CHIPDeviceController-JNI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#include <support/SafeInt.h>
#include <support/ThreadOperationalDataset.h>
#include <support/logging/CHIPLogging.h>
#include <system/SystemLayerImpl.h>

#include <zap-generated/CHIPClientCallbacks.h>
#include <zap-generated/CHIPClusters.h>
Expand Down Expand Up @@ -91,7 +92,7 @@ constexpr uint64_t kBreadcrumb = 0;
constexpr uint32_t kZclTimeoutMs = 10000;

JavaVM * sJVM;
System::Layer sSystemLayer;
System::LayerImpl sSystemLayer;
Inet::InetLayer sInetLayer;

#if CONFIG_NETWORK_LAYER_BLE
Expand Down Expand Up @@ -208,7 +209,7 @@ void JNI_OnUnload(JavaVM * jvm, void * reserved)
if (sIOThread != PTHREAD_NULL)
{
sShutdown = true;
sSystemLayer.WatchableEventsManager().Signal();
sSystemLayer.Signal();

StackUnlockGuard unlockGuard(JniReferences::GetInstance().GetStackLock());
pthread_join(sIOThread, NULL);
Expand Down Expand Up @@ -1043,18 +1044,17 @@ void * IOThreadMain(void * arg)
// Lock the stack to prevent collisions with Java threads.
pthread_mutex_lock(JniReferences::GetInstance().GetStackLock());

System::WatchableEventManager & watchState = sSystemLayer.WatchableEventsManager();
watchState.EventLoopBegins();
sSystemLayer.EventLoopBegins();

// Loop until we are told to exit.
while (!quit.load(std::memory_order_relaxed))
{
watchState.PrepareEvents();
sSystemLayer.PrepareEvents();

// Unlock the stack so that Java threads can make API calls.
pthread_mutex_unlock(JniReferences::GetInstance().GetStackLock());

watchState.WaitForEvents();
sSystemLayer.WaitForEvents();

// Break the loop if requested to shutdown.
// if (sShutdown)
Expand All @@ -1063,9 +1063,9 @@ void * IOThreadMain(void * arg)
// Re-lock the stack.
pthread_mutex_lock(JniReferences::GetInstance().GetStackLock());

watchState.HandleEvents();
sSystemLayer.HandleEvents();
}
watchState.EventLoopEnds();
sSystemLayer.EventLoopEnds();

// Detach the thread from the JVM.
sJVM->DetachCurrentThread();
Expand Down
3 changes: 2 additions & 1 deletion src/include/platform/CHIPDeviceLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <platform/GeneralUtils.h>
#include <platform/PlatformManager.h>
#include <system/SystemClock.h>
#include <system/SystemLayerImpl.h>
#if CHIP_DEVICE_CONFIG_ENABLE_SOFTWARE_UPDATE_MANAGER
#include <platform/SoftwareUpdateManager.h>
#endif // CHIP_DEVICE_CONFIG_ENABLE_SOFTWARE_UPDATE_MANAGER
Expand All @@ -44,7 +45,7 @@ namespace chip {
namespace DeviceLayer {

struct ChipDeviceEvent;
extern chip::System::Layer SystemLayer;
extern chip::System::LayerImpl SystemLayer;
extern Inet::InetLayer InetLayer;

} // namespace DeviceLayer
Expand Down
6 changes: 3 additions & 3 deletions src/include/platform/internal/GenericPlatformManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ CHIP_ERROR GenericPlatformManagerImpl<ImplClass>::_InitChipStack()
SuccessOrExit(err);

// Initialize the CHIP system layer.
new (&SystemLayer) System::Layer();
new (&SystemLayer) System::LayerImpl();
err = SystemLayer.Init();
if (err != CHIP_NO_ERROR)
{
Expand Down 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.WatchableEventsManager().HandleEvent(*event->ChipSystemLayerEvent.Target, event->ChipSystemLayerEvent.Type,
event->ChipSystemLayerEvent.Argument);
err = SystemLayer.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.WatchableEventsManager().HandlePlatformTimer();
err = SystemLayer.HandlePlatformTimer();
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Error handling CHIP timers: %s", ErrorStr(err));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ bool GenericPlatformManagerImpl_POSIX<ImplClass>::_IsChipStackLockedByCurrentThr
template <class ImplClass>
CHIP_ERROR GenericPlatformManagerImpl_POSIX<ImplClass>::_StartChipTimer(int64_t aMilliseconds)
{
// Let WatchableEventManager.PrepareEvents() handle timers.
// Let SystemLayer.PrepareEvents() handle timers.
return CHIP_NO_ERROR;
}

Expand All @@ -125,7 +125,7 @@ void GenericPlatformManagerImpl_POSIX<ImplClass>::_PostEvent(const ChipDeviceEve
{
mChipEventQueue.Push(*event);

SystemLayer.WatchableEventsManager().Signal(); // Trigger wake select on CHIP thread
SystemLayer.Signal(); // Trigger wake select on CHIP thread
}

template <class ImplClass>
Expand Down Expand Up @@ -160,21 +160,20 @@ void GenericPlatformManagerImpl_POSIX<ImplClass>::_RunEventLoop()

Impl()->LockChipStack();

System::WatchableEventManager & watchState = SystemLayer.WatchableEventsManager();
watchState.EventLoopBegins();
SystemLayer.EventLoopBegins();
do
{
watchState.PrepareEvents();
SystemLayer.PrepareEvents();

Impl()->UnlockChipStack();
watchState.WaitForEvents();
SystemLayer.WaitForEvents();
Impl()->LockChipStack();

watchState.HandleEvents();
SystemLayer.HandleEvents();

ProcessDeviceEvents();
} while (mShouldRunEventLoop.load(std::memory_order_relaxed));
watchState.EventLoopEnds();
SystemLayer.EventLoopEnds();

Impl()->UnlockChipStack();

Expand Down Expand Up @@ -255,7 +254,7 @@ CHIP_ERROR GenericPlatformManagerImpl_POSIX<ImplClass>::_StopEventLoopTask()
// SystemLayer.
//
Impl()->LockChipStack();
SystemLayer.WatchableEventsManager().Signal();
SystemLayer.Signal();
Impl()->UnlockChipStack();

pthread_mutex_lock(&mStateLock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ void GenericPlatformManagerImpl_Zephyr<ImplClass>::_UnlockChipStack(void)
template <class ImplClass>
CHIP_ERROR GenericPlatformManagerImpl_Zephyr<ImplClass>::_StartChipTimer(uint32_t aMilliseconds)
{
// Let WatchableEventManager.PrepareEvents() handle timers.
// Let Systemlayer.PrepareEvents() handle timers.
return CHIP_NO_ERROR;
}

Expand All @@ -106,7 +106,7 @@ void GenericPlatformManagerImpl_Zephyr<ImplClass>::_PostEvent(const ChipDeviceEv
// k_msgq_put takes `void*` instead of `const void*`. Nonetheless, it should be safe to
// const_cast here and there are components in Zephyr itself which do the same.
if (k_msgq_put(&mChipEventQueue, const_cast<ChipDeviceEvent *>(event), K_NO_WAIT) == 0)
SystemLayer.WatchableEventsManager().Signal(); // Trigger wake on CHIP thread
SystemLayer.Signal(); // Trigger wake on CHIP thread
else
ChipLogError(DeviceLayer, "Failed to post event to CHIP Platform event queue");
}
Expand All @@ -125,21 +125,20 @@ void GenericPlatformManagerImpl_Zephyr<ImplClass>::_RunEventLoop(void)
{
Impl()->LockChipStack();

System::WatchableEventManager & watchState = SystemLayer.WatchableEventsManager();
watchState.EventLoopBegins();
SystemLayer.EventLoopBegins();
while (true)
{
watchState.PrepareEvents();
SystemLayer.PrepareEvents();

Impl()->UnlockChipStack();
watchState.WaitForEvents();
SystemLayer.WaitForEvents();
Impl()->LockChipStack();

watchState.HandleEvents();
SystemLayer.HandleEvents();

ProcessDeviceEvents();
}
watchState.EventLoopEnds();
SystemLayer.EventLoopEnds();

Impl()->UnlockChipStack();
}
Expand Down
4 changes: 2 additions & 2 deletions src/inet/IPEndPointBasis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ IPPacketInfo * IPEndPointBasis::GetPacketInfo(const System::PacketBufferHandle &
return (lPacketInfo);
}

CHIP_ERROR IPEndPointBasis::PostPacketBufferEvent(chip::System::Layer & aLayer, System::Object & aTarget,
CHIP_ERROR IPEndPointBasis::PostPacketBufferEvent(chip::System::LayerLwIP & aLayer, System::Object & aTarget,
System::EventType aEventType, System::PacketBufferHandle && aBuffer)
{
const CHIP_ERROR error =
Expand Down Expand Up @@ -941,7 +941,7 @@ CHIP_ERROR IPEndPointBasis::GetSocket(IPAddressType aAddressType, int aType, int
mSocket = ::socket(family, aType, aProtocol);
if (mSocket == -1)
return chip::System::MapErrorPOSIX(errno);
ReturnErrorOnFailure(SystemLayer().StartWatchingSocket(mSocket, &mWatch));
ReturnErrorOnFailure(static_cast<System::LayerSockets &>(SystemLayer()).StartWatchingSocket(mSocket, &mWatch));

mAddrType = aAddressType;

Expand Down
4 changes: 2 additions & 2 deletions src/inet/IPEndPointBasis.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ class DLL_EXPORT IPEndPointBasis : public EndPointBasis
#if CHIP_SYSTEM_CONFIG_USE_LWIP
public:
static struct netif * FindNetifFromInterfaceId(InterfaceId aInterfaceId);
static CHIP_ERROR PostPacketBufferEvent(chip::System::Layer & aLayer, System::Object & aTarget, System::EventType aEventType,
System::PacketBufferHandle && aBuffer);
static CHIP_ERROR PostPacketBufferEvent(chip::System::LayerLwIP & aLayer, System::Object & aTarget,
System::EventType aEventType, System::PacketBufferHandle && aBuffer);

protected:
void HandleDataReceived(chip::System::PacketBufferHandle && aBuffer);
Expand Down
4 changes: 2 additions & 2 deletions src/inet/InetLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ InetLayer::InetLayer()
}

#if CHIP_SYSTEM_CONFIG_USE_LWIP
chip::System::LwIPEventHandlerDelegate InetLayer::sInetEventHandlerDelegate;
chip::System::LayerLwIP::EventHandlerDelegate InetLayer::sInetEventHandlerDelegate;
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP

#if INET_CONFIG_MAX_DROPPABLE_EVENTS && CHIP_SYSTEM_CONFIG_USE_LWIP
Expand Down Expand Up @@ -274,7 +274,7 @@ CHIP_ERROR InetLayer::Init(chip::System::Layer & aSystemLayer, void * aContext)
err = InitQueueLimiter();
SuccessOrExit(err);

mSystemLayer->AddEventHandlerDelegate(sInetEventHandlerDelegate);
static_cast<System::LayerLwIP *>(mSystemLayer)->AddEventHandlerDelegate(sInetEventHandlerDelegate);
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP

State = kState_Initialized;
Expand Down
2 changes: 1 addition & 1 deletion src/inet/InetLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ class DLL_EXPORT InetLayer
#if CHIP_SYSTEM_CONFIG_USE_LWIP
static CHIP_ERROR HandleInetLayerEvent(chip::System::Object & aTarget, chip::System::EventType aEventType, uintptr_t aArgument);

static chip::System::LwIPEventHandlerDelegate sInetEventHandlerDelegate;
static chip::System::LayerLwIP::EventHandlerDelegate sInetEventHandlerDelegate;

// In some implementations, there may be a shared event / message
// queue for the InetLayer used by other system events / messages.
Expand Down
21 changes: 11 additions & 10 deletions src/inet/RawEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,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 = static_cast<System::LayerImplSelect &>(SystemLayer()).GetDispatchQueue();
if (dispatchQueue != nullptr)
{
unsigned long fd = static_cast<unsigned long>(mSocket);
Expand Down Expand Up @@ -350,7 +350,7 @@ CHIP_ERROR RawEndPoint::BindIPv6LinkLocal(InterfaceId intfId, const IPAddress &

optfail:
res = chip::System::MapErrorPOSIX(errno);
SystemLayer().StopWatchingSocket(&mWatch);
static_cast<System::LayerSockets &>(SystemLayer()).StopWatchingSocket(&mWatch);
close(mSocket);
mSocket = INET_INVALID_SOCKET_FD;
mAddrType = kIPAddressType_Unknown;
Expand Down Expand Up @@ -424,8 +424,9 @@ CHIP_ERROR RawEndPoint::Listen(IPEndPointBasis::OnMessageReceivedFunct onMessage

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
// Wait for ability to read on this endpoint.
ReturnErrorOnFailure(SystemLayer().SetCallback(mWatch, HandlePendingIO, reinterpret_cast<intptr_t>(this)));
ReturnErrorOnFailure(SystemLayer().RequestCallbackOnPendingRead(mWatch));
ReturnErrorOnFailure(
static_cast<System::LayerSockets &>(SystemLayer()).SetCallback(mWatch, HandlePendingIO, reinterpret_cast<intptr_t>(this)));
ReturnErrorOnFailure(static_cast<System::LayerSockets &>(SystemLayer()).RequestCallbackOnPendingRead(mWatch));
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS

return CHIP_NO_ERROR;
Expand Down Expand Up @@ -467,7 +468,7 @@ void RawEndPoint::Close()
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
if (mSocket != INET_INVALID_SOCKET_FD)
{
SystemLayer().StopWatchingSocket(&mWatch);
static_cast<System::LayerSockets &>(SystemLayer()).StopWatchingSocket(&mWatch);
close(mSocket);
mSocket = INET_INVALID_SOCKET_FD;
}
Expand Down Expand Up @@ -930,11 +931,11 @@ u8_t RawEndPoint::LwIPReceiveRawMessage(void * arg, struct raw_pcb * pcb, struct
u8_t RawEndPoint::LwIPReceiveRawMessage(void * arg, struct raw_pcb * pcb, struct pbuf * p, ip_addr_t * addr)
#endif // LWIP_VERSION_MAJOR > 1 || LWIP_VERSION_MINOR >= 5
{
RawEndPoint * ep = static_cast<RawEndPoint *>(arg);
chip::System::Layer & lSystemLayer = ep->SystemLayer();
IPPacketInfo * pktInfo = NULL;
uint8_t enqueue = 1;
System::PacketBufferHandle buf = System::PacketBufferHandle::Adopt(p);
RawEndPoint * ep = static_cast<RawEndPoint *>(arg);
System::LayerLwIP & lSystemLayer = static_cast<System::LayerLwIP &>(ep->SystemLayer());
IPPacketInfo * pktInfo = NULL;
uint8_t enqueue = 1;
System::PacketBufferHandle buf = System::PacketBufferHandle::Adopt(p);

// Filtering based on the saved ICMP6 types (the only protocol currently supported.)
if ((ep->IPVer == kIPVersion_6) && (ep->IPProto == kIPProtocol_ICMPv6))
Expand Down
Loading

0 comments on commit c8b38c5

Please sign in to comment.