* fix(test): make the native-windows test suite build and run
pio test -e native-windows failed every suite at the build stage. Five
independent causes, all Windows-only:
- TestUtil.cpp called lstat(), which MinGW-w64 does not provide. The
state-checkpoint walk added in #11322 is fenced with ARCH_PORTDUINO,
which native-windows also satisfies, so all 53 suites failed to
compile. Route it through a stat() shim on _WIN32.
- test_default, test_http_content_handler, test_meshpacket_serializer
and test_serial define no setUp/tearDown and relied on the weak
defaults PlatformIO emits in unity_config.c. GCC lowers a weak
definition on PE-COFF to a weak external, leaving the symbol
undefined, so it does not satisfy unity.c's reference and the link
fails. Define them explicitly, as the other 49 suites already do.
- test_mqtt included <arpa/inet.h>, absent on MinGW, for htonl(). Use
winsock2.h there.
- test_gps_update_scheduling uses TEST_ASSERT_DOUBLE_WITHIN. Unity
omits double support unless UNITY_INCLUDE_DOUBLE is defined, so the
assertion compiled to an unconditional failure. Define it for the
env.
- test_getfiles_rejects_overlong_path is excluded on _WIN32. Overrunning
the 228-byte file_name needs at least 229 bytes below the portduino
root, and that root is already ~34 bytes, so every qualifying path
passes the 260-byte MAX_PATH: the nested mkdir() fails, the file is
never created, and getFiles() has nothing to drop. No component
layout satisfies both limits.
Each of the seven suites that failed on Windows was verified
individually after the change. test_fscommon_getfiles still fails in a
full run, for a cause outside this change: rmDir() does not remove
directories on Windows, so empty dirs left by an earlier run survive
setUp() and make getFiles() report a depth truncation. That is a
pre-existing FSCommon bug, reported separately.
No Linux or macOS behaviour changes: every guard is _WIN32-only except
UNITY_INCLUDE_DOUBLE, which is scoped to env:native-windows.
* fix(test): define UNITY_INCLUDE_DOUBLE for every native env
The flag was scoped to env:native-windows, but the gap is not
Windows-specific. Verified on Debian with gcc against the Linux env's
own Unity 2.6.1 and PlatformIO's generated native unity_config:
UNITY_INCLUDE_DOUBLE : NOT defined
UNITY_EXCLUDE_DOUBLE : defined
test_double_within:FAIL: Unity Double Precision Disabled
UNITY_INCLUDE_DOUBLE appears nowhere in the repo, the ini files, the
workflow, or PlatformIO's unity runner, which adds only
UNITY_INCLUDE_CONFIG_H. So TEST_ASSERT_DOUBLE_* is an always-failing
stub on Linux and macOS too, not only on Windows.
Moved to portduino_base.build_flags_common, which every native env
resolves: native, native-tft, native-fb, native-tft-debug, coverage,
coverage-event-policy, native-macos, native-windows and native-wasm.
This does change Linux and macOS: TEST_ASSERT_DOUBLE_* becomes a real
comparison instead of a stub. test_gps_update_scheduling is the only
suite using those macros and its arithmetic is integer-based and
bit-identical across platforms, so it should pass wherever it runs.
Note it currently reports PASSED on CI in 0.03s while emitting no Unity
output at all, so those assertions appear never to execute there; that
is tracked separately and is not addressed here.
* fix(serial): validate serial module config on every platform
AdminModule guarded the serial config validation by architecture but not the
assignment beneath it:
#if ARCH_ESP32 || ARCH_NRF52 || ARCH_RP2040
if (!SerialModule::isValidConfig(...)) return false;
disableBluetooth();
#endif
moduleConfig.serial = c.payload_variant.serial;
So on every other platform an admin "set module config: serial" stored a config
the firmware rejects on ESP32. override_console_serial_port combined with
DEFAULT, SIMPLE, TEXTMSG or PROTO is accepted and persisted today.
Two families are affected, for different reasons:
- portduino/meshtasticd, where the validation did not exist at all:
isValidConfig was a static member of SerialModule, and that class is inside
the same architecture guard, so `nm` finds no such symbol in the native
object.
- STM32WL (rak3172, wio-e5, CDEBYTE_E77-MBL, russell), where it existed and
was never called: the class guard includes ARCH_STM32WL and the AdminModule
call site did not.
Validation is pure config logic with no serial hardware behind it, so it moves
out of the class and out of the guard as a free serialConfigIsValid(). Its only
external references - clientNotificationPool, service, getValidTime - are
already unguarded elsewhere, so it links on every target. AdminModule's include
of SerialModule.h is unguarded for the same reason; the class itself stays
guarded inside the header. Only disableBluetooth() remains architecture-specific.
This changes what meshtasticd and the STM32WL targets accept: a host relying on
the unvalidated path (override_console_serial_port with a mode other than NMEA,
CalTopo or MS_CONFIG) is now rejected, as it already is on ESP32.
test/test_serial has asserted nothing since it was added in 28aeb0f09e
(2025-07-26): its body is behind the same guard, so on portduino it logged a
warning and ran zero assertions while counting as one of the canonical suites.
Enabling it showed the code did not even compile - its designated initializers
list .override_console_serial_port before .mode, which is not declaration order,
and C++ requires that. PlatformIO only builds test/ for the native env, so no
build had ever compiled these lines. Reordered; all nine now run and pass.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* I'd say gimme 5 bees for a dollar. That's what we called a nickel, because they had bees on em.
* style: wrap over-long warning string to the 120-col limit
* Cover MS_CONFIG override and correct the validator comment
serialConfigIsValid() accepts MS_CONFIG alongside NMEA and CALTOPO when
override_console_serial_port is set, but only the first two had a valid-case
test. Add the missing one.
The declaration comment described the function as pure config logic; it also
logs and, in non-test builds, sends a client notification on rejection.
---------
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
I thought git would be smart enough to understand all the whitespace changes but even with all the flags I know to make it ignore theses it still blows up if there are identical changes on both sides.
I have a solution but it require creating a new commit at the merge base for each conflicting PR and merging it into develop.
I don't think blowing up all PRs is worth for now, maybe if we can coordinate this for V3 let's say.
This reverts commit 0d11331d18.
Thomas Göttgens