From 0c62a794105242989c4580a51e721d5e8cda6b37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=9Ean=20G=C3=BCne=C5=9F?= <180301198+sgunes-wirepas@users.noreply.github.com> Date: Thu, 25 Jun 2026 18:07:19 +0300 Subject: [PATCH 1/3] Remove termios and only use ioctl_tty on linux Earlier, serial interface was configured with termios, and ioctl_tty (TCSETS2/TCGETS2 ioctl calls) was used for configuring a custom baud rate because it is not possible with termios. There have been observations on some kernels that the TCSETS2 with custom baud rate reset the earlier VMIN/VTIME parameters set via termios. This should fix that since everything is now made with one ioctl call. Since this is linux specific serial implementation and we anyway depend on termios2 here, there is no harm in removing the termios part. This also simplifies the code. --- lib/platform/linux/CMakeLists.txt | 1 - lib/platform/linux/makefile | 3 +- lib/platform/linux/serial.c | 41 ++++++++++++++-------------- lib/platform/linux/serial_termios2.c | 41 ---------------------------- lib/platform/linux/serial_termios2.h | 13 --------- 5 files changed, 21 insertions(+), 78 deletions(-) delete mode 100644 lib/platform/linux/serial_termios2.c delete mode 100644 lib/platform/linux/serial_termios2.h diff --git a/lib/platform/linux/CMakeLists.txt b/lib/platform/linux/CMakeLists.txt index ee7dd6c..bcccc67 100644 --- a/lib/platform/linux/CMakeLists.txt +++ b/lib/platform/linux/CMakeLists.txt @@ -2,7 +2,6 @@ add_library(wpc_platform STATIC ${CMAKE_CURRENT_LIST_DIR}/logger.c ${CMAKE_CURRENT_LIST_DIR}/platform.c ${CMAKE_CURRENT_LIST_DIR}/serial.c - ${CMAKE_CURRENT_LIST_DIR}/serial_termios2.c ) target_include_directories(wpc_platform PRIVATE diff --git a/lib/platform/linux/makefile b/lib/platform/linux/makefile index 9f2b791..f0b96c2 100644 --- a/lib/platform/linux/makefile +++ b/lib/platform/linux/makefile @@ -3,8 +3,7 @@ PLATFORM_MODULE = $(SOURCEPREFIX)platform/linux SOURCES += $(PLATFORM_MODULE)/platform.c SOURCES += $(PLATFORM_MODULE)/serial.c -SOURCES += $(PLATFORM_MODULE)/serial_termios2.c SOURCES += $(PLATFORM_MODULE)/logger.c # Add the reentrant flag as using pthread lib -CFLAGS += -D_REENTRANT \ No newline at end of file +CFLAGS += -D_REENTRANT diff --git a/lib/platform/linux/serial.c b/lib/platform/linux/serial.c index 8db069f..3776b13 100644 --- a/lib/platform/linux/serial.c +++ b/lib/platform/linux/serial.c @@ -4,7 +4,6 @@ * */ #include -#include #include #include #include @@ -12,8 +11,9 @@ #include #include #include +#include +#include -#include "serial_termios2.h" #include "platform.h" #define LOG_MODULE_NAME "SERIAL" @@ -31,24 +31,21 @@ static char m_port_name[256]; /** \brief Bitrate to use */ static unsigned long m_bitrate; +// VMIN=0, VTIME=1 => blocking for max 100ms in non-canonical mode +static const cc_t SERIAL_VMIN = 0; +static const cc_t SERIAL_VTIME = 1; + static int set_interface_attribs(int fd, unsigned long bitrate, int parity) { - struct termios tty; + struct termios2 tty; struct serial_struct serial_s; - memset(&tty, 0, sizeof tty); - if (tcgetattr(fd, &tty) != 0) + if (ioctl(fd, TCGETS2, &tty) < 0) { - LOGE("Error %d from tcgetattr", errno); + LOGE("Error %d from TCGETS2\n", errno); return -1; } - // default to 9600 bps, but use TCSETS2 to set the actual bitrate - // use a bitrate that is not 115200 or 125000 bps here, to make sure - // TCSETS2 actually sets the bitrate - cfsetospeed(&tty, B9600); - cfsetispeed(&tty, B9600); - // 8-bit chars tty.c_cflag = (tty.c_cflag & ~CSIZE) | CS8; // disable break processing @@ -59,9 +56,9 @@ static int set_interface_attribs(int fd, unsigned long bitrate, int parity) tty.c_lflag = 0; // no remapping, no delays tty.c_oflag = 0; - // VMIN=0, VTIME=1 => blocking for max 100ms - tty.c_cc[VMIN] = 0; - tty.c_cc[VTIME] = 1; + + tty.c_cc[VMIN] = SERIAL_VMIN; + tty.c_cc[VTIME] = SERIAL_VTIME; // shut off xon/xoff ctrl tty.c_iflag &= ~(IXON | IXOFF | IXANY); @@ -74,17 +71,19 @@ static int set_interface_attribs(int fd, unsigned long bitrate, int parity) tty.c_cflag &= ~CSTOPB; tty.c_cflag &= ~CRTSCTS; - if (tcsetattr(fd, TCSANOW, &tty) != 0) - { - LOGE("Error %d from tcsetattr", errno); - return -1; - } + tty.c_cflag &= ~CBAUD; + tty.c_cflag |= BOTHER; + tty.c_ispeed = bitrate; + tty.c_ospeed = bitrate; - if (Serial_set_termios2_bitrate(fd, bitrate) != 0) + if (ioctl(fd, TCSETS2, &tty) < 0) { + LOGE("Error %d from TCSETS2\n", errno); return -1; } + LOGD("Custom bitrate set: %lu\n", bitrate); + // Set low latency flag to serial ioctl(fd, TIOCGSERIAL, &serial_s); serial_s.flags |= ASYNC_LOW_LATENCY; diff --git a/lib/platform/linux/serial_termios2.c b/lib/platform/linux/serial_termios2.c deleted file mode 100644 index 3c06503..0000000 --- a/lib/platform/linux/serial_termios2.c +++ /dev/null @@ -1,41 +0,0 @@ -/* Wirepas Oy licensed under Apache License, Version 2.0 - * - * See file LICENSE for full license details. - * - */ - -#include -#include -#include -#include - -#define LOG_MODULE_NAME "SERIAL" -#define MAX_LOG_LEVEL DEBUG_LOG_LEVEL -#include "logger.h" - -int Serial_set_termios2_bitrate(int fd, unsigned long bitrate) -{ - struct termios2 tty2; - - if (ioctl(fd, TCGETS2, &tty2) < 0) - { - LOGE("Error %d from TCGETS2", errno); - return -1; - } - - // set custom bitrate - tty2.c_cflag &= ~CBAUD; - tty2.c_cflag |= BOTHER; - tty2.c_ispeed = bitrate; - tty2.c_ospeed = bitrate; - - if (ioctl(fd, TCSETS2, &tty2) < 0) - { - LOGE("Error %d from TCSETS2", errno); - return -1; - } - - LOGD("Custom bitrate set: %lu\n", bitrate); - - return 0; -} diff --git a/lib/platform/linux/serial_termios2.h b/lib/platform/linux/serial_termios2.h deleted file mode 100644 index 259edd4..0000000 --- a/lib/platform/linux/serial_termios2.h +++ /dev/null @@ -1,13 +0,0 @@ -/* Wirepas Oy licensed under Apache License, Version 2.0 - * - * See file LICENSE for full license details. - * - */ - -/** - * \file serial_termios2.h - * A function to set a custom bitrate on Linux, using struct termios2. - * This needs to be in its own file due to Linux header file issues. - */ - -int Serial_set_termios2_bitrate(int fd, unsigned long bitrate); From 6ea933cee88fae4c11df5e28373c214c2ff813a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=9Ean=20G=C3=BCne=C5=9F?= <180301198+sgunes-wirepas@users.noreply.github.com> Date: Thu, 2 Jul 2026 18:04:22 +0300 Subject: [PATCH 2/3] Use non-blocking read with poll() for serial timeout Previously, read() was configured to block with a timeout using VMIN/VTIME and ICANON(c_lflag) attributes. These attributes belong to the shared serial device and they might be changed by another process. This can make read() calls block without a timeout. If read() starts blocking forver, it can cause a deadlock when multiple threads try to send a dualmcu api request and wait for response bytes. Since wpc_internal.c:check_if_timeout_reached_locked() is called in these threads after writing or reading bytes, the timeout mechanism might never kick in and program will not exit. TCSETS2 ioctl call might also report success if *any* of the given attributes were updated so it is not guaranteed that the serial has correct attributes after the set_interface_attribs() call. However, the other attributes being invalid is likely to cause data corruptions and other errors which should be more visible elsewhere (as opposed to being stuck in read()). --- lib/platform/linux/serial.c | 170 ++++++++++++++++++++++++++++++------ 1 file changed, 145 insertions(+), 25 deletions(-) diff --git a/lib/platform/linux/serial.c b/lib/platform/linux/serial.c index 3776b13..feb4693 100644 --- a/lib/platform/linux/serial.c +++ b/lib/platform/linux/serial.c @@ -9,7 +9,8 @@ #include #include #include -#include +#include +#include #include #include #include @@ -31,10 +32,6 @@ static char m_port_name[256]; /** \brief Bitrate to use */ static unsigned long m_bitrate; -// VMIN=0, VTIME=1 => blocking for max 100ms in non-canonical mode -static const cc_t SERIAL_VMIN = 0; -static const cc_t SERIAL_VTIME = 1; - static int set_interface_attribs(int fd, unsigned long bitrate, int parity) { struct termios2 tty; @@ -57,9 +54,6 @@ static int set_interface_attribs(int fd, unsigned long bitrate, int parity) // no remapping, no delays tty.c_oflag = 0; - tty.c_cc[VMIN] = SERIAL_VMIN; - tty.c_cc[VTIME] = SERIAL_VTIME; - // shut off xon/xoff ctrl tty.c_iflag &= ~(IXON | IXOFF | IXANY); @@ -94,7 +88,7 @@ static int set_interface_attribs(int fd, unsigned long bitrate, int parity) static int int_open() { - fd = open(m_port_name, O_RDWR | O_NOCTTY | O_SYNC); + fd = open(m_port_name, O_RDWR | O_NOCTTY | O_SYNC | O_NONBLOCK); if (fd < 0) { LOGE("Error %d opening serial link %s: %s\n", errno, m_port_name, strerror(errno)); @@ -143,13 +137,141 @@ int Serial_close() return 0; } +/* + * \brief Get timespec representing the deadline in the future + * \param[in] timeout_ms + * Timeout in milliseconds + * \return On success: returns a timespec for the time in future + * On failure: Logs a warning and returns a zero-filled dummy timespec + */ +static struct timespec get_deadline(const unsigned int timeout_ms) +{ + struct timespec deadline = {0}; + + struct timespec now; + if (0 != clock_gettime(CLOCK_MONOTONIC, &now)) + { + LOGW("Could not get current time, will have invalid deadline: %s\n", strerror(errno)); + return deadline; + } + + const time_t timeout_s = timeout_ms / 1000; + const int_fast32_t timeout_nsec = (timeout_ms % 1000) * 1000000; + + deadline.tv_sec = now.tv_sec + timeout_s; + deadline.tv_nsec = now.tv_nsec + timeout_nsec; + if (deadline.tv_nsec > 999999999) + { + deadline.tv_nsec -= 1000000000; + deadline.tv_sec++; + } + + return deadline; +} + +/* + * \brief Get remaining time in milliseconds based on the deadline + * \param[in] deadline + * Deadline + * \return If deadline has not passed: Returns remaining time in milliseconds + * If deadline has passed: Silently returns zero + * On failure: logs a warning and returns zero + */ +static int get_remaining_time_ms(const struct timespec *const deadline) +{ + struct timespec now; + if (0 != clock_gettime(CLOCK_MONOTONIC, &now)) + { + LOGW("Could not get current time, will assume deadline has passed: %s\n", strerror(errno)); + return 0; + } + + if ((deadline->tv_sec < now.tv_sec) || + (deadline->tv_sec == now.tv_sec && deadline->tv_nsec < now.tv_nsec)) + { + return 0; + } + + time_t remaining_sec = deadline->tv_sec - now.tv_sec; + int_fast32_t remaining_nsec = deadline->tv_nsec - now.tv_nsec; + if (remaining_nsec < 0) + { + remaining_sec--; + remaining_nsec += 1000000000; + + } + + return (remaining_sec * 1000) + (remaining_nsec / 1000000); +} + +/* + * \brief Poll the serial file descriptor with the given timeout + * + * Polls the serial file descriptor with the given timeout. If poll fails with + * EINTR due to a signal, retries again with the remaining timeout. + * + * \param[in] timeout_ms + * Timeout in milliseconds + * \return If an event occured on the file descriptor: Returns true + * On timeout: Silently returns false + * On failure: Logs an error and returns false + */ +static bool poll_serial_with_timeout(const unsigned int timeout_ms) +{ + struct pollfd pfd = { + .fd = fd, + .events = POLLIN + }; + + const struct timespec deadline = get_deadline(timeout_ms); + + int res; + bool interrupted; + int remaining = get_remaining_time_ms(&deadline); + do + { + res = poll(&pfd, 1, remaining); + interrupted = (res < 0 && errno == EINTR); + if (interrupted) + { + remaining = get_remaining_time_ms(&deadline); + if (remaining <= 0) + { + res = 0; + break; + } + } + } while (interrupted); + + if (res < 0) + { + LOGE("Error when waiting for char on serial line: %s\n", strerror(errno)); + return false; + } + + return res > 0; +} + +/* + * \brief Get a single byte from the serial line + * + * Returns the next buffered byte if one is available. Otherwise waits up to + * the given timeout for data on the serial line, refills the internal buffer + * with a single read and returns the first byte from it. + * + * \param[out] c + * Pointer where the read byte is stored + * \param[in] timeout_ms + * Timeout in milliseconds to wait + * \return If a byte was read: Returns 1 + * On timeout or if no data could be read: Returns 0 + */ static ssize_t get_single_char(unsigned char * c, unsigned int timeout_ms) { static unsigned char m_buffer[128]; static uint8_t m_elem = 0; static uint8_t m_read = 0; ssize_t read_bytes; - uint16_t attempts; // Do we still have char buffered? if (m_elem > 0) @@ -159,24 +281,22 @@ static ssize_t get_single_char(unsigned char * c, unsigned int timeout_ms) return 1; } - // Convert timeout_ms in attempt of 100ms (timeout set) - attempts = ((timeout_ms + 99) / 100) - 1; + if (!poll_serial_with_timeout(timeout_ms)) + { + LOGD("Timeout to wait for char on serial line\n"); + return 0; + } - // Local buffer is empty, refill it - do + read_bytes = read(fd, m_buffer, sizeof(m_buffer)); + if (read_bytes > 0) { - read_bytes = read(fd, m_buffer, sizeof(m_buffer)); - if (read_bytes > 0) - { - m_elem = read_bytes; - m_read = 0; - *c = m_buffer[m_read++]; - m_elem--; - return 1; - } - } while (attempts-- > 0); + m_elem = read_bytes; + m_read = 0; + *c = m_buffer[m_read++]; + m_elem--; + return 1; + } - LOGD("Timeout to wait for char on serial line\n"); return 0; } From 7a51dd8b7eecd4047ea7c1d4cb79ee9d8a8c153b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=9Ean=20G=C3=BCne=C5=9F?= <180301198+sgunes-wirepas@users.noreply.github.com> Date: Thu, 2 Jul 2026 18:48:17 +0300 Subject: [PATCH 3/3] Log read() failures This is similar to the earlier behavior; read() failures are treated as timeouts. Therefore, using debug log level for now. --- lib/platform/linux/serial.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/platform/linux/serial.c b/lib/platform/linux/serial.c index feb4693..ad1729a 100644 --- a/lib/platform/linux/serial.c +++ b/lib/platform/linux/serial.c @@ -297,6 +297,11 @@ static ssize_t get_single_char(unsigned char * c, unsigned int timeout_ms) return 1; } + if (read_bytes < 0) + { + LOGD("Reading the serial device failed, but treating it as a timeout: %s\n", strerror(errno)); + } + return 0; }