From 812fc1eaead946b00f6b32387cf2381709fe793c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Maciejo=C5=84czyk?= <32327281+lmaciejonczyk@users.noreply.github.com> Date: Mon, 30 Aug 2021 21:41:16 +0200 Subject: [PATCH] [spinel] refactor connection handling for USB transport (#6963) This commit simplifies USB connection handling after resetting (hard reset) RCP device with USB transport. It drops udev dependency making the new approach generic for USB CDC ACM across all platform. It is working also on dockers. The feature is enabled by new radio Url parameter: 'uart-reset'. Radio url example: spinel+hdlc+uart:///dev/serial/by-id/SYMLINK_TO_RCP_DEVICE?uart-reset It is meant only for connection with RCP devices working with USB transport and performing hard reset on host reset command. --- configure.ac | 15 ---- examples/README.md | 1 - src/lib/spinel/openthread-spinel-config.h | 10 --- src/lib/spinel/radio_spinel_impl.hpp | 4 - src/posix/Makefile-posix | 6 -- src/posix/platform/CMakeLists.txt | 15 ---- src/posix/platform/hdlc_interface.cpp | 89 ++++------------------- src/posix/platform/hdlc_interface.hpp | 25 ++----- src/posix/platform/radio_url.cpp | 3 +- src/posix/platform/spi_interface.hpp | 7 ++ 10 files changed, 30 insertions(+), 145 deletions(-) diff --git a/configure.ac b/configure.ac index 8e9530452..f49566f38 100644 --- a/configure.ac +++ b/configure.ac @@ -614,21 +614,6 @@ AC_ARG_ENABLE(ncp, AC_MSG_RESULT(${enable_ncp}) AM_CONDITIONAL([OPENTHREAD_ENABLE_NCP], [test "${enable_ncp}" = "yes"]) -# -# Udev - udev library to use for POSIX NCP -# - -AC_ARG_WITH([udev], - AS_HELP_STRING([--with-udev], [Use udev library for monitoring tty events])) - -AS_IF([test "x$with_udev" = "xyes"], [ - AC_CHECK_LIB([udev], [udev_new]) - AC_CHECK_HEADER([libudev.h]) -]) - -AS_IF([test "x$ac_cv_lib_udev_udev_new" == "xno" || test "x$ac_cv_header_libudev_h" == "xno"], - [AC_MSG_ERROR([--with-udev was given, but test for udev failed])]) - # # Readline - readline library to use for POSIX CLI # diff --git a/examples/README.md b/examples/README.md index 85db8188c..87b3d55c5 100644 --- a/examples/README.md +++ b/examples/README.md @@ -54,7 +54,6 @@ This page lists the available common switches with description. Unless stated ot | OTNS | OT_OTNS | Enables support for [OpenThread Network Simulator](https://github.com/openthread/ot-ns). Enable this switch if you are building OpenThread for OpenThread Network Simulator. | | PLATFORM_UDP | OT_PLATFORM_UDP | Enables platform UDP support. | | REFERENCE_DEVICE | OT_REFERENCE_DEVICE | Enables support for Thread Test Harness reference device. Enable this switch on the reference device during certification. | -| RESET_CONNECTION | OT_SPINEL_RESET_CONNECTION | Enables resetting connection with RCP device. Enable this switch if RCP uses USB transport and performs hard reset. Use symlink instead of file name pointing the RCP device i.e. /dev/serial/by-id/usb-device-name instead of /dev/ttyUSB0 in a connection configuration. | | SERVICE | OT_SERVICE | Enables support for injecting Service entries into the Thread Network Data. | | SETTINGS_RAM | OT_SETTINGS_RAM | Enables volatile-only storage of settings. | | SLAAC | OT_SLAAC | Enables support for adding auto-configured SLAAC addresses by OpenThread. This feature is enabled by default. | diff --git a/src/lib/spinel/openthread-spinel-config.h b/src/lib/spinel/openthread-spinel-config.h index dd6c2253a..3cd91f243 100644 --- a/src/lib/spinel/openthread-spinel-config.h +++ b/src/lib/spinel/openthread-spinel-config.h @@ -55,14 +55,4 @@ #define OPENTHREAD_SPINEL_CONFIG_RCP_RESTORATION_MAX_COUNT 0 #endif -/** - * @def OPENTHREAD_SPINEL_CONFIG_RESET_CONNECTION - * - * Define 1 to reset connection after hard resetting RCP(USB CDC ACM) device. - * - */ -#ifndef OPENTHREAD_SPINEL_CONFIG_RESET_CONNECTION -#define OPENTHREAD_SPINEL_CONFIG_RESET_CONNECTION 0 -#endif - #endif // OPENTHREAD_SPINEL_CONFIG_H_ diff --git a/src/lib/spinel/radio_spinel_impl.hpp b/src/lib/spinel/radio_spinel_impl.hpp index 29ee3d874..86c12a9fa 100644 --- a/src/lib/spinel/radio_spinel_impl.hpp +++ b/src/lib/spinel/radio_spinel_impl.hpp @@ -234,9 +234,7 @@ void RadioSpinel::Init(bool aResetRadio, if (aResetRadio) { SuccessOrExit(error = SendReset()); -#if OPENTHREAD_SPINEL_CONFIG_RESET_CONNECTION SuccessOrDie(mSpinelInterface.ResetConnection()); -#endif } SuccessOrExit(error = WaitResponse()); @@ -2244,9 +2242,7 @@ void RadioSpinel::RecoverFromRcpFailure(void) if (mResetRadioOnStartup) { SuccessOrDie(SendReset()); -#if OPENTHREAD_SPINEL_CONFIG_RESET_CONNECTION SuccessOrDie(mSpinelInterface.ResetConnection()); -#endif } SuccessOrDie(WaitResponse()); diff --git a/src/posix/Makefile-posix b/src/posix/Makefile-posix index 07a1f43ba..11a3ed6fb 100644 --- a/src/posix/Makefile-posix +++ b/src/posix/Makefile-posix @@ -68,7 +68,6 @@ NETDATA_PUBLISHER ?= 1 PING_SENDER ?= 1 READLINE ?= readline REFERENCE_DEVICE ?= 1 -RESET_CONNECTION ?= 0 SERVICE ?= 1 SNTP_CLIENT ?= 1 SRP_CLIENT ?= 1 @@ -128,11 +127,6 @@ ifeq ($(VIRTUAL_TIME),1) COMMONCFLAGS += -DOPENTHREAD_POSIX_VIRTUAL_TIME=1 endif -ifeq ($(RESET_CONNECTION),1) -COMMONCFLAGS += -DOPENTHREAD_SPINEL_CONFIG_RESET_CONNECTION=1 -configure_OPTIONS += --with-udev -endif - include $(dir $(abspath $(lastword $(MAKEFILE_LIST))))/../../examples/common-switches.mk CPPFLAGS += \ diff --git a/src/posix/platform/CMakeLists.txt b/src/posix/platform/CMakeLists.txt index 5917d2b77..0e06d4eb2 100644 --- a/src/posix/platform/CMakeLists.txt +++ b/src/posix/platform/CMakeLists.txt @@ -62,20 +62,6 @@ if(NOT OT_CONFIG) set(OT_CONFIG "openthread-core-posix-config.h" PARENT_SCOPE) endif() -option(OT_SPINEL_RESET_CONNECTION "reset connection after hard resetting RCP(USB CDC ACM) device" OFF) -if (OT_SPINEL_RESET_CONNECTION) - target_compile_definitions(ot-posix-config - INTERFACE "OPENTHREAD_SPINEL_CONFIG_RESET_CONNECTION=1" - ) - - find_library(LIBUDEV "udev") - if (LIBUDEV) - set(UDEV_LINK_LIBRARIES "${LIBUDEV}") - else() - message(FATAL_ERROR "test for udev failed") - endif() -endif() - add_library(openthread-posix alarm.cpp backbone.cpp @@ -107,7 +93,6 @@ target_link_libraries(openthread-posix ot-config ot-posix-config util - ${UDEV_LINK_LIBRARIES} $<$:rt> ) diff --git a/src/posix/platform/hdlc_interface.cpp b/src/posix/platform/hdlc_interface.cpp index 484e29fd6..264a12392 100644 --- a/src/posix/platform/hdlc_interface.cpp +++ b/src/posix/platform/hdlc_interface.cpp @@ -47,9 +47,6 @@ #include #endif #endif -#if OPENTHREAD_SPINEL_CONFIG_RESET_CONNECTION -#include -#endif #include #include #include @@ -667,90 +664,34 @@ void HdlcInterface::HandleHdlcFrame(otError aError) } } -#if OPENTHREAD_SPINEL_CONFIG_RESET_CONNECTION otError HdlcInterface::ResetConnection(void) { - otError error = OT_ERROR_NONE; + otError error = OT_ERROR_NONE; + uint64_t end; - VerifyOrExit(mRadioUrl != nullptr, error = OT_ERROR_FAILED); - SuccessOrExit(error = WaitForUsbDevice(mRadioUrl->GetPath())); - - CloseFile(); - - mSockFd = OpenFile(*mRadioUrl); - VerifyOrExit(mSockFd != -1, error = OT_ERROR_FAILED); - -exit: - return error; -} - -otError HdlcInterface::WaitForUsbDevice(const char *aRadioUrlPath) -{ - int fd; - uint64_t end; - struct udev_monitor *mon; - otError error = OT_ERROR_NONE; - - struct udev *udev = udev_new(); - VerifyOrExit(udev != nullptr, error = OT_ERROR_FAILED); - - mon = udev_monitor_new_from_netlink(udev, "udev"); - udev_monitor_filter_add_match_subsystem_devtype(mon, "tty", NULL); - udev_monitor_enable_receiving(mon); - fd = udev_monitor_get_fd(mon); - - // wait maximally 10 seconds - end = otPlatTimeGet() + 10 * US_PER_S; - do + if (mRadioUrl->GetValue("uart-reset") != nullptr) { - int ret; - fd_set fds; - struct timeval tv; + usleep(static_cast(kRemoveRcpDelay) * US_PER_MS); + CloseFile(); - tv.tv_sec = 0; - tv.tv_usec = 100 * US_PER_MS; - - FD_ZERO(&fds); - FD_SET(fd, &fds); - - ret = select(fd + 1, &fds, NULL, NULL, &tv); - if (ret > 0 && FD_ISSET(fd, &fds)) + end = otPlatTimeGet() + kResetTimeout * US_PER_MS; + do { - struct udev_device *dev = udev_monitor_receive_device(mon); - if (dev) + mSockFd = OpenFile(*mRadioUrl); + if (mSockFd != -1) { - const char *action = udev_device_get_action(dev); - VerifyOrExit(action != nullptr, error = OT_ERROR_FAILED); - if (strcmp(action, "add") == 0) - { - struct udev_list_entry *entry; - udev_list_entry_foreach(entry, udev_device_get_devlinks_list_entry(dev)) - { - const char *name = udev_list_entry_get_name(entry); - VerifyOrExit(name != nullptr, error = OT_ERROR_FAILED); - if (strcmp(name, aRadioUrlPath) == 0) - { - udev_device_unref(dev); - ExitNow(); - } - } - } - udev_device_unref(dev); + ExitNow(); } - } - } while (end > otPlatTimeGet()); + usleep(static_cast(kOpenFileDelay) * US_PER_MS); + } while (end > otPlatTimeGet()); - error = OT_ERROR_FAILED; - -exit: - if (udev) - { - udev_unref(udev); + otLogCritPlat("Failed to reopen UART connection after resetting the RCP device."); + error = OT_ERROR_FAILED; } +exit: return error; } -#endif // OPENTHREAD_SPINEL_CONFIG_RESET_CONNECTION } // namespace Posix } // namespace ot diff --git a/src/posix/platform/hdlc_interface.hpp b/src/posix/platform/hdlc_interface.hpp index 92a7d07c7..73c1a3936 100644 --- a/src/posix/platform/hdlc_interface.hpp +++ b/src/posix/platform/hdlc_interface.hpp @@ -163,13 +163,11 @@ public: */ void OnRcpReset(void); -#if OPENTHREAD_SPINEL_CONFIG_RESET_CONNECTION /** * This method is called when RCP is reset to recreate the connection with it. * */ otError ResetConnection(void); -#endif private: /** @@ -237,29 +235,18 @@ private: */ void CloseFile(void); -#if OPENTHREAD_SPINEL_CONFIG_RESET_CONNECTION - /** - * This method waits until enumeration of RCP(USB CDC ACM) device ends. - * - * This is blocking call, this method waits for up to 10 seconds. - * - * @param[in] aRadioUrlPath A path to RCP device. - * - * @retval OT_ERROR_NONE The RCP device has been added to the host OS before timeout ends. - * @retval OT_ERROR_FAILED The RCP device has not been added to the host OS before timeout ends. - * - */ - otError WaitForUsbDevice(const char *aRadioUrlPath); -#endif - #if OPENTHREAD_POSIX_CONFIG_RCP_PTY_ENABLE static int ForkPty(const Url::Url &aRadioUrl); #endif enum { - kMaxFrameSize = Spinel::SpinelInterface::kMaxFrameSize, - kMaxWaitTime = 2000, ///< Maximum wait time in Milliseconds for socket to become writable (see `SendFrame`). + kMaxFrameSize = Spinel::SpinelInterface::kMaxFrameSize, + kMaxWaitTime = 2000, ///< Maximum wait time in Milliseconds for socket to become writable (see `SendFrame`). + kResetTimeout = 5000, ///< Maximum wait time in Milliseconds for file to become ready (see `ResetConnection`). + kOpenFileDelay = 500, ///< Delay between open file calls, in Milliseconds (see `ResetConnection`). + kRemoveRcpDelay = + 2000, ///< Delay for removing RCP device from host OS after hard reset (see `ResetConnection`). }; Spinel::SpinelInterface::ReceiveFrameCallback mReceiveFrameCallback; diff --git a/src/posix/platform/radio_url.cpp b/src/posix/platform/radio_url.cpp index ca0f160e4..b8ae8c5cb 100644 --- a/src/posix/platform/radio_url.cpp +++ b/src/posix/platform/radio_url.cpp @@ -73,7 +73,8 @@ const char *otSysGetRadioUrlHelpString(void) " uart-parity[=even|odd] Uart parity config, optional.\n" \ " uart-stop[=number-of-bits] Uart stop bit, default is 1.\n" \ " uart-baudrate[=baudrate] Uart baud rate, default is 115200.\n" \ - " uart-flow-control Enable flow control, disabled by default.\n" + " uart-flow-control Enable flow control, disabled by default.\n" \ + " uart-reset Reset connection after hard resetting RCP(USB CDC ACM).\n" #endif // OPENTHREAD_POSIX_CONFIG_RCP_BUS == OT_POSIX_RCP_BUS_SPI diff --git a/src/posix/platform/spi_interface.hpp b/src/posix/platform/spi_interface.hpp index 6d1e5e3e3..cff2bca8c 100644 --- a/src/posix/platform/spi_interface.hpp +++ b/src/posix/platform/spi_interface.hpp @@ -152,6 +152,13 @@ public: */ void OnRcpReset(void); + /** + * This method is called when RCP is reset to recreate the connection with it. + * Intentionally empty. + * + */ + otError ResetConnection(void) { return OT_ERROR_NONE; } + private: int SetupGpioHandle(int aFd, uint8_t aLine, uint32_t aHandleFlags, const char *aLabel); int SetupGpioEvent(int aFd, uint8_t aLine, uint32_t aHandleFlags, uint32_t aEventFlags, const char *aLabel);