Skip to content

Commit

Permalink
Adapt System layer to Zephyr OS (#1594)
Browse files Browse the repository at this point in the history
* Adapt System layer to Zephyr socket API

The System layer was based on the assumption that when
BSD socket API is available then other POSIX functions
can be used as well. Unfortunately, this is not the case
in Zephyr which only implements a subset of POSIX
requirements.

Replace usage of pipe() system call with an equivalent
usage of socketpair(). Update KConfig of the door-lock
example for nrfconnect platform to set up networking
properly.

This only makes sure that libSystemLayer.a builds correctly
when NCS and the socket-based Inet layer is used. However,
the Inet layer itself still has some build issues which
need to be resolved.

* Use eventfd for system event

* Adapt to GN build system and add unit tests

Co-authored-by: Rafał Kuźnia <rafal.kuznia@nordicsemi.no>
  • Loading branch information
Damian-Nordic and e-rk authored Jul 17, 2020
1 parent 694f20d commit 6465fc7
Show file tree
Hide file tree
Showing 12 changed files with 467 additions and 49 deletions.
2 changes: 2 additions & 0 deletions src/system/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ static_library("system") {
"SystemStats.h",
"SystemTimer.cpp",
"SystemTimer.h",
"SystemWakeEvent.cpp",
"SystemWakeEvent.h",
"TimeSource.h",
]

Expand Down
16 changes: 16 additions & 0 deletions src/system/SystemConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -609,4 +609,20 @@ struct LwIPEvent;
#define CHIP_SYSTEM_CONFIG_VALID_REAL_TIME_THRESHOLD 946684800
#endif // CHIP_SYSTEM_CONFIG_VALID_REAL_TIME_THRESHOLD

/**
* @def CHIP_SYSTEM_CONFIG_USE_POSIX_PIPE
*
* @brief
* Use the POSIX pipe() function.
*
* Use the POSIX pipe() function to create an anonymous data stream.
*
* Defaults to enabled if the system is using sockets (except for Zephyr RTOS).
*/
#ifndef CHIP_SYSTEM_CONFIG_USE_POSIX_PIPE
#if (CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK) && !__ZEPHYR__
#define CHIP_SYSTEM_CONFIG_USE_POSIX_PIPE 1
#endif
#endif // CHIP_SYSTEM_CONFIG_USE_POSIX_PIPE

#endif // defined(SYSTEMCONFIG_H)
2 changes: 2 additions & 0 deletions src/system/SystemLayer.am
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ CHIP_BUILD_SYSTEM_LAYER_SOURCE_FILES = \
@top_builddir@/src/system/SystemLayer.cpp \
@top_builddir@/src/system/SystemMutex.cpp \
@top_builddir@/src/system/SystemObject.cpp \
@top_builddir@/src/system/SystemWakeEvent.cpp \
@top_builddir@/src/system/SystemTimer.cpp \
@top_builddir@/src/system/SystemPacketBuffer.cpp \
@top_builddir@/src/system/SystemStats.cpp \
Expand All @@ -48,6 +49,7 @@ CHIP_BUILD_SYSTEM_LAYER_HEADER_FILES = \
@top_builddir@/src/system/SystemLayer.h \
@top_builddir@/src/system/SystemMutex.h \
@top_builddir@/src/system/SystemObject.h \
@top_builddir@/src/system/SystemWakeEvent.h \
@top_builddir@/src/system/SystemTimer.h \
@top_builddir@/src/system/SystemPacketBuffer.h \
@top_builddir@/src/system/TimeSource.h \
Expand Down
59 changes: 13 additions & 46 deletions src/system/SystemLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,6 @@ Layer::Layer() : mLayerState(kLayerState_NotInitialized), mContext(NULL), mPlatf
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK
this->mWakePipeIn = 0;
this->mWakePipeOut = 0;

#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING
this->mHandleSelectThread = PTHREAD_NULL;
#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING
Expand All @@ -110,10 +107,6 @@ Layer::Layer() : mLayerState(kLayerState_NotInitialized), mContext(NULL), mPlatf
Error Layer::Init(void * aContext)
{
Error lReturn;
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK
int lPipeFDs[2];
int lOSReturn, lFlags;
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK

RegisterLayerErrorFormatter();
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
Expand All @@ -134,21 +127,9 @@ Error Layer::Init(void * aContext)
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK
// Create a Unix pipe to allow an arbitrary thread to wake the thread in the select loop.
lOSReturn = ::pipe(lPipeFDs);
VerifyOrExit(lOSReturn == 0, lReturn = chip::System::MapErrorPOSIX(errno));

this->mWakePipeIn = lPipeFDs[0];
this->mWakePipeOut = lPipeFDs[1];

// Enable non-blocking mode for both ends of the pipe.
lFlags = ::fcntl(this->mWakePipeIn, F_GETFL, 0);
lOSReturn = ::fcntl(this->mWakePipeIn, F_SETFL, lFlags | O_NONBLOCK);
VerifyOrExit(lOSReturn == 0, lReturn = chip::System::MapErrorPOSIX(errno));

lFlags = ::fcntl(this->mWakePipeOut, F_GETFL, 0);
lOSReturn = ::fcntl(this->mWakePipeOut, F_SETFL, lFlags | O_NONBLOCK);
VerifyOrExit(lOSReturn == 0, lReturn = chip::System::MapErrorPOSIX(errno));
// Create an event to allow an arbitrary thread to wake the thread in the select loop.
lReturn = this->mWakeEvent.Open();
SuccessOrExit(lReturn);
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK

this->mLayerState = kLayerState_Initialized;
Expand All @@ -172,12 +153,7 @@ Error Layer::Shutdown()
SuccessOrExit(lReturn);

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK
if (this->mWakePipeOut != -1)
{
::close(this->mWakePipeOut);
this->mWakePipeOut = -1;
this->mWakePipeIn = -1;
}
mWakeEvent.Close();
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK

for (size_t i = 0; i < Timer::sPool.Size(); ++i)
Expand Down Expand Up @@ -615,10 +591,11 @@ void Layer::PrepareSelect(int & aSetSize, fd_set * aReadSet, fd_set * aWriteSet,
if (this->State() != kLayerState_Initialized)
return;

if (this->mWakePipeIn + 1 > aSetSize)
aSetSize = this->mWakePipeIn + 1;
const int wakeEventFd = this->mWakeEvent.GetNotifFD();
FD_SET(wakeEventFd, aReadSet);

FD_SET(this->mWakePipeIn, aReadSet);
if (wakeEventFd + 1 > aSetSize)
aSetSize = wakeEventFd + 1;

const Timer::Epoch kCurrentEpoch = Timer::GetCurrentEpoch();
Timer::Epoch lAwakenEpoch = kCurrentEpoch + static_cast<Timer::Epoch>(aSleepTime.tv_sec) * 1000 + aSleepTime.tv_usec / 1000;
Expand Down Expand Up @@ -689,17 +666,9 @@ void Layer::HandleSelectResult(int aSetSize, fd_set * aReadSet, fd_set * aWriteS

if (aSetSize > 0)
{
// If we woke because of someone writing to the wake pipe, clear the contents of the pipe before returning.
if (FD_ISSET(this->mWakePipeIn, aReadSet))
{
while (true)
{
uint8_t lBytes[128];
int lTmp = ::read(this->mWakePipeIn, static_cast<void *>(lBytes), sizeof(lBytes));
if (lTmp < static_cast<int>(sizeof(lBytes)))
break;
}
}
// If we woke because of someone writing to the wake event, clear the event before returning.
if (FD_ISSET(this->mWakeEvent.GetNotifFD(), aReadSet))
this->mWakeEvent.Confirm();
}

const Timer::Epoch kCurrentEpoch = Timer::GetCurrentEpoch();
Expand Down Expand Up @@ -747,10 +716,8 @@ void Layer::WakeSelect()
}
#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING

// Write a single byte to the wake pipe to wake up the select call.
const uint8_t kByte = 0;
const ssize_t kIOResult = ::write(this->mWakePipeOut, &kByte, 1);
static_cast<void>(kIOResult);
// Send notification to wake up the select call.
this->mWakeEvent.Notify();
}

#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK
Expand Down
6 changes: 3 additions & 3 deletions src/system/SystemLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@

// Include dependent headers
#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
#include <system/SystemWakeEvent.h>

#include <sys/select.h>
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS

Expand Down Expand Up @@ -188,9 +190,7 @@ class DLL_EXPORT Layer
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK
int mWakePipeIn;
int mWakePipeOut;

SystemWakeEvent mWakeEvent;
#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING
pthread_t mHandleSelectThread;
#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING
Expand Down
127 changes: 127 additions & 0 deletions src/system/SystemWakeEvent.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/*
*
* Copyright (c) 2020 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* @file
* This file declares the abstraction of one-directional, anonymous
* data stream built on top of two file descriptors.
*/

#include <system/SystemWakeEvent.h>

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS

// Include additional CHIP headers
#include <support/CodeUtils.h>

// Include system and language headers
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>

#if !CHIP_SYSTEM_CONFIG_USE_POSIX_PIPE
#include <sys/eventfd.h>
#endif

namespace chip {
namespace System {

#if CHIP_SYSTEM_CONFIG_USE_POSIX_PIPE

namespace {
inline int SetNonBlockingMode(int fd)
{
int flags = ::fcntl(fd, F_GETFL, 0);
return ::fcntl(fd, F_SETFL, flags | O_NONBLOCK);
}
} // anonymous namespace

Error SystemWakeEvent::Open()
{
mFDs[FD_READ] = mFDs[FD_WRITE] = -1;

if (::pipe(mFDs) < 0)
return chip::System::MapErrorPOSIX(errno);

if (SetNonBlockingMode(mFDs[FD_READ]) < 0)
return chip::System::MapErrorPOSIX(errno);

if (SetNonBlockingMode(mFDs[FD_WRITE]) < 0)
return chip::System::MapErrorPOSIX(errno);

return CHIP_SYSTEM_NO_ERROR;
}

void SystemWakeEvent::Close()
{
::close(mFDs[FD_WRITE]);
::close(mFDs[FD_READ]);
mFDs[FD_READ] = mFDs[FD_WRITE] = -1;
}

void SystemWakeEvent::Confirm()
{
uint8_t buffer[128];

while (::read(mFDs[FD_READ], buffer, sizeof(buffer)) == sizeof(buffer))
continue;
}

void SystemWakeEvent::Notify()
{
char byte = 1;
::write(mFDs[FD_WRITE], &byte, 1);
}

#else // CHIP_SYSTEM_CONFIG_USE_POSIX_PIPE

Error SystemWakeEvent::Open()
{
mFD = eventfd(0, EFD_NONBLOCK);

if (mFD == -1)
{
return chip::System::MapErrorPOSIX(errno);
}

return CHIP_SYSTEM_NO_ERROR;
}

void SystemWakeEvent::Close()
{
::close(mFD);
mFD = -1;
}

void SystemWakeEvent::Confirm()
{
uint64_t value;
::read(mFD, &value, sizeof(value));
}

void SystemWakeEvent::Notify()
{
uint64_t value = 1;
::write(mFD, &value, sizeof(value));
}

#endif // CHIP_SYSTEM_CONFIG_USE_POSIX_PIPE

} // namespace System
} // namespace chip

#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS
69 changes: 69 additions & 0 deletions src/system/SystemWakeEvent.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
*
* Copyright (c) 2020 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* @file
* This file declares the abstraction of system wake event used for
* resuming task from select system call.
*/

#ifndef SYSTEMWAKEEVENT_H
#define SYSTEMWAKEEVENT_H

// Include configuration headers
#include <system/SystemConfig.h>

#include <system/SystemError.h>

namespace chip {
namespace System {

using ::chip::System::Error;

class SystemWakeEvent
{
public:
Error Open(); /**< Initialize the pipeline */
void Close(); /**< Close both ends of the pipeline. */

#if CHIP_SYSTEM_CONFIG_USE_POSIX_PIPE
int GetNotifFD() const { return mFDs[FD_READ]; }
#else
int GetNotifFD() const { return mFD; }
#endif

void Notify(); /**< Set the event. */
void Confirm(); /**< Clear the event. */

private:
#if CHIP_SYSTEM_CONFIG_USE_POSIX_PIPE
enum
{
FD_READ = 0,
FD_WRITE = 1
};

int mFDs[2];
#else
int mFD;
#endif
};

} // namespace System
} // namespace chip

#endif // SYSTEMWAKEEVENT_H
2 changes: 2 additions & 0 deletions src/system/tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ chip_test_suite("tests") {
"TestSystemObject.cpp",
"TestSystemPacketBuffer.cpp",
"TestSystemTimer.cpp",
"TestSystemWakeEvent.cpp",
"TestTimeSource.cpp",
]

Expand All @@ -40,6 +41,7 @@ chip_test_suite("tests") {
"TestSystemObject",
"TestSystemPacketBuffer",
"TestSystemTimer",
"TestSystemWakeEvent",
"TestTimeSource",
]
}
Loading

0 comments on commit 6465fc7

Please sign in to comment.