Cppcheck and compiler improvments#1
Conversation
… 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.
…ansmitterRadioLink
…amer Einstellungen für native und Simulationsumgebungen
There was a problem hiding this comment.
Pull request overview
Diese PR stärkt die Build-/CI-Qualitätssicherung für die OpenDriveHub-Firmware (ESP32, Native-Tests und Simulation), indem Formatierung und Static Analysis strikter durchgesetzt und die PlatformIO-Konfiguration wartbarer strukturiert wird.
Changes:
- CI erweitert: clang-format (v19) als verpflichtender Style-Check und cppcheck mit strengerer/exhaustiver Konfiguration.
- PlatformIO-Refactor: gemeinsame Basis-Environments (
esp32_base,native_base,sim_base) und strengere Compiler-Warnings/Errors. - Kleinere Code-Cleanups: u.a.
memset→std::fill/Value-Init und inline cppcheck-Suppressions für Library-Callback-Signaturen.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
firmware/platformio.ini |
Refactor mit Base-Envs; neue -Wall/-Wextra/-Werror und Sim-/Native-Vererbung. |
.github/workflows/ci.yml |
cppcheck-Invocation verschärft (u.a. exhaustive Check-Level) und globale Suppressions reduziert. |
.github/copilot-instructions.md |
Dokumentiert verpflichtende lokale clang-format/cppcheck Nutzung und Inline-Suppressions. |
firmware/lib/odh-radio/TransmitterRadioLink.cpp |
Typ-sicheres Zeroing via std::fill/Value-Init statt memset. |
firmware/lib/odh-radio/ReceiverRadioLink.cpp |
Inline cppcheck-Suppression für ESP-IDF Callback-Signatur. |
firmware/src/receiver/web/ReceiverApi.cpp |
Inline cppcheck-Suppression und Cleanup für Body-Handler-Lambdas. |
firmware/src/transmitter/web/TransmitterApi.cpp |
Inline cppcheck-Suppression für onBody-Callback-Signatur. |
firmware/src/transmitter/modules/PotModule.h |
Typänderungen rund um ADC-Max/Mapping-Signatur. |
firmware/src/transmitter/modules/PotModule.cpp |
Mapping-Funktion-Signatur auf int32_t angepasst. |
| build_flags = | ||
| ${native_base.build_flags} | ||
| -DNATIVE_SIM | ||
| -lpthread |
There was a problem hiding this comment.
In sim_base, -lpthread is included in build_flags while native_base enables -Werror. PlatformIO applies build_flags during compilation as well as linking, so passing -lpthread on compile-only steps commonly triggers an “unused command line argument” warning that becomes an error with -Werror. Prefer -pthread (works for both compile+link) or move this into a dedicated link flag setting so native/sim builds don’t fail.
| -lpthread | |
| -pthread |
| Adafruit_ADS1115 _adc; | ||
| int16_t _raw[kPotCount] = {}; | ||
| uint16_t _values[kPotCount] = {}; | ||
|
|
||
| static uint16_t mapToChannel(int16_t raw); | ||
| static uint16_t mapToChannel(int32_t raw); |
There was a problem hiding this comment.
PR description says the potentiometer ADC values were changed from int16_t to int32_t, but the module still stores raw readings as int16_t (_raw and rawValue()), and only kPotAdcMax / mapToChannel() were widened. Either update the description to match what changed, or (if the intent really is to widen ADC storage) adjust the member/storage types and related APIs consistently.
This pull request introduces several improvements to the firmware build system, code quality enforcement, and codebase consistency. The most significant changes are the addition of stricter code formatting and static analysis requirements, refactoring of the build configuration for better maintainability, and minor code cleanups to address static analysis warnings.
Build system and code quality enforcement:
clang-formatandcppcheckchecks to the development workflow, with detailed instructions for local usage. CI now rejects any PRs with formatting violations or new static analysis warnings. (.github/copilot-instructions.md.github/copilot-instructions.mdR175-R225)cppcheckanalysis and removed global suppressions forconstParameterPointerandconstParameterCallback, which must now be suppressed inline when necessary. (.github/workflows/ci.yml[1].github/copilot-instructions.md[2]PlatformIO build configuration refactor:
platformio.inito use shared base environments (esp32_base,native_base,sim_base) for cleaner inheritance and reduced duplication. Added stricter compiler warnings (-Wall,-Wextra,-Werror) and environment-specific suppressions. (firmware/platformio.ini[1] [2] [3]Codebase consistency and static analysis fixes:
std::memsetwithstd::fillor value-initialization for zeroing arrays of non-trivial types, improving type safety and clarity. (firmware/lib/odh-radio/TransmitterRadioLink.cpp[1] [2]cppchecksuppressions (constParameterCallback,constParameterPointer) at specific locations where required by ESP-IDF or library callbacks. (firmware/lib/odh-radio/ReceiverRadioLink.cpp[1]firmware/src/receiver/web/ReceiverApi.cpp[2]firmware/src/transmitter/web/TransmitterApi.cpp[3]Minor code improvements:
int16_ttoint32_tfor improved range and consistency, updating method signatures accordingly. (firmware/src/transmitter/modules/PotModule.h[1] [2]firmware/src/transmitter/modules/PotModule.cpp[3]<algorithm>to support use ofstd::fill. (firmware/lib/odh-radio/TransmitterRadioLink.cppfirmware/lib/odh-radio/TransmitterRadioLink.cppR6)