From d16f4bdaf730cb7e953d639ea9dd516fdf8c557d Mon Sep 17 00:00:00 2001 From: Andrey Bychkov Date: Mon, 31 Aug 2026 18:27:09 +0300 Subject: [PATCH] feat: decouple PISerial from PIP_HAS_THREADS, gate termios/stty code by PIP_HAS_TTY - CMake: drop PIP_HAS_SERIAL -> PIP_HAS_THREADS dependency and the forced PIP_HAS_SERIAL=OFF for PICO_BOARD, so serial can be built on Pico - piserial.cpp: guard termios/stty code (tcgetattr/tcsetattr/tcflush/tcdrain, TIOCSBRK/TIOCCBRK, TIOCMBIS/TIOCMBIC/TIOCMGET, termios members, sys/ioctl.h and termios.h includes) with PIP_HAS_TTY; O_NOCTTY replaced by PISERIAL_NOCTTY (0 on no-TTY targets) - add missing #ifndef fallbacks for B110..B115200 used by convertSpeed() when termios.h is absent - guard thread-only PIIODevice API calls (isThreadedRead/stopThreadedRead) with PIP_HAS_THREADS in closeDevice/readDevice - gate the availableDevicesInfo() test loop with PIP_HAS_FILESYSTEM||WINDOWS (device enumeration is a filesystem feature) - AGENTS.md: note that clang-format-18 is applied automatically on file edits --- AGENTS.md | 20 ++++--- CMakeLists.txt | 2 - libs/main/io_devices/piserial.cpp | 93 ++++++++++++++++++++++++++----- 3 files changed, 90 insertions(+), 25 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0c982375..58b9dbe1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,6 +15,7 @@ cmake --build build -j16 - Embedded builds: `PICO_BOARD` (Raspberry Pi Pico SDK, see `esp-pip/`) or `PIP_FREERTOS`/PlatformIO (`library.json`, `platformio_pre.py`) force `LOCAL` install and disable most modules. ### Key options + - `TESTS=ON` — build gtest suites; `TESTS_RUN=ON` — additionally register tests with CTest (**without it `ctest` finds nothing**). - `COVERAGE=ON` (needs gcov), `STD_IOSTREAM=ON`, `INTROSPECTION=ON` (slows down), `LOCAL=ON` (install to build/bin|lib|include). - `ICU=ON` — OFF by default (README claims ON; trust CMakeLists). @@ -22,7 +23,8 @@ cmake --build build -j16 - `PIP_HAS_` — feature flags (`FILESYSTEM THREADS SOCKET PROCESS DYNLIB FFT SERIAL`), default ON. A dependency table auto-disables dependents (e.g. `PIP_HAS_SOCKET=OFF` turns off `cloud`, `client_server`, `http_server`, `mqtt_client`). ### Targets -- Main library target is **`pip`** (`libpip.so`); feature modules are `pip_` (`pip_console`, `pip_crypt`, ...). + +- Main library target is `**pip**` (`libpip.so`); feature modules are `pip_` (`pip_console`, `pip_crypt`, ...). - `pip_test` (from root `main.cpp`, built only with `PIP_MANUAL_TEST=ON` + curl + microhttpd) is a manual dev harness, **not** a gtest suite. ## Tests @@ -35,28 +37,29 @@ cmake --build build -j16 ./tests/pip_math_test --gtest_filter="Vector2DTest.*" ``` Note: `ctest -R math` matches nothing (names are PascalCase). -- Test sources: `tests//test.cpp`, added via `pip_test()` in `tests/CMakeLists.txt`. +- Test sources: `tests/[[ORCA_RICH_MD:7a80511347c7c70281266e426adc97ca:inline-html:%3Csuite%3E]]/test[[ORCA_RICH_MD:7a80511347c7c70281266e426adc97ca:inline-html:%3Cname%3E]].cpp`, added via `pip_test([[ORCA_RICH_MD:7a80511347c7c70281266e426adc97ca:inline-html:%3Csuite%3E]])` in `tests/CMakeLists.txt`. - A `doc` CMake target builds Doxygen docs when Doxygen is installed. ## Architecture -- **Module header/source split is non-obvious**: public headers of feature modules live in `libs/main//`, their sources in `libs//` (e.g. `libs/main/crypt/picrypt.h` + `libs/crypt/picrypt.cpp`). All of `libs/main/**` compiles into the `pip` target. +- **Module header/source split is non-obvious**: public headers of feature modules live in `libs/main/[[ORCA_RICH_MD:7a80511347c7c70281266e426adc97ca:inline-html:%3Cmodule%3E]]/`, their sources in `libs/[[ORCA_RICH_MD:7a80511347c7c70281266e426adc97ca:inline-html:%3Cmodule%3E]]/` (e.g. `libs/main/crypt/picrypt.h` + `libs/crypt/picrypt.cpp`). All of `libs/main/**` compiles into the `pip` target. - `libs/main/` core areas: `core` (PIObject, events), `containers`, `types`, `text`, `math`, `thread`, `system`, `io_devices`, `serialization`, `units`, `geo`, `resources`, `introspection`. - Bundled in `3rd/`: PCRE2 (built 16-bit static only), BLAKE2, SipHash, Lua + LuaBridge, paho.mqtt.c. -- Feature-flag macros (`PIP_HAS_*`, `PIP__EXPORT`, ...) are defined per-target by CMake; guard platform-specific code with them, not raw OS macros where a flag exists. +- Feature-flag macros (`PIP_HAS_*`, `PIP_[[ORCA_RICH_MD:7a80511347c7c70281266e426adc97ca:inline-html:%3CMODULE%3E]]_EXPORT`, ...) are defined per-target by CMake; guard platform-specific code with them, not raw OS macros where a flag exists. - Non-Android POSIX builds use `-fno-exceptions`; MSVC uses `/EH-`. Avoid exceptions/RTTI in new code. ## Codegen tools (built under `utils/`) -- `pip_cmg` — code model generator; downstream CMake: `pip_code_model( files...)` macro. -- `pip_rc` — resource compiler (`.conf` → C++); downstream CMake: `pip_resources( file)`. +- `pip_cmg` — code model generator; downstream CMake: `pip_code_model([[ORCA_RICH_MD:7a80511347c7c70281266e426adc97ca:inline-html:%3Cout%3E]] files...)` macro. +- `pip_rc` — resource compiler (`.conf` → C++); downstream CMake: `pip_resources([[ORCA_RICH_MD:7a80511347c7c70281266e426adc97ca:inline-html:%3Cout%3E]] file)`. - `pip_tr` — translation compiler (`lang/*.btf`); `deploy_tool` — version/deploy metadata. ## Style -- `clang-format` is the authority (`.clang-format`: 140 cols, 4-space indent, sorted case-sensitive includes, C++11). Run `clang-format -i ` on touched files. +- `clang-format-18` is the authority (`.clang-format`: 140 cols, 4-space indent, sorted case-sensitive includes, C++11). +- The LSP auto-applies `clang-format-18` on file edits. - Doxygen comments on public APIs are **bilingual**: `\~english` + `\~russian` lines (see `libs/main/core/piobject.h`). -- Naming: classes `PascalCase`, methods/vars `camelCase`; files `pi.h`, private `pi_p.h`, tests `test.cpp`. +- Naming: classes `PascalCase`, methods/vars `camelCase`; files `pi[[ORCA_RICH_MD:7a80511347c7c70281266e426adc97ca:inline-html:%3Cname%3E]].h`, private `pi[[ORCA_RICH_MD:7a80511347c7c70281266e426adc97ca:inline-html:%3Cname%3E]]_p.h`, tests `test[[ORCA_RICH_MD:7a80511347c7c70281266e426adc97ca:inline-html:%3Cname%3E]].cpp`. - `NO_COPY_CLASS(X)` disables copies; `PIP_EXPORT` marks exported symbols. ## Repo notes @@ -64,3 +67,4 @@ cmake --build build -j16 - `AGENTS.md`, `build*/`, `plans/` are gitignored (local-only). - `.guides/BUGHUNT_GUIDE.md` (Russian) documents the bug-hunting workflow; `.guides/BUGHUNT_FALSE_POSITIVES.md` lists already-verified non-bugs — read it before flagging similar patterns. - `plans/` holds in-progress work plans (e.g. `wasm_build.md`); check before touching related code. + diff --git a/CMakeLists.txt b/CMakeLists.txt index fa49c805..7cf1018c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -242,7 +242,6 @@ if(DEFINED PICO_BOARD OR PIP_FREERTOS) set(PIP_HAS_THREADS OFF CACHE BOOL "" FORCE) endif() set(PIP_HAS_FILESYSTEM OFF CACHE BOOL "" FORCE) - set(PIP_HAS_SERIAL OFF CACHE BOOL "" FORCE) set(PIP_HAS_SOCKET OFF CACHE BOOL "" FORCE) set(PIP_BUILD_MQTT_CLIENT OFF CACHE BOOL "" FORCE) message(STATUS "Building PIP for Pi Pico SDK ${PICO_SDK_VERSION_STRING}") @@ -269,7 +268,6 @@ set(PIP_HAS_DEPS_TARGETS "PIP_BUILD_FFTW" "PIP_BUILD_HTTP_CLIENT" "PIP_BUILD_HTTP_SERVER" "PIP_BUILD_MQTT_CLIENT") set(PIP_HAS_DEPS_PIP_HAS_PROCESS "PIP_HAS_THREADS") -set(PIP_HAS_DEPS_PIP_HAS_SERIAL "PIP_HAS_THREADS") set(PIP_HAS_DEPS_INTROSPECTION "PIP_HAS_THREADS;PIP_HAS_SOCKET") set(PIP_HAS_DEPS_PIP_BUILD_CONSOLE "PIP_HAS_THREADS") set(PIP_HAS_DEPS_PIP_BUILD_CLIENT_SERVER "PIP_HAS_THREADS;PIP_HAS_SOCKET;PIP_BUILD_IO_UTILS") diff --git a/libs/main/io_devices/piserial.cpp b/libs/main/io_devices/piserial.cpp index 2883a8f5..2487db43 100644 --- a/libs/main/io_devices/piserial.cpp +++ b/libs/main/io_devices/piserial.cpp @@ -94,14 +94,52 @@ # define B4000000 4000000 # else # include -# include -# include +# if defined(PIP_HAS_TTY) +# include +# include +# define PISERIAL_NOCTTY O_NOCTTY +# else +# define PISERIAL_NOCTTY 0 +# endif # ifndef B50 # define B50 0000001 # endif # ifndef B75 # define B75 0000002 # endif +# ifndef B110 +# define B110 0000013 +# endif +# ifndef B300 +# define B300 0000015 +# endif +# ifndef B600 +# define B600 0000016 +# endif +# ifndef B1200 +# define B1200 0000017 +# endif +# ifndef B2400 +# define B2400 0000020 +# endif +# ifndef B4800 +# define B4800 0000021 +# endif +# ifndef B9600 +# define B9600 0000022 +# endif +# ifndef B19200 +# define B19200 0000023 +# endif +# ifndef B38400 +# define B38400 0000024 +# endif +# ifndef B57600 +# define B57600 0000025 +# endif +# ifndef B115200 +# define B115200 0000026 +# endif # ifndef B230400 # define B230400 0010003 # endif @@ -190,7 +228,9 @@ PRIVATE_DEFINITION_START(PISerial) DWORD readed = 0, mask = 0; OVERLAPPED overlap, overlap_write; # else +# if defined(PIP_HAS_TTY) termios desc, sdesc; +# endif uint readed = 0; # endif PRIVATE_DEFINITION_END(PISerial) @@ -370,12 +410,14 @@ bool PISerial::setBreak(bool enabled) { } } # else +# if defined(PIP_HAS_TTY) if (ioctl(fd, enabled ? TIOCSBRK : TIOCCBRK) < 0) { piCoutObj << "setBreak error: " << errorString(); return false; } else { return true; } +# endif # endif return false; } @@ -399,11 +441,13 @@ bool PISerial::setBit(int bit, bool on, const PIString & bname) { return true; } # else +# if defined(PIP_HAS_TTY) if (ioctl(fd, on ? TIOCMBIS : TIOCMBIC, &bit) < 0) { piCoutObj << "setBit" << bname << " error: " << errorString(); return false; } return true; +# endif # endif # endif piCoutObj << "setBit" << bname << " doesn`t implemented, sorry :-("; @@ -419,9 +463,11 @@ bool PISerial::isBit(int bit, const PIString & bname) const { # ifndef PISERIAL_NO_PINS # ifdef WINDOWS # else +# if defined(PIP_HAS_TTY) int ret = 0; if (ioctl(fd, TIOCMGET, &ret) < 0) piCoutObj << "isBit" << bname << " error: " << errorString(); return ret & bit; +# endif # endif # endif piCoutObj << "isBit" << bname << " doesn`t implemented, sorry :-("; @@ -431,7 +477,9 @@ bool PISerial::isBit(int bit, const PIString & bname) const { void PISerial::flush() { # ifndef WINDOWS +# if defined(PIP_HAS_TTY) if (fd != -1) tcflush(fd, TCIOFLUSH); +# endif # endif } @@ -730,13 +778,16 @@ bool PISerial::openDevice() { case PIIODevice::WriteOnly: om = O_WRONLY; break; case PIIODevice::ReadWrite: om = O_RDWR; break; } - fd = ::open(p.data(), O_NOCTTY | om); + om |= PISERIAL_NOCTTY; + fd = ::open(p.data(), om); if (fd == -1) { piCoutObj << "Unable to open \"%1\": %2"_tr("PISerial").arg(p).arg(errorString()); return false; } +# if defined(PIP_HAS_TTY) tcgetattr(fd, &PRIVATE->desc); PRIVATE->sdesc = PRIVATE->desc; +# endif // piCoutObj << "Initialized " << p; # endif applySettings(); @@ -750,9 +801,11 @@ bool PISerial::openDevice() { bool PISerial::closeDevice() { +# ifdef PIP_HAS_THREADS if (isThreadedRead() && !isThreadedReadStopping()) { stopThreadedRead(); } +# endif if (fd != -1) { # ifdef WINDOWS SetCommState(PRIVATE->hCom, &PRIVATE->sdesc); @@ -761,7 +814,9 @@ bool PISerial::closeDevice() { CloseHandle(PRIVATE->hCom); PRIVATE->hCom = 0; # else +# if defined(PIP_HAS_TTY) tcsetattr(fd, TCSANOW, &PRIVATE->sdesc); +# endif ::close(fd); # endif fd = -1; @@ -803,6 +858,7 @@ void PISerial::applySettings() { } # else if (fd == -1) return; +# if defined(PIP_HAS_TTY) tcgetattr(fd, &PRIVATE->desc); PRIVATE->desc.c_oflag = PRIVATE->desc.c_lflag = PRIVATE->desc.c_cflag = 0; PRIVATE->desc.c_iflag = IGNBRK; @@ -835,6 +891,7 @@ void PISerial::applySettings() { piCoutObj << "Can`t set attributes for \"%1\""_tr("PISerial").arg(path()); return; } +# endif # endif } @@ -914,7 +971,9 @@ ssize_t PISerial::readDevice(void * read_to, ssize_t max_size) { if (ret < 0) { int err = errno; if (err == EBADF || err == EFAULT || err == EINVAL || err == EIO) { +# if defined(PIP_HAS_THREADS) stopThreadedRead(); +# endif close(); return 0; } @@ -944,7 +1003,9 @@ ssize_t PISerial::writeDevice(const void * data, ssize_t max_size) { # else ssize_t wrote; wrote = ::write(fd, data, max_size); +# if defined(PIP_HAS_TTY) if (isOptionSet(BlockingWrite)) tcdrain(fd); +# endif # endif return (ssize_t)wrote; // piCoutObj << "Error while sending"; @@ -1270,9 +1331,10 @@ PIVector PISerial::availableDevicesInfo(bool test) { # endif // PIP_HAS_FILESYSTEM # endif # endif +# if defined(PIP_HAS_FILESYSTEM) || defined(WINDOWS) if (test) { for (int i = 0; i < ret.size_s(); ++i) { -# ifdef WINDOWS +# ifdef WINDOWS void * hComm = CreateFileA(ret[i].path.dataAscii(), GENERIC_READ, FILE_SHARE_READ, @@ -1281,38 +1343,39 @@ PIVector PISerial::availableDevicesInfo(bool test) { FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED, 0); if (hComm == INVALID_HANDLE_VALUE) { -# else - int fd = ::open(ret[i].path.dataAscii(), O_NOCTTY | O_RDONLY); +# else + int fd = ::open(ret[i].path.dataAscii(), PISERIAL_NOCTTY | O_RDONLY); if (fd == -1) { -# endif +# endif ret.remove(i); --i; continue; } bool rok = true; -# ifndef WINDOWS +# ifndef WINDOWS int void_ = 0; fcntl(fd, F_SETFL, O_NONBLOCK); if (::read(fd, &void_, 1) == -1) rok = errno != EIO; -# endif +# endif if (!rok) { ret.remove(i); --i; -# ifdef WINDOWS +# ifdef WINDOWS CloseHandle(hComm); -# else +# else ::close(fd); -# endif +# endif continue; } -# ifdef WINDOWS +# ifdef WINDOWS CloseHandle(hComm); -# else +# else ::close(fd); -# endif +# endif } } +# endif // PIP_HAS_FILESYSTEM || WINDOWS return ret; }