From 1593fd6e6c45a042056100c251162a74278693a1 Mon Sep 17 00:00:00 2001 From: Peter Buchegger Date: Tue, 10 Mar 2026 17:49:15 +0100 Subject: [PATCH 1/3] Update CI configuration and code to enforce cppcheck and clang-format standards - Adjusted cppcheck suppressions in CI configuration. - Added mandatory clang-format checks for all modified files. - Included inline cppcheck suppressions in ReceiverApi and TransmitterApi implementations. --- .github/copilot-instructions.md | 55 ++++++++++++++++++- .github/workflows/ci.yml | 3 +- firmware/lib/odh-radio/ReceiverRadioLink.cpp | 1 + firmware/src/receiver/web/ReceiverApi.cpp | 3 +- .../src/transmitter/web/TransmitterApi.cpp | 1 + 5 files changed, 58 insertions(+), 5 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index bce10dc..4417ad2 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -106,8 +106,8 @@ Always run builds from the `firmware/` directory – `platformio.ini` is there. ### Static Analysis (cppcheck) - CI runs cppcheck with `--enable=warning,style,performance,portability` -- Suppressed globally: `missingIncludeSystem`, `missingInclude`, `constParameterPointer`, `constParameterCallback` -- Use `// cppcheck-suppress ` for inline suppressions when needed +- Suppressed globally: `missingIncludeSystem`, `missingInclude` +- Use `// cppcheck-suppress ` for inline suppressions when needed (e.g. `constParameterCallback` for ESP-IDF callbacks) ## Key Architecture Details @@ -172,6 +172,57 @@ After ANY code change, verify at minimum: 1. `pio test -e native` – all 90 tests pass 2. `pio run -e receiver` – compiles 3. `pio run -e transmitter` – compiles +4. **clang-format** – all changed files must be formatted before committing +5. **cppcheck** – no new warnings allowed + +### clang-format (mandatory) + +CI **rejects** any PR with formatting violations. Always run clang-format **19** +on every touched `.cpp` / `.h` / `.c` file before committing: + +```bash +# Format a single file +clang-format -i --style=file firmware/src/receiver/web/ReceiverApi.cpp + +# Format all project sources +find firmware/src firmware/lib \ + \( -name '*.cpp' -o -name '*.h' -o -name '*.c' \) \ + ! -path '*/.*' \ + -exec clang-format -i --style=file {} + +``` + +The `.clang-format` file in the repo root defines the style. Do **not** override +it with `--style=…` other than `--style=file`. + +### cppcheck (mandatory) + +CI **rejects** any PR that introduces new cppcheck warnings. Run locally: + +```bash +cppcheck \ + --error-exitcode=1 \ + --enable=warning,style,performance,portability \ + --suppress=missingIncludeSystem \ + --suppress=missingInclude \ + --suppress=unmatchedSuppression \ + --check-level=exhaustive \ + --inline-suppr \ + -I firmware/src/receiver \ + -I firmware/src/transmitter \ + -I firmware/lib/odh-protocol \ + -I firmware/lib/odh-config \ + -I firmware/lib/odh-radio \ + -I firmware/lib/odh-telemetry \ + -I firmware/lib/odh-web \ + firmware/src \ + firmware/lib +``` + +If a warning is a false positive, suppress it inline: +```cpp +// cppcheck-suppress constParameterCallback +void onData(uint8_t *mac, uint8_t *data, int len) { … } +``` ## Common Pitfalls diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f8b7b35..4305b7e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -196,9 +196,8 @@ jobs: --enable=warning,style,performance,portability \ --suppress=missingIncludeSystem \ --suppress=missingInclude \ - --suppress=constParameterPointer \ - --suppress=constParameterCallback \ --suppress=unmatchedSuppression \ + --check-level=exhaustive \ --inline-suppr \ -I firmware/src/receiver \ -I firmware/src/transmitter \ diff --git a/firmware/lib/odh-radio/ReceiverRadioLink.cpp b/firmware/lib/odh-radio/ReceiverRadioLink.cpp index 32e24e5..4b76dc4 100644 --- a/firmware/lib/odh-radio/ReceiverRadioLink.cpp +++ b/firmware/lib/odh-radio/ReceiverRadioLink.cpp @@ -16,6 +16,7 @@ namespace odh { static volatile int8_t sPromiscRssi = 0; #ifndef NATIVE_SIM +// cppcheck-suppress constParameterCallback static void promiscRxCb(void *buf, wifi_promiscuous_pkt_type_t /*type*/) { const auto *pkt = static_cast(buf); sPromiscRssi = pkt->rx_ctrl.rssi; diff --git a/firmware/src/receiver/web/ReceiverApi.cpp b/firmware/src/receiver/web/ReceiverApi.cpp index 9e6186a..4a0304e 100644 --- a/firmware/src/receiver/web/ReceiverApi.cpp +++ b/firmware/src/receiver/web/ReceiverApi.cpp @@ -25,9 +25,10 @@ void ReceiverApi::begin() { // POST /api/config (JSON body) _server.onBody( "/api/config", HTTP_POST, - [](AsyncWebServerRequest *req) { + [](AsyncWebServerRequest *) { // Handler called after body is received – see body handler below. }, + // cppcheck-suppress constParameterPointer [this](AsyncWebServerRequest *req, uint8_t *data, size_t len, size_t index, size_t total) { handlePostConfig(req, data, len, index, total); }); // POST /api/config/reset diff --git a/firmware/src/transmitter/web/TransmitterApi.cpp b/firmware/src/transmitter/web/TransmitterApi.cpp index 93d5c68..8df8819 100644 --- a/firmware/src/transmitter/web/TransmitterApi.cpp +++ b/firmware/src/transmitter/web/TransmitterApi.cpp @@ -24,6 +24,7 @@ void TransmitterApi::begin() { _server.on("/api/config", HTTP_GET, [this](AsyncWebServerRequest *r) { handleGetConfig(r); }); + // cppcheck-suppress constParameterPointer _server.onBody("/api/config", HTTP_POST, [](AsyncWebServerRequest *) {}, [this](AsyncWebServerRequest *r, uint8_t *d, size_t l, size_t, size_t) { handlePostConfig(r, d, l); }); _server.on("/api/config/reset", HTTP_POST, [this](AsyncWebServerRequest *r) { handlePostReset(r); }); From e2084e729b7b38d300bb33ffce0685255848fbe7 Mon Sep 17 00:00:00 2001 From: Peter Buchegger Date: Tue, 10 Mar 2026 18:24:21 +0100 Subject: [PATCH 2/3] Verbessere Typen und nutze moderne C++-Funktionen in PotModule und TransmitterRadioLink --- .../lib/odh-radio/TransmitterRadioLink.cpp | 5 ++-- firmware/platformio.ini | 23 ++++++++++++++++++- .../src/transmitter/modules/PotModule.cpp | 2 +- firmware/src/transmitter/modules/PotModule.h | 4 ++-- 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/firmware/lib/odh-radio/TransmitterRadioLink.cpp b/firmware/lib/odh-radio/TransmitterRadioLink.cpp index 35bc1af..9c4ddff 100644 --- a/firmware/lib/odh-radio/TransmitterRadioLink.cpp +++ b/firmware/lib/odh-radio/TransmitterRadioLink.cpp @@ -3,6 +3,7 @@ #include #include +#include #include namespace odh { @@ -45,7 +46,7 @@ void TransmitterRadioLink::startScanning() { _bound = false; _scanning = true; _discoveredCount = 0; - std::memset(_discovered, 0, sizeof(_discovered)); + std::fill(std::begin(_discovered), std::end(_discovered), DiscoveredVehicle{}); } const DiscoveredVehicle *TransmitterRadioLink::discoveredVehicle(uint8_t index) const { @@ -63,7 +64,7 @@ void TransmitterRadioLink::pruneStaleVehicles(uint32_t timeoutMs) { _discovered[j] = _discovered[j + 1]; } _discoveredCount--; - std::memset(&_discovered[_discoveredCount], 0, sizeof(DiscoveredVehicle)); + _discovered[_discoveredCount] = DiscoveredVehicle{}; } else { i++; } diff --git a/firmware/platformio.ini b/firmware/platformio.ini index 14d49f2..c08f8c0 100644 --- a/firmware/platformio.ini +++ b/firmware/platformio.ini @@ -25,6 +25,15 @@ upload_speed = 921600 build_flags = -std=gnu++2a + -Wall + -Wextra + -Werror + ; ESP-IDF framework headers (gpio_ll.h, soc_memory_types.h) have unused params + ; in inline functions – cannot fix, must relax for ESP32 builds. + -Wno-error=unused-parameter + ; Third-party libraries (TFT_eSPI, etc.) trigger these. + -Wno-error=missing-field-initializers + -Wno-error=unused-variable -DCORE_DEBUG_LEVEL=3 build_unflags = -std=gnu++11 @@ -70,7 +79,7 @@ board_build.filesystem = littlefs ; ── Native unit tests ──────────────────────────────────────────────────── [env:native] platform = native -build_flags = -std=gnu++20 -DNATIVE_TEST +build_flags = -std=gnu++20 -Wall -Wextra -Werror -DNATIVE_TEST build_src_filter = -<*> test_build_src = no @@ -83,6 +92,9 @@ build_src_filter = + build_flags = -std=gnu++20 + -Wall + -Wextra + -Werror -DNATIVE_SIM -DODH_RECEIVER -DSIM_RX @@ -100,6 +112,11 @@ build_src_filter = + build_flags = -std=gnu++20 + -Wall + -Wextra + -Werror + ; LVGL has unused parameters in its C code. + -Wno-error=unused-parameter -DNATIVE_SIM -DODH_TRANSMITTER -DSIM_TX @@ -109,6 +126,10 @@ build_flags = !pkg-config --libs sdl2 -lpthread +; Re-enable strict unused-parameter for our own source code. +build_src_flags = + -Werror=unused-parameter + lib_deps = lvgl/lvgl @ ^8.3.11 bblanchon/ArduinoJson @ ^7.0.0 diff --git a/firmware/src/transmitter/modules/PotModule.cpp b/firmware/src/transmitter/modules/PotModule.cpp index 89fa1c3..55eab4f 100644 --- a/firmware/src/transmitter/modules/PotModule.cpp +++ b/firmware/src/transmitter/modules/PotModule.cpp @@ -37,7 +37,7 @@ int16_t PotModule::rawValue(uint8_t index) const { return _raw[index]; } -uint16_t PotModule::mapToChannel(int16_t raw) { +uint16_t PotModule::mapToChannel(int32_t raw) { if (raw < 0) raw = 0; if (raw > kPotAdcMax) diff --git a/firmware/src/transmitter/modules/PotModule.h b/firmware/src/transmitter/modules/PotModule.h index 303e448..e6e35e8 100644 --- a/firmware/src/transmitter/modules/PotModule.h +++ b/firmware/src/transmitter/modules/PotModule.h @@ -15,7 +15,7 @@ namespace odh { inline constexpr uint8_t kPotI2cAddr = 0x48; inline constexpr uint8_t kPotCount = 4; -inline constexpr int16_t kPotAdcMax = 32767; +inline constexpr int32_t kPotAdcMax = 32767; class PotModule : public IModule { public: @@ -35,7 +35,7 @@ class PotModule : public IModule { int16_t _raw[kPotCount] = {}; uint16_t _values[kPotCount] = {}; - static uint16_t mapToChannel(int16_t raw); + static uint16_t mapToChannel(int32_t raw); }; } // namespace odh From e090712a4f4ee831da09977fdd7ba3ad961a703a Mon Sep 17 00:00:00 2001 From: Peter Buchegger Date: Tue, 10 Mar 2026 18:26:43 +0100 Subject: [PATCH 3/3] =?UTF-8?q?Verbessere=20die=20PlattformIO-Konfiguratio?= =?UTF-8?q?n=20durch=20die=20Einf=C3=BChrung=20gemeinsamer=20Einstellungen?= =?UTF-8?q?=20f=C3=BCr=20native=20und=20Simulationsumgebungen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- firmware/platformio.ini | 61 +++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/firmware/platformio.ini b/firmware/platformio.ini index c08f8c0..879a022 100644 --- a/firmware/platformio.ini +++ b/firmware/platformio.ini @@ -7,6 +7,11 @@ ; native – Host unit tests (no hardware) ; sim_rx – Linux receiver simulation (terminal) ; sim_tx – Linux transmitter simulation (SDL2 display) +; +; Inheritance: +; esp32_base → receiver, transmitter +; native_base → native, sim_base +; sim_base → sim_rx, sim_tx ; ═══════════════════════════════════════════════════════════════════════ [platformio] @@ -38,6 +43,29 @@ build_flags = build_unflags = -std=gnu++11 +; ── Shared native settings (tests + simulations) ──────────────────────── +[native_base] +platform = native + +build_flags = + -std=gnu++20 + -Wall + -Wextra + -Werror + +; ── Shared simulation settings ─────────────────────────────────────────── +[sim_base] +extends = native_base +extra_scripts = pre:sim/sim_build.py + +build_flags = + ${native_base.build_flags} + -DNATIVE_SIM + -lpthread + +lib_deps = + bblanchon/ArduinoJson @ ^7.0.0 + ; ── Receiver ───────────────────────────────────────────────────────────── [env:receiver] extends = esp32_base @@ -78,58 +106,43 @@ board_build.filesystem = littlefs ; ── Native unit tests ──────────────────────────────────────────────────── [env:native] -platform = native -build_flags = -std=gnu++20 -Wall -Wextra -Werror -DNATIVE_TEST +extends = native_base +build_flags = + ${native_base.build_flags} + -DNATIVE_TEST build_src_filter = -<*> test_build_src = no ; ── Simulation: Receiver (terminal, no SDL2) ───────────────────────────── [env:sim_rx] -platform = native -extra_scripts = pre:sim/sim_build.py - +extends = sim_base build_src_filter = + build_flags = - -std=gnu++20 - -Wall - -Wextra - -Werror - -DNATIVE_SIM + ${sim_base.build_flags} -DODH_RECEIVER -DSIM_RX - -lpthread - -lib_deps = - bblanchon/ArduinoJson @ ^7.0.0 ; ── Simulation: Transmitter (SDL2 display) ─────────────────────────────── [env:sim_tx] -platform = native -extra_scripts = pre:sim/sim_build.py - +extends = sim_base build_src_filter = + build_flags = - -std=gnu++20 - -Wall - -Wextra - -Werror + ${sim_base.build_flags} ; LVGL has unused parameters in its C code. -Wno-error=unused-parameter - -DNATIVE_SIM -DODH_TRANSMITTER -DSIM_TX -I include -DLV_CONF_INCLUDE_SIMPLE !pkg-config --cflags sdl2 !pkg-config --libs sdl2 - -lpthread ; Re-enable strict unused-parameter for our own source code. build_src_flags = -Werror=unused-parameter lib_deps = + ${sim_base.lib_deps} lvgl/lvgl @ ^8.3.11 - bblanchon/ArduinoJson @ ^7.0.0