From 21e3a583bd88241bd1d0baea7c34a17e8cb8039f Mon Sep 17 00:00:00 2001 From: Tom <116762865+NomDeTom@users.noreply.github.com> Date: Thu, 30 Jul 2026 12:06:24 +0100 Subject: [PATCH] Yaml check for Meshtasticd (#11224) * feat(portduino): add `meshtasticd --check` config validator Users hand-writing files in /etc/meshtasticd/config.d/ get no feedback when a key is misplaced, misspelled or duplicated: meshtasticd silently ignores what it does not read, so a broken config looks identical to a working one. Add a --check mode that loads the configuration exactly as startup does, then reports what it found and exits: - Duplicate keys, via the yaml-cpp Parser/EventHandler stream. The Node API cannot see them because the map is already collapsed by the time it exists, and yaml-cpp keeps the FIRST occurrence, so a later override is discarded. - Unknown or misnested keys, against a schema mirroring what loadConfig() reads, with a hint naming the section a stray key actually belongs to. - rfswitch_table validation: unrecognised pins, mode rows whose length does not match the pin list, values that are not HIGH/LOW, and unknown modes. - Cross-file overlap: every .yaml in the config directory merges into one portduino_config, so the file loaded LAST wins, the opposite of the within-file rule. Those files are read in filesystem order, not alphabetical. - A warning when more than one file defines a Lora section: spidev, spiSpeed, gpiochip, DIO2_AS_RF_SWITCH, DIO3_TCXO_VOLTAGE and USB_PID/VID/Serialnum are assigned unconditionally with a default every time one is seen, so any of them not repeated in the last file loaded is silently reset. - The resolved gpiochip/line for each pin, since a line that exists on the wrong chip is claimed successfully and then silently does nothing. Exits non-zero when errors were found so it can also gate CI over bin/config.d/**, keeping one implementation rather than a second schema. Co-Authored-By: Claude Opus 5 (1M context) * fix(portduino): flag pins that resolve to -1 in --check A pin key whose value will not convert to a number falls back to RADIOLIB_NC (-1) while still being marked enabled, and initGPIOPin() then trips an assertion inside LinuxGPIOPin rather than failing cleanly. YAML indentation makes this easy to hit by accident: a stray line under "CS: 8" folds into the value as a multi-line scalar, so the file parses, the daemon crashes with a stack trace from a library file, and --check reported "Configuration looks good" while printing "pin -1" two lines above. Report it as an error naming the likely cause instead. Also correct a comment claiming unparseable config.d files are skipped silently. They are not: loadConfig() prints "*** Exception ..." with the line and column. It is the discarded return value, not the diagnostic, that makes the file's absence from the merged config easy to miss. Co-Authored-By: Claude Opus 5 (1M context) * test(portduino): cover `meshtasticd --check` with fixtures and a fuzz suite Adds the tests the config validator was missing, and the checks and fixes that writing them turned up. The theme throughout is configuration that the YAML parser accepts but that does not mean what it looks like it means. Tests ----- bin/test-config-check.sh - 57 assertions driving a built meshtasticd against test/fixtures/portduino-config (50 fixtures plus two config.d trees). A shell test rather than a Unity suite because both behaviours under test are properties of the process: --check is judged by its exit status and printed report, and the "a normal run rejects a bad config" path ends in exit() inside portduinoSetup(), neither of which is reachable from a suite that links one translation unit. Every fixture carries a comment header naming its planted fault and the expected finding, so it can be read on its own. Coverage: * a clean config for each of the ten radio module families (RF95, sx1262, sx1268, LLCC68, sx1280, lr1110, lr1120, lr1121, sim, auto), asserted both findings-free and resolving to that module, so a silent fallback to sim cannot pass * LR11xx rfswitch tables: unrecognised pins, rows longer and shorter than the pin count, levels that are not exactly HIGH, a missing pins list, more than five pins, a scalar table, unknown MODE_ keys, a MODE_ row stranded one level out, and a legal partial table * the PA gain table in both accepted shapes, entries outside the uint16 range it is stored in, and more than the 22 points that are kept * values of the wrong type, split by consequence: the two settings read with no fallback stop meshtasticd starting, everything else is silently replaced by its default * out-of-range and unit mistakes: TCXO voltage written in millivolts, ports outside their usable range, an over-long StatusMessage * MAC sources: both keys set at once, a malformed address, an interface that does not exist * structural faults: duplicate keys, non-mapping and unknown sections, a key left at the top level, a sequence at the document root, an empty file, unreadable pins, unparseable YAML * cross-file behaviour over a config.d directory, including the switch tables that do not override each other * five configs run WITHOUT --check, each of which must still be refused, so check mode cannot quietly make the normal path permissive test/test_fuzz_config - adversarial fuzzing of the checker itself, the "the tool meant to diagnose your config crashes on it" failure mode. Scope is deliberately narrow: yaml-cpp does the parsing and is fuzzed upstream, so what is exercised here is our code above the parse, above all the duplicate-key detector, which is the one hand-rolled piece and walks the raw parser event stream with its own stack. Groups: the checked-in fixtures as a seed corpus, 3000 byte mutations of them (flips, truncation, insertion, splicing, deletion), and structural torture (nesting to 4096 in flow and block style, duplicate keys at depth, anchors, aliases and merge keys, 64KB keys, 256KB scalars, multi-document files). A fourth group of random bytes is present but disabled behind FUZZ_CONFIG_RANDOM_BYTES: it was half the runtime for the least return, since uniform noise is rejected on the first token. The contract is crash-freedom and termination under AddressSanitizer, not any particular finding. CI runs the shell test in the existing native simulator job; the fuzz suite is picked up by the existing ^test_fuzz_ area rule. native-suite-count 40 -> 41. The fixtures are exempt from trunk in .trunk/trunk.yaml, since prettier rejects the duplicate keys and bad indentation that are the point of them. Checker fixes found while writing the tests ------------------------------------------- --check reported a clean exit 0 on configs meshtasticd then refuses to boot, the worst failure a diagnostic tool can have. Four hard exits inside loadConfig() killed the report before it printed: an unparseable file, an unknown Lora.Module, MACAddress and MACAddressSource both set, and HUB75 on a build without it. All are now reported as findings, and all are still refused on a normal run. New validation: Lora.Module against the accepted spellings, which are matched exactly and inconsistently cased, with a suggestion when only case differs; a per-key value type table covering ~85 keys, tested by asking yaml-cpp to perform the same conversion loadConfig() will so it cannot drift; the PA gain table; DIO3_TCXO_VOLTAGE, which is in volts and multiplied by 1000, so the millivolt value everything else uses silently asks for 1800V; APIPort and Webserver.Port ranges; MaxNodes; StatusMessage truncation; MAC address and source; and an unreadable ConfigDirectory. Also fixes a crash: a ConfigDirectory that cannot be read threw an uncaught filesystem_error from directory_iterator and aborted meshtasticd with SIGABRT, taking --check down with it. It now fails cleanly. Two smaller ones: cppcheck's uselessCallsSubstr on the ancestor walk, which was failing every check job; and the duplicate-key detector's stack pop, which was unguarded and relied on yaml-cpp emitting balanced events. Switch tables are the one place "the file loaded last wins" is false. The loader only ever writes HIGH and never writes LOW back, so a HIGH from an earlier file survives a later file that clears it and the radio drives the OR of every table loaded. Confirmed with --output-yaml. Reported as an error for now; the loader itself is left alone, as that changes RF behaviour. * fix(portduino): report CH341 pins as adapter indexes, not gpiochip lines --check printed "Resolved GPIO lines (what meshtasticd will try to claim)" for every config, listing a gpiochip and line for each Lora pin and advising they be confirmed against gpiodetect and gpioinfo. For spidev: ch341 every part of that is false. portduinoSetup() skips initGPIOPin() for every Lora pin when spidev is ch341 and hands the raw numbers to Ch341Hal, so nothing is claimed from a gpiochip -- and on Windows and macOS, where a USB adapter is the only way to attach a radio, there is no gpiochip, gpiodetect or gpioinfo to check against in the first place. The checker had no ch341 coverage at all: not one fixture used it, so the whole USB-SPI path went unexercised. The summary now splits on the transport. A ch341 device gets its pins listed as adapter indexes with the gpiod advice dropped, and a gpiochip or line mapping written alongside it is reported: those are read, stored, and never used. Also: "RF switch table: not set" read as a gap on an SX126x, where there is nothing to set. setRfSwitchTable() is only ever called for an LR11xx, so absence is now "not needed for this module" everywhere else, and "not resolved yet" for auto, which has no module to judge against. Fixtures: usb-ch341.yaml (clean, the meshstick shape) and ch341-gpiochip.yaml. CI fix ------ test-native was RED on "config.d overrides are reported", which wanted 2 warnings and got 1. The fixture's two config.d files name different modules, so which one wins -- and whether the LR11xx-without-a-switch-table warning fires -- depends on the order the filesystem returns them in. That is the very thing the fixture exists to demonstrate, so the count is no longer asserted; the report's own order caveat is asserted instead. Review fixes ------------ The unreadable-ConfigDirectory diagnostic was the one new print in PortduinoGlue.cpp not gated behind !configCheck, so it landed ahead of the report header and broke the clean output the rest of the change is careful to keep. Docs: rfswitch-valid.yaml carries seven modes, not eight, and empty-file.yaml is comments-only rather than zero bytes. * style(portduino): trim --check comment blocks and reconcile suite count Condense the multi-paragraph comment blocks in the --check validator to the one-to-two-line convention, and bump test/native-suite-count to 42 for the test_fuzz_config suite added here. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- .github/workflows/test_native.yml | 8 + .trunk/trunk.yaml | 8 + bin/test-config-check.sh | 391 ++++++ src/platform/portduino/ConfigCheck.cpp | 1094 +++++++++++++++++ src/platform/portduino/ConfigCheck.h | 12 + src/platform/portduino/PortduinoGlue.cpp | 93 +- test/README.md | 14 + test/fixtures/portduino-config/README.md | 158 +++ .../portduino-config/ch341-gpiochip.yaml | 16 + .../configd-conflict/config.d/lora-a.yaml | 6 + .../configd-conflict/config.d/lora-b.yaml | 8 + .../configd-conflict/config.yaml | 12 + .../portduino-config/configdir-missing.yaml | 8 + .../portduino-config/duplicate-key.yaml | 8 + .../fixtures/portduino-config/empty-file.yaml | 2 + .../portduino-config/empty-sections.yaml | 5 + .../portduino-config/hub75-unknown-key.yaml | 11 + .../portduino-config/mac-conflict.yaml | 9 + .../portduino-config/mac-malformed.yaml | 9 + .../portduino-config/mac-source-missing.yaml | 7 + .../portduino-config/malformed-indent.yaml | 8 + .../portduino-config/module-auto.yaml | 3 + .../portduino-config/module-llcc68.yaml | 9 + .../portduino-config/module-lr1110.yaml | 18 + .../portduino-config/module-lr1120.yaml | 18 + .../portduino-config/module-lr1121.yaml | 18 + .../module-mismatch-lr11xx.yaml | 7 + .../module-mismatch-sx126x.yaml | 13 + .../portduino-config/module-rf95.yaml | 10 + .../fixtures/portduino-config/module-sim.yaml | 3 + .../portduino-config/module-sx1262.yaml | 12 + .../portduino-config/module-sx1268.yaml | 9 + .../portduino-config/module-sx1280.yaml | 10 + .../portduino-config/module-unknown.yaml | 4 + .../portduino-config/module-wrong-case.yaml | 4 + .../portduino-config/nonmap-section.yaml | 4 + .../portduino-config/pin-unknown-subkey.yaml | 8 + .../portduino-config/pin-unreadable.yaml | 7 + .../portduino-config/port-out-of-range.yaml | 9 + .../portduino-config/rfswitch-bad-level.yaml | 9 + .../portduino-config/rfswitch-bad-pin.yaml | 6 + .../portduino-config/rfswitch-no-pins.yaml | 6 + .../portduino-config/rfswitch-not-a-map.yaml | 4 + .../portduino-config/rfswitch-partial.yaml | 12 + .../portduino-config/rfswitch-row-length.yaml | 8 + .../rfswitch-sticky/config.d/a-first.yaml | 6 + .../rfswitch-sticky/config.d/b-second.yaml | 9 + .../rfswitch-sticky/config.yaml | 4 + .../rfswitch-stranded-modes.yaml | 9 + .../rfswitch-too-many-pins.yaml | 7 + .../rfswitch-unknown-mode.yaml | 8 + .../portduino-config/rfswitch-valid.yaml | 17 + .../portduino-config/statusmessage-long.yaml | 7 + .../portduino-config/stranded-key.yaml | 6 + .../portduino-config/tcxo-millivolts.yaml | 9 + .../portduino-config/top-level-list.yaml | 3 + .../portduino-config/txgain-out-of-range.yaml | 5 + .../portduino-config/txgain-scalar.yaml | 8 + .../portduino-config/txgain-too-many.yaml | 5 + .../portduino-config/unknown-key.yaml | 9 + .../portduino-config/unknown-section.yaml | 6 + test/fixtures/portduino-config/usb-ch341.yaml | 15 + test/fixtures/portduino-config/valid.yaml | 8 + .../value-type-fatal-list.yaml | 5 + .../portduino-config/value-type-fatal.yaml | 8 + .../portduino-config/value-type-silent.yaml | 10 + test/native-suite-count | 2 +- test/test_fuzz_config/test_main.cpp | 267 ++++ 68 files changed, 2507 insertions(+), 24 deletions(-) create mode 100755 bin/test-config-check.sh create mode 100644 src/platform/portduino/ConfigCheck.cpp create mode 100644 src/platform/portduino/ConfigCheck.h create mode 100644 test/fixtures/portduino-config/README.md create mode 100644 test/fixtures/portduino-config/ch341-gpiochip.yaml create mode 100644 test/fixtures/portduino-config/configd-conflict/config.d/lora-a.yaml create mode 100644 test/fixtures/portduino-config/configd-conflict/config.d/lora-b.yaml create mode 100644 test/fixtures/portduino-config/configd-conflict/config.yaml create mode 100644 test/fixtures/portduino-config/configdir-missing.yaml create mode 100644 test/fixtures/portduino-config/duplicate-key.yaml create mode 100644 test/fixtures/portduino-config/empty-file.yaml create mode 100644 test/fixtures/portduino-config/empty-sections.yaml create mode 100644 test/fixtures/portduino-config/hub75-unknown-key.yaml create mode 100644 test/fixtures/portduino-config/mac-conflict.yaml create mode 100644 test/fixtures/portduino-config/mac-malformed.yaml create mode 100644 test/fixtures/portduino-config/mac-source-missing.yaml create mode 100644 test/fixtures/portduino-config/malformed-indent.yaml create mode 100644 test/fixtures/portduino-config/module-auto.yaml create mode 100644 test/fixtures/portduino-config/module-llcc68.yaml create mode 100644 test/fixtures/portduino-config/module-lr1110.yaml create mode 100644 test/fixtures/portduino-config/module-lr1120.yaml create mode 100644 test/fixtures/portduino-config/module-lr1121.yaml create mode 100644 test/fixtures/portduino-config/module-mismatch-lr11xx.yaml create mode 100644 test/fixtures/portduino-config/module-mismatch-sx126x.yaml create mode 100644 test/fixtures/portduino-config/module-rf95.yaml create mode 100644 test/fixtures/portduino-config/module-sim.yaml create mode 100644 test/fixtures/portduino-config/module-sx1262.yaml create mode 100644 test/fixtures/portduino-config/module-sx1268.yaml create mode 100644 test/fixtures/portduino-config/module-sx1280.yaml create mode 100644 test/fixtures/portduino-config/module-unknown.yaml create mode 100644 test/fixtures/portduino-config/module-wrong-case.yaml create mode 100644 test/fixtures/portduino-config/nonmap-section.yaml create mode 100644 test/fixtures/portduino-config/pin-unknown-subkey.yaml create mode 100644 test/fixtures/portduino-config/pin-unreadable.yaml create mode 100644 test/fixtures/portduino-config/port-out-of-range.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-bad-level.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-bad-pin.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-no-pins.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-not-a-map.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-partial.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-row-length.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-sticky/config.d/a-first.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-sticky/config.d/b-second.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-sticky/config.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-stranded-modes.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-too-many-pins.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-unknown-mode.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-valid.yaml create mode 100644 test/fixtures/portduino-config/statusmessage-long.yaml create mode 100644 test/fixtures/portduino-config/stranded-key.yaml create mode 100644 test/fixtures/portduino-config/tcxo-millivolts.yaml create mode 100644 test/fixtures/portduino-config/top-level-list.yaml create mode 100644 test/fixtures/portduino-config/txgain-out-of-range.yaml create mode 100644 test/fixtures/portduino-config/txgain-scalar.yaml create mode 100644 test/fixtures/portduino-config/txgain-too-many.yaml create mode 100644 test/fixtures/portduino-config/unknown-key.yaml create mode 100644 test/fixtures/portduino-config/unknown-section.yaml create mode 100644 test/fixtures/portduino-config/usb-ch341.yaml create mode 100644 test/fixtures/portduino-config/valid.yaml create mode 100644 test/fixtures/portduino-config/value-type-fatal-list.yaml create mode 100644 test/fixtures/portduino-config/value-type-fatal.yaml create mode 100644 test/fixtures/portduino-config/value-type-silent.yaml create mode 100644 test/test_fuzz_config/test_main.cpp diff --git a/.github/workflows/test_native.yml b/.github/workflows/test_native.yml index f41a91199..c5186c9f1 100644 --- a/.github/workflows/test_native.yml +++ b/.github/workflows/test_native.yml @@ -81,6 +81,14 @@ jobs: lcov ${{ env.LCOV_CAPTURE_FLAGS }} --initial --output-file coverage_base.info sed -i -e "s#${PWD}#.#" coverage_base.info # Make paths relative. + - name: Config check tests + # Drives the same binary against test/fixtures/portduino-config: asserts that + # `--check` reports each planted fault, and that a normal run still refuses the + # configs it should. Runs before the simulator test because it is seconds long + # and a failure here explains a lot of downstream weirdness. + timeout-minutes: 5 + run: ./bin/test-config-check.sh .pio/build/coverage/meshtasticd + - name: Integration test # Cap the whole step: if the simulator ever fails to exit (e.g. the # exit_simulator admin path regresses again) the job must fail fast, diff --git a/.trunk/trunk.yaml b/.trunk/trunk.yaml index ad8a3eed9..741a51e58 100644 --- a/.trunk/trunk.yaml +++ b/.trunk/trunk.yaml @@ -102,6 +102,14 @@ lint: - linters: [gitleaks] paths: - test/fixtures/nodedb/seed_v25_*.jsonl + # Deliberately broken YAML: these are the inputs to bin/test-config-check.sh, + # which asserts that `meshtasticd --check` reports each planted fault. Prettier + # rejects the duplicate keys and bad indentation that are the whole point of + # them, so the directory is exempt. The README there is normal prose and stays + # linted. + - linters: [ALL] + paths: + - test/fixtures/portduino-config/**/*.yaml # checkov/CKV_SECRET_6 flags the commented-out "large4cats" example, # which is the public default credential for the meshtastic.org MQTT # broker rather than a real secret. Ignore the whole file so the diff --git a/bin/test-config-check.sh b/bin/test-config-check.sh new file mode 100755 index 000000000..b3027fe7b --- /dev/null +++ b/bin/test-config-check.sh @@ -0,0 +1,391 @@ +#!/usr/bin/env bash +# Drive a built meshtasticd against the fixtures in test/fixtures/portduino-config +# and assert what it does with each one. +# +# Why this is a shell test and not a Unity suite: both behaviours under test are +# properties of the process, not of a function. `--check` is judged by its exit +# status and its printed report, and the "normal run rejects a bad config" path +# ends in exit(EXIT_FAILURE) inside portduinoSetup() - neither is reachable from +# a native unit test that links a single translation unit. +# +# Usage: +# ./bin/test-config-check.sh # auto-detect the binary +# ./bin/test-config-check.sh path/to/meshtasticd # explicit binary +# MESHTASTICD_BIN=... ./bin/test-config-check.sh +# +# Exit codes: 0 = GREEN, 1 = RED. The final line is machine-readable: +# RESULT: GREEN 42/42 assertions passed +# RESULT: RED 2 of 42 assertions failed + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +ROOT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" +FIXTURES="$ROOT_DIR/test/fixtures/portduino-config" + +BIN="${1:-${MESHTASTICD_BIN-}}" +if [[ -z $BIN ]]; then + for candidate in "$ROOT_DIR/.pio/build/native/meshtasticd" "$ROOT_DIR/.pio/build/coverage/meshtasticd"; do + [[ -x $candidate ]] && BIN="$candidate" && break + done +fi +if [[ -z $BIN || ! -x $BIN ]]; then + echo "RESULT: RED no meshtasticd binary (build one with 'pio run -e native', or pass a path)" + exit 1 +fi +# Each case runs from a directory of its own choosing, so a relative path (CI passes +# .pio/build/coverage/meshtasticd) has to be resolved before the first cd. +BIN="$(cd "$(dirname "$BIN")" && pwd)/$(basename "$BIN")" + +# Note: `pio test -e native` writes its Unity test program to this same path, so after a test +# run the auto-detected binary is that program rather than the application. That turns out to be +# harmless here -- portduino's main() parses argv and runs portduinoSetup(), which prints the +# report and exit()s before setup() (Unity's entry point) is ever reached -- so every case below +# behaves identically either way. It stops being true only for a case that lets meshtasticd run +# past portduinoSetup(), which this suite deliberately never does: a config it accepts boots a +# node and blocks. + +# The coverage env links ASan/LSan. Every case here ends in exit(), so LSan would +# report the still-reachable config objects and turn a passing case into a non-zero +# exit that has nothing to do with what is being asserted. +export ASAN_OPTIONS="${ASAN_OPTIONS:-detect_leaks=0}" + +# Keep the virtual filesystem out of the user's home, and run from a directory with +# no config.yaml in it so the fixture we name is the only config in play. +WORKDIR="$(mktemp -d -t meshcheck.XXXXXX)" +trap 'rm -rf "$WORKDIR"' EXIT + +PASS=0 +FAIL=0 + +# assert [expected-substring...] +# mode: "check" adds --check, "check-yaml" adds --check --output-yaml, +# "normal" runs with neither. +# fixture: a bare name runs from a scratch directory. A name containing a slash +# (configd-conflict/config.yaml) runs from that fixture's own directory, +# so a relative ConfigDirectory in it resolves the way it would in situ. +assert() { + local desc="$1" want_rc="$2" fixture="$3" mode="$4" + shift 4 + + local cwd="$WORKDIR" config="$FIXTURES/$fixture" + if [[ $fixture == */* ]]; then + cwd="$FIXTURES/$(dirname "$fixture")" + config="$(basename "$fixture")" + fi + + local args=(--config "$config" -d "$WORKDIR/fs") + case $mode in + check) args+=(--check) ;; + check-yaml) args+=(--check --output-yaml) ;; + esac + + local out rc + # A regression that hangs must fail the test, not stall CI. Every case here is + # expected to exit in well under a second. + out="$(cd "$cwd" && timeout 60 "$BIN" "${args[@]}" 2>&1)" + rc=$? + + local problems=() + [[ $rc -ne $want_rc ]] && problems+=("exit $rc, wanted $want_rc") + local needle + for needle in "$@"; do + grep -qF -- "$needle" <<<"$out" || problems+=("missing: $needle") + done + + if [[ ${#problems[@]} -eq 0 ]]; then + PASS=$((PASS + 1)) + echo " ok $desc" + else + FAIL=$((FAIL + 1)) + echo " FAIL $desc" + for problem in "${problems[@]}"; do + echo " $problem" + done + sed 's/^/ | /' <<<"$out" + fi +} + +# A config that meshtasticd fully accepts is asserted clean AND asserted to resolve +# to the module we asked for - otherwise a silent fallback to sim would still pass. +assert_clean_module() { + local fixture="$1" module="$2" + assert "$module" 0 "$fixture" check \ + "Result: 0 errors, 0 warnings" \ + "Module : $module" +} + +echo "meshtasticd config-check tests" +echo " binary: $BIN" +echo + +echo "a valid config for every radio module family is clean:" +assert_clean_module module-rf95.yaml RF95 +assert_clean_module module-sx1262.yaml sx1262 +assert_clean_module module-sx1268.yaml sx1268 +assert_clean_module module-llcc68.yaml LLCC68 +assert_clean_module module-sx1280.yaml sx1280 +assert_clean_module module-lr1110.yaml lr1110 +assert_clean_module module-lr1120.yaml lr1120 +assert_clean_module module-lr1121.yaml lr1121 +assert_clean_module module-sim.yaml sim +assert_clean_module module-auto.yaml auto + +echo +echo "other configs that must not be flagged:" +assert "minimal valid config" 0 valid.yaml check \ + "Result: 0 errors, 0 warnings" +assert "empty sections are not a fault" 0 empty-sections.yaml check \ + "Result: 0 errors, 0 warnings" +assert "warnings alone do not fail the run" 0 unknown-key.yaml check \ + "unknown key 'Lora.Frequency'" \ + "Result: 0 errors, 1 warning" + +echo +echo "module names are matched exactly:" +assert "unknown module names the valid set" 1 module-unknown.yaml check \ + "Lora.Module 'sx1263' is not a module meshtasticd knows" \ + "Result: 1 error, 0 warnings" +assert "wrong-case module suggests the right spelling" 1 module-wrong-case.yaml check \ + "did you mean 'LLCC68'?" \ + "Result: 1 error, 0 warnings" + +echo +echo "LR11xx rfswitch table:" +assert "unrecognised switch pin" 1 rfswitch-bad-pin.yaml check \ + "'DIO9' is not a recognised pin" \ + "Result: 1 error, 0 warnings" +assert "row length must match the pin count" 1 rfswitch-row-length.yaml check \ + "MODE_STBY has 2 values but 3 pins are declared" \ + "MODE_RX has 4 values but 3 pins are declared" \ + "Result: 2 errors, 0 warnings" +# Anything not exactly "HIGH" is silently treated as LOW, so lowercase "high" is a +# switch that never closes - the failure this check exists to catch. +assert "levels must be exactly HIGH or LOW" 1 rfswitch-bad-level.yaml check \ + "'high' is not HIGH or LOW" \ + "'On' is not HIGH or LOW" \ + "Result: 2 errors, 0 warnings" +assert "table with no pins list" 1 rfswitch-no-pins.yaml check \ + "has no 'pins' list, so no switch pins are driven" \ + "Result: 1 error, 0 warnings" +assert "only the first 5 pins are read" 1 rfswitch-too-many-pins.yaml check \ + "lists 6 pins but only the first 5 are read" \ + "Result: 1 error, 0 warnings" +assert "table must be a mapping" 1 rfswitch-not-a-map.yaml check \ + "Lora.rfswitch_table must be a mapping" \ + "Result: 1 error, 0 warnings" +assert "unknown MODE_ key" 1 rfswitch-unknown-mode.yaml check \ + "unknown key 'Lora.rfswitch_table.MODE_TRANSMIT'" \ + "Result: 1 error, 0 warnings" +# One level of over-indentation strands MODE_ rows under Lora: instead of under the +# table, where they do nothing. The hint is what makes that findable. +assert "MODE_ row stranded outside the table" 0 rfswitch-stranded-modes.yaml check \ + "unknown key 'Lora.MODE_RX'" \ + "It is a valid key of Lora.rfswitch_table" \ + "Result: 0 errors, 1 warning" +# A partial table is legal, but the modes left out are driven all-LOW, which for most +# modules is the shutdown state - so the report says which ones rather than leaving it +# to be discovered on air. +assert "omitted modes are called out" 0 rfswitch-partial.yaml check \ + "omits MODE_GNSS, MODE_TX_HF, MODE_TX_HP, MODE_WIFI" \ + "default to all pins LOW" \ + "Result: 0 errors, 0 warnings" + +echo +echo "radio module and switch table must agree:" +assert "LR11xx without a table cannot transmit" 0 module-mismatch-lr11xx.yaml check \ + "Module is lr1121 but no Lora.rfswitch_table is set" \ + "Result: 0 errors, 1 warning" +assert "table on a non-LR11xx radio is ignored" 0 module-mismatch-sx126x.yaml check \ + "the table is only applied to LR11xx radios" \ + "Result: 0 errors, 1 warning" + +echo +echo "PA gain table (TX_GAIN_LORA):" +# Both shapes are legal and they fail differently. The scalar case is a regression +# guard: an earlier version of the checker called this working config a fatal error. +assert "a bare scalar is legal" 0 txgain-scalar.yaml check \ + "Result: 0 errors, 0 warnings" +assert "a non-numeric list entry stops meshtasticd" 1 value-type-fatal-list.yaml check \ + "TX_GAIN_LORA entry 'high' is not a whole number" \ + "refuses to start" \ + "Result: 1 error, 0 warnings" +assert "entries outside the uint16 range wrap" 1 txgain-out-of-range.yaml check \ + "entry -5 does not fit the 0-65535 range" \ + "entry 70000 does not fit the 0-65535 range" \ + "Result: 2 errors, 0 warnings" +assert "points past the 22nd are dropped" 0 txgain-too-many.yaml check \ + "lists 25 points but only the first 22 are stored" \ + "Result: 0 errors, 1 warning" + +echo +echo "values of the wrong type:" +# The two settings read without a fallback: a bad value throws inside loadConfig() and +# meshtasticd will not start. Before this check, --check called these files clean. +assert "no-fallback read is fatal" 1 value-type-fatal.yaml check \ + "Logging.AsciiLogs is not a true/false value" \ + "refuses to start" \ + "Result: 1 error, 0 warnings" +assert "defaulted reads are silently ignored" 0 value-type-silent.yaml check \ + "Lora.spiSpeed is not a whole number" \ + "Lora.DIO2_AS_RF_SWITCH is not a true/false value" \ + "General.MaxNodes is not a whole number" \ + "silently replaced by the default" \ + "Result: 0 errors, 4 warnings" + +echo +echo "out-of-range and unit mistakes:" +# The volts/millivolts trap: every other Meshtastic surface says millivolts, so 1800 is +# the natural thing to write and it silently asks for 1800V. +assert "TCXO voltage written in millivolts" 1 tcxo-millivolts.yaml check \ + "resolves to 1800000 mV" \ + "The value is in VOLTS" \ + "Result: 1 error, 0 warnings" +assert "ports outside their usable range" 1 port-out-of-range.yaml check \ + "General.APIPort 80 is outside 1024-65535" \ + "Webserver.Port 99999 is not a usable TCP port" \ + "Result: 1 error, 1 warning" +assert "StatusMessage longer than its buffer" 0 statusmessage-long.yaml check \ + "is truncated to 79 when it is stored" \ + "Result: 0 errors, 1 warning" +# Regression guard for a crash: this used to abort meshtasticd (and --check with it) +# via an uncaught filesystem_error from directory_iterator. +assert "unreadable ConfigDirectory is reported, not fatal" 1 configdir-missing.yaml check \ + "is not a directory that can be read" \ + "Result: 1 error, 0 warnings" + +echo +echo "MAC address sources:" +assert "both MACAddress and MACAddressSource" 1 mac-conflict.yaml check \ + "General.MACAddress and General.MACAddressSource are both set" \ + "Result: 1 error, 0 warnings" +assert "MACAddress that is not 12 hex digits" 1 mac-malformed.yaml check \ + "is not 12 hex digits" \ + "Result: 1 error, 0 warnings" +assert "MACAddressSource naming no interface" 0 mac-source-missing.yaml check \ + "has no /sys/class/net/nosuchiface99/address" \ + "Result: 0 errors, 1 warning" + +echo +echo "pin mappings:" +assert "unknown pin sub-key" 1 pin-unknown-subkey.yaml check \ + "unknown key 'Lora.CS.chipline'" \ + "A pin mapping accepts only pin, gpiochip and line" \ + "Result: 1 error, 0 warnings" +assert "pin value that resolves to -1" 1 pin-unreadable.yaml check \ + "Lora.CS is set, but its value could not be read as a pin number" \ + "Result: 1 error, 0 warnings" + +echo +echo "CH341 USB-SPI adapters:" +# The Lora pins of a ch341 device are indexes on the adapter, driven by the usermode +# driver: portduinoSetup() skips initGPIOPin() for all of them. Reporting them as +# gpiochip lines to confirm with gpioinfo is wrong on Linux and meaningless on the +# Windows and macOS hosts where a USB adapter is the only way to attach a radio. +assert "adapter pins are not reported as GPIO lines" 0 usb-ch341.yaml check \ + "CH341 adapter pins (driven over USB, not claimed from a gpiochip)" \ + "pin indexes on the CH341 itself" \ + "Module : sx1262" \ + "Result: 0 errors, 0 warnings" +assert "gpiochip mapping on a ch341 device is ignored" 0 ch341-gpiochip.yaml check \ + "Lora.spidev is ch341" \ + "Lora.CS.gpiochip, Lora.CS.line, Lora.gpiochip are read but never used" \ + "Result: 0 errors, 1 warning" + +echo +echo "structural faults:" +assert "duplicate key" 1 duplicate-key.yaml check \ + "duplicate key 'Lora.CS'" \ + "yaml-cpp keeps the FIRST occurrence" \ + "Result: 1 error, 0 warnings" +assert "section body that is not a mapping" 1 nonmap-section.yaml check \ + "'Lora' is not a mapping" \ + "Result: 1 error, 0 warnings" +assert "unknown top-level section" 1 unknown-section.yaml check \ + "unknown top-level section 'Telemetry'" \ + "Result: 1 error, 0 warnings" +assert "key left at the top level" 1 stranded-key.yaml check \ + "'spidev' is a key of Display or Lora or Touchscreen" \ + "indent it one level" \ + "Result: 1 error, 0 warnings" +assert "top level is a list" 1 top-level-list.yaml check \ + "top level is not a mapping" \ + "Result: 1 error, 0 warnings" +assert "empty file" 0 empty-file.yaml check \ + "is empty" \ + "Result: 0 errors, 1 warning" +# Only the unknown key is asserted: the accompanying "built without HUB75 support" +# error depends on whether rgbmatrix was present at build time, so it is not a stable +# expectation across builds. +assert "unknown HUB75 option" 1 hub75-unknown-key.yaml check \ + "unknown key 'Display.HUB75.Colours'" +# The point of the case: a config that will not parse must still reach the report +# with a file and a line, rather than exiting early with just "Unable to use ...". +# The asserted line number counts the fixture's comment header - the only assertion +# here that moves if you edit those comments. +assert "unparseable config still reaches the report" 1 malformed-indent.yaml check \ + "malformed-indent.yaml" \ + "could not be parsed" \ + "error at line 7" \ + "Result: 1 error, 0 warnings" + +echo +echo "across a config directory:" +# Three files each opening a 'Lora:' section. Every key not repeated in the last +# one loaded is silently reset to its default - here config.yaml's TCXO voltage. +# The warning COUNT is deliberately not asserted: the two config.d files name +# different modules, so which one wins - and therefore whether the LR11xx-without-a +# switch-table warning fires - depends on the order the filesystem returns them in, +# which is the very thing this fixture exists to demonstrate. +assert "config.d overrides are reported" 0 configd-conflict/config.yaml check \ + "config.d/lora-a.yaml" \ + "config.d/lora-b.yaml" \ + "not necessarily alphabetical" \ + "files define a 'Lora:' section" \ + "The file loaded last wins" \ + "Result: 0 errors," +# Switch tables are the one place "last wins" is false: the loader only ever writes +# HIGH, so the effective table is the OR of every file. Proven with --output-yaml. +assert "switch tables across files do not override" 1 rfswitch-sticky/config.yaml check \ + "These do NOT override each other" \ + "a HIGH from an earlier file survives a later file that sets LOW" \ + "Enable exactly one" + +echo +echo "--check takes precedence over --output-yaml:" +assert "report wins over the yaml dump" 0 valid.yaml check-yaml \ + "meshtasticd configuration check" \ + "Result: 0 errors, 0 warnings" + +echo +echo "normal operation rejects a bad config:" +assert "unparseable config is refused" 1 malformed-indent.yaml normal \ + "Unable to use" \ + "as config file" +assert "non-mapping section is refused" 1 nonmap-section.yaml normal \ + "Unable to use" \ + "as config file" +assert "unknown module is refused" 1 module-unknown.yaml normal \ + "Unknown Lora.Module: sx1263" +assert "MACAddress conflict is refused" 1 mac-conflict.yaml normal \ + "Cannot set both MACAddress and MACAddressSource!" +# Only meaningful on a build without rgbmatrix. A build that supports HUB75 accepts +# the panel and goes on to start a node, which is not something to assert here. +# Captured rather than piped: this run exits non-zero by design, and pipefail would +# make the pipeline look failed however the grep went. +HUB75_PROBE="$("$BIN" --check --config "$FIXTURES/hub75-unknown-key.yaml" -d "$WORKDIR/fs" 2>&1)" +if grep -qF "built without HUB75 support" <<<"$HUB75_PROBE"; then + assert "unsupported display panel is refused" 1 hub75-unknown-key.yaml normal \ + "this build does not support HUB75" +else + echo " skip unsupported display panel is refused (this build has HUB75 support)" +fi + +echo +TOTAL=$((PASS + FAIL)) +if [[ $FAIL -eq 0 ]]; then + echo "RESULT: GREEN $PASS/$TOTAL assertions passed" + exit 0 +fi +echo "RESULT: RED $FAIL of $TOTAL assertions failed" +exit 1 diff --git a/src/platform/portduino/ConfigCheck.cpp b/src/platform/portduino/ConfigCheck.cpp new file mode 100644 index 000000000..aa0f0c4cb --- /dev/null +++ b/src/platform/portduino/ConfigCheck.cpp @@ -0,0 +1,1094 @@ +#include "ConfigCheck.h" + +#ifndef ARCH_PORTDUINO_WASM + +#include "configuration.h" + +#include "PortduinoGlue.h" + +#include "yaml-cpp/eventhandler.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ + +// --------------------------------------------------------------------------- +// Schema +// --------------------------------------------------------------------------- +// Mirrors the keys loadConfig() reads: meshtasticd silently ignores anything else, so +// an unlisted key is honestly "unknown". Teach loadConfig() a new key, add it here too -- +// CI runs --check over bin/config.d/**, so an omission fails the build rather than a user. + +const std::set kLoraPinKeys = {"CS", "IRQ", "Busy", "Reset", "TXen", "RXen", "SX126X_ANT_SW", "GPIO_DETECT_PA"}; + +const std::map> &schema() +{ + static const std::map> s = { + {"Lora", + {"Module", + "gpiochip", + "spidev", + "spiSpeed", + "DIO2_AS_RF_SWITCH", + "DIO3_TCXO_VOLTAGE", + "Enable_Pins", + "rfswitch_table", + "LR1110_MAX_POWER", + "LR1120_MAX_POWER", + "RF95_MAX_POWER", + "SX126X_MAX_POWER", + "SX128X_MAX_POWER", + "TX_GAIN_LORA", + "USB_PID", + "USB_VID", + "USB_Serialnum", + "CS", + "IRQ", + "Busy", + "Reset", + "TXen", + "RXen", + "SX126X_ANT_SW", + "GPIO_DETECT_PA"}}, + {"General", + {"MACAddress", "MACAddressSource", "MaxNodes", "MaxMessageQueue", "APIPort", "ConfigDirectory", "AvailableDirectory"}}, + {"Config", {"DisplayMode", "EnableUDP", "StatusMessage"}}, + {"Display", + {"Panel", "spidev", "BusFrequency", "Width", "Height", "Invert", "Rotate", "OffsetX", "OffsetY", "OffsetRotate", + "RGBOrder", "HUB75", "DC", "CS", "Backlight", "BacklightInvert", "BacklightPWMChannel", "Reset"}}, + {"Touchscreen", {"Module", "spidev", "BusFrequency", "I2CAddr", "Rotate", "CS", "IRQ"}}, + {"Input", + {"KeyboardDevice", "PointerDevice", "JoystickDevice", "JoystickButtons", "TrackballDirection", "User", "TrackballUp", + "TrackballDown", "TrackballLeft", "TrackballRight", "TrackballPress"}}, + {"GPIO", {"User", "ExtraPins"}}, + {"GPS", {"SerialPath", "GpsdHost", "GpsdPort"}}, + {"I2C", {"I2CDevice"}}, + {"Logging", {"LogLevel", "TraceFile", "JSONFile", "JSONFileRotate", "JSONFilter", "AsciiLogs"}}, + {"Webserver", {"Port", "RootPath", "SSLCert", "SSLKey"}}, + {"HostMetrics", {"ReportInterval", "Channel", "UserStringCommand"}}, + // Read by packaging/menu tooling rather than by meshtasticd itself. + {"Meta", {}}, + }; + return s; +} + +// Sections whose contents meshtasticd never reads, so unknown keys inside them are +// not worth reporting. +const std::set kFreeFormSections = {"Meta"}; + +const std::set kPinSubKeys = {"pin", "gpiochip", "line"}; + +const std::set kHub75Keys = { + "HardwareMapping", "Rows", "Cols", "ChainLength", "Parallel", "PWMBits", + "PWMLSBNanoseconds", "Brightness", "ScanMode", "RowAddressType", "Multiplexing", "DisableHardwarePulsing", + "ShowRefreshRate", "InverseColors", "RGBSequence", "PixelMapper", "PanelType", "LimitRefreshRateHz", + "GPIOSlowdown"}; + +const std::set kRfSwitchModes = {"MODE_STBY", "MODE_RX", "MODE_TX", "MODE_TX_HP", + "MODE_TX_HF", "MODE_GNSS", "MODE_WIFI"}; + +const std::set kRfSwitchPins = {"DIO5", "DIO6", "DIO7", "DIO8", "DIO10"}; + +// Reverse index: key name -> sections it is valid in. Powers the "you probably meant +// to nest this under X" hint that turns a silent no-op into an actionable message. +const std::map> &keyOwners() +{ + static const std::map> owners = [] { + std::map> m; + for (const auto §ion : schema()) + for (const auto &key : section.second) + m[key].insert(section.first); + // rfswitch_table's sub-keys are worth hinting on too: a table indented one + // level too far leaves MODE_* rows stranded directly under Lora. + for (const auto &mode : kRfSwitchModes) + m[mode].insert("Lora.rfswitch_table"); + m["pins"].insert("Lora.rfswitch_table"); + return m; + }(); + return owners; +} + +// --------------------------------------------------------------------------- +// Findings +// --------------------------------------------------------------------------- + +enum Level { kInfo, kWarn, kError }; + +struct Finding { + Level level; + std::string file; + int line; // 1-based; 0 when the finding is not tied to a line + std::string message; +}; + +const char *levelName(Level l) +{ + switch (l) { + case kError: + return "ERROR"; + case kWarn: + return "WARN "; + default: + return "INFO "; + } +} + +std::string joinSections(const std::set &s) +{ + std::string out; + for (const auto &item : s) { + if (!out.empty()) + out += " or "; + out += item; + } + return out; +} + +int lineOf(const YAML::Node &node) +{ + const YAML::Mark mark = node.Mark(); + return mark.is_null() ? 0 : mark.line + 1; +} + +// --------------------------------------------------------------------------- +// Duplicate key detection +// --------------------------------------------------------------------------- +// yaml-cpp silently keeps the FIRST of duplicate keys, so a later override looks applied +// but is not. The Node API sees an already-collapsed map, so walk the parser events. + +class DuplicateKeyFinder : public YAML::EventHandler +{ + public: + struct Duplicate { + std::string path; + int line; + int firstLine; + }; + std::vector duplicates; + + void OnDocumentStart(const YAML::Mark &) override {} + void OnDocumentEnd() override {} + void OnNull(const YAML::Mark &, YAML::anchor_t) override { advance(); } + void OnAlias(const YAML::Mark &, YAML::anchor_t) override { advance(); } + void OnAnchor(const YAML::Mark &, const std::string &) override {} + + void OnScalar(const YAML::Mark &mark, const std::string &, YAML::anchor_t, const std::string &value) override + { + if (!stack.empty() && stack.back().isMap && stack.back().expectKey) { + auto &seen = stack.back().seen; + const auto existing = seen.find(value); + if (existing != seen.end()) + duplicates.push_back({pathTo(value), static_cast(mark.line) + 1, existing->second}); + else + seen.emplace(value, static_cast(mark.line) + 1); + stack.back().key = value; + } + advance(); + } + + void OnSequenceStart(const YAML::Mark &, const std::string &, YAML::anchor_t, YAML::EmitterStyle::value) override + { + push(false); + } + void OnSequenceEnd() override { pop(); } + void OnMapStart(const YAML::Mark &, const std::string &, YAML::anchor_t, YAML::EmitterStyle::value) override { push(true); } + void OnMapEnd() override { pop(); } + + private: + struct Context { + bool isMap; + bool expectKey; + std::string key; + std::map seen; + }; + std::vector stack; + + // Inside a mapping the parser alternates key, value, key, value... + void advance() + { + if (!stack.empty() && stack.back().isMap) + stack.back().expectKey = !stack.back().expectKey; + } + void push(bool isMap) + { + advance(); // the collection itself occupies a slot in its parent + stack.push_back(Context{isMap, true, "", {}}); + } + // No advance(): push() already consumed the collection's slot in the parent. Guarded + // because balance rests on yaml-cpp emitting matched start/end events -- its invariant. + void pop() + { + if (!stack.empty()) + stack.pop_back(); + } + std::string pathTo(const std::string &leaf) const + { + std::string path; + for (size_t i = 0; i + 1 < stack.size(); i++) + if (stack[i].isMap && !stack[i].key.empty()) + path += stack[i].key + "."; + return path + leaf; + } +}; + +void checkDuplicateKeys(const std::string &file, std::vector &findings) +{ + std::ifstream stream(file); + if (!stream) + return; + DuplicateKeyFinder finder; + try { + YAML::Parser parser(stream); + while (parser.HandleNextDocument(finder)) { + } + } catch (const std::exception &) { + return; // reported separately by checkFile() + } + for (const auto &dup : finder.duplicates) + findings.push_back({kError, file, dup.line, + "duplicate key '" + dup.path + "' (first defined on line " + std::to_string(dup.firstLine) + + "). yaml-cpp keeps the FIRST occurrence, so this one is silently discarded"}); +} + +// --------------------------------------------------------------------------- +// Structural checks +// --------------------------------------------------------------------------- + +void checkPinNode(const std::string &file, const std::string &path, const YAML::Node &node, std::vector &findings) +{ + if (!node.IsMap()) + return; // plain scalar pin number, always fine + for (const auto &entry : node) { + const std::string key = entry.first.as(""); + if (!kPinSubKeys.count(key)) + findings.push_back({kError, file, lineOf(entry.first), + "unknown key '" + path + "." + key + "'. A pin mapping accepts only pin, gpiochip and line"}); + } +} + +void checkRfSwitchTable(const std::string &file, const YAML::Node &table, std::vector &findings) +{ + if (!table.IsMap()) { + findings.push_back({kError, file, lineOf(table), "Lora.rfswitch_table must be a mapping"}); + return; + } + + size_t pinCount = 0; + if (const YAML::Node pins = table["pins"]) { + if (!pins.IsSequence()) { + findings.push_back({kError, file, lineOf(pins), "Lora.rfswitch_table.pins must be a list"}); + } else { + pinCount = pins.size(); + for (const auto &pin : pins) { + const std::string name = pin.as(""); + if (!kRfSwitchPins.count(name)) + findings.push_back({kError, file, lineOf(pin), + "Lora.rfswitch_table.pins: '" + name + + "' is not a recognised pin. Valid values are DIO5, DIO6, DIO7, DIO8 and DIO10"}); + } + if (pinCount > 5) + findings.push_back( + {kError, file, lineOf(pins), + "Lora.rfswitch_table.pins lists " + std::to_string(pinCount) + " pins but only the first 5 are read"}); + } + } else { + findings.push_back({kError, file, lineOf(table), "Lora.rfswitch_table has no 'pins' list, so no switch pins are driven"}); + } + + for (const auto &entry : table) { + const std::string key = entry.first.as(""); + if (key == "pins") + continue; + if (!kRfSwitchModes.count(key)) { + findings.push_back({kError, file, lineOf(entry.first), "unknown key 'Lora.rfswitch_table." + key + "'"}); + continue; + } + const YAML::Node &row = entry.second; + if (!row.IsSequence()) { + findings.push_back({kError, file, lineOf(row), "Lora.rfswitch_table." + key + " must be a list"}); + continue; + } + if (pinCount && row.size() != pinCount) + findings.push_back({kError, file, lineOf(row), + "Lora.rfswitch_table." + key + " has " + std::to_string(row.size()) + " values but " + + std::to_string(pinCount) + " pins are declared"}); + for (const auto &value : row) { + const std::string level = value.as(""); + if (level != "HIGH" && level != "LOW") + findings.push_back({kError, file, lineOf(value), + "Lora.rfswitch_table." + key + ": '" + level + + "' is not HIGH or LOW. Anything that is not exactly \"HIGH\" is treated as LOW"}); + } + } + + // Every mode absent from the table defaults to all-LOW, which for most modules is + // the shutdown state. Worth saying out loud rather than leaving to be discovered. + std::vector missing; + for (const auto &mode : kRfSwitchModes) + if (!table[mode]) + missing.push_back(mode); + if (!missing.empty()) { + std::string list; + for (const auto &mode : missing) + list += (list.empty() ? "" : ", ") + mode; + findings.push_back( + {kInfo, file, lineOf(table), "Lora.rfswitch_table omits " + list + "; those modes default to all pins LOW"}); + } +} + +// --------------------------------------------------------------------------- +// Value types +// --------------------------------------------------------------------------- +// A wrong-typed value silently does nothing under .as(default), but the two reads with +// NO default (Logging.AsciiLogs, TX_GAIN_LORA) throw and stop meshtasticd. Conversions are +// tested by asking yaml-cpp, so this cannot drift from what loadConfig() accepts. + +enum ValueType { kBool, kInt, kFloat, kString, kIntList, kBoolOrFloat, kIntOrString }; + +struct ValueSpec { + ValueType type; + bool fatal; // read without a default: a bad value stops meshtasticd outright +}; + +const std::map &valueSpecs() +{ + static const std::map s = { + {"Lora.spiSpeed", {kInt, false}}, + {"Lora.gpiochip", {kInt, false}}, + {"Lora.spidev", {kString, false}}, + {"Lora.DIO2_AS_RF_SWITCH", {kBool, false}}, + // Accepts a float (volts) or `true` (meaning 1.8V), so both are allowed here. + {"Lora.DIO3_TCXO_VOLTAGE", {kBoolOrFloat, false}}, + {"Lora.LR1110_MAX_POWER", {kInt, false}}, + {"Lora.LR1120_MAX_POWER", {kInt, false}}, + {"Lora.RF95_MAX_POWER", {kInt, false}}, + {"Lora.SX126X_MAX_POWER", {kInt, false}}, + {"Lora.SX128X_MAX_POWER", {kInt, false}}, + // TX_GAIN_LORA is not in this table: it accepts a list OR a bare scalar, and + // only the list path is fatal. See checkTxGain(). + {"Lora.USB_PID", {kInt, false}}, + {"Lora.USB_VID", {kInt, false}}, + {"Lora.USB_Serialnum", {kString, false}}, + {"General.MaxNodes", {kInt, false}}, + {"General.MaxMessageQueue", {kInt, false}}, + {"General.APIPort", {kInt, false}}, + {"General.ConfigDirectory", {kString, false}}, + {"General.AvailableDirectory", {kString, false}}, + {"Config.DisplayMode", {kString, false}}, + {"Config.EnableUDP", {kBool, false}}, + {"Config.StatusMessage", {kString, false}}, + {"Display.Panel", {kString, false}}, + {"Display.spidev", {kString, false}}, + {"Display.BusFrequency", {kInt, false}}, + {"Display.Width", {kInt, false}}, + {"Display.Height", {kInt, false}}, + {"Display.Invert", {kBool, false}}, + {"Display.Rotate", {kInt, false}}, + {"Display.OffsetX", {kInt, false}}, + {"Display.OffsetY", {kInt, false}}, + {"Display.OffsetRotate", {kInt, false}}, + {"Display.RGBOrder", {kBool, false}}, + {"Display.BacklightInvert", {kBool, false}}, + {"Touchscreen.Module", {kString, false}}, + {"Touchscreen.spidev", {kString, false}}, + {"Touchscreen.BusFrequency", {kInt, false}}, + {"Touchscreen.I2CAddr", {kInt, false}}, + {"Touchscreen.Rotate", {kBool, false}}, + {"Input.KeyboardDevice", {kString, false}}, + {"Input.PointerDevice", {kString, false}}, + {"Input.JoystickDevice", {kString, false}}, + {"Input.TrackballDirection", {kString, false}}, + {"GPS.SerialPath", {kString, false}}, + {"GPS.GpsdHost", {kString, false}}, + {"GPS.GpsdPort", {kInt, false}}, + {"I2C.I2CDevice", {kString, false}}, + {"Logging.LogLevel", {kString, false}}, + {"Logging.TraceFile", {kString, false}}, + {"Logging.JSONFile", {kString, false}}, + {"Logging.JSONFileRotate", {kInt, false}}, + // Read as an int and as a string (the "textmessage"-style aliases). + {"Logging.JSONFilter", {kIntOrString, false}}, + {"Logging.AsciiLogs", {kBool, true}}, + {"Webserver.Port", {kInt, false}}, + {"Webserver.RootPath", {kString, false}}, + {"Webserver.SSLCert", {kString, false}}, + {"Webserver.SSLKey", {kString, false}}, + {"HostMetrics.ReportInterval", {kInt, false}}, + {"HostMetrics.Channel", {kInt, false}}, + {"HostMetrics.UserStringCommand", {kString, false}}, + {"Display.HUB75.Rows", {kInt, false}}, + {"Display.HUB75.Cols", {kInt, false}}, + {"Display.HUB75.ChainLength", {kInt, false}}, + {"Display.HUB75.Parallel", {kInt, false}}, + {"Display.HUB75.PWMBits", {kInt, false}}, + {"Display.HUB75.PWMLSBNanoseconds", {kInt, false}}, + {"Display.HUB75.Brightness", {kInt, false}}, + {"Display.HUB75.ScanMode", {kInt, false}}, + {"Display.HUB75.RowAddressType", {kInt, false}}, + {"Display.HUB75.Multiplexing", {kInt, false}}, + {"Display.HUB75.GPIOSlowdown", {kInt, false}}, + {"Display.HUB75.LimitRefreshRateHz", {kInt, false}}, + {"Display.HUB75.DisableHardwarePulsing", {kBool, false}}, + {"Display.HUB75.ShowRefreshRate", {kBool, false}}, + {"Display.HUB75.InverseColors", {kBool, false}}, + {"Display.HUB75.HardwareMapping", {kString, false}}, + {"Display.HUB75.RGBSequence", {kString, false}}, + {"Display.HUB75.PixelMapper", {kString, false}}, + {"Display.HUB75.PanelType", {kString, false}}, + }; + return s; +} + +const char *typeName(ValueType type) +{ + switch (type) { + case kBool: + return "a true/false value"; + case kInt: + return "a whole number"; + case kFloat: + return "a number"; + case kIntList: + return "a list of whole numbers"; + case kBoolOrFloat: + return "a number or true/false"; + case kIntOrString: + return "a whole number or a name"; + default: + return "a text value"; + } +} + +// Ask yaml-cpp to perform the same conversion loadConfig() will. +bool converts(const YAML::Node &node, ValueType type) +{ + try { + switch (type) { + case kBool: + node.as(); + return true; + case kInt: + node.as(); + return true; + case kFloat: + node.as(); + return true; + case kBoolOrFloat: + try { + node.as(); + return true; + } catch (const std::exception &) { + node.as(); + return true; + } + case kIntOrString: + try { + node.as(); + return true; + } catch (const std::exception &) { + node.as(); + return true; + } + case kIntList: + if (!node.IsSequence()) + return false; + for (const auto &item : node) + item.as(); + return true; + default: + node.as(); + return true; + } + } catch (const std::exception &) { + return false; + } +} + +void checkValueType(const std::string &file, const std::string &path, const YAML::Node &value, std::vector &findings) +{ + const auto spec = valueSpecs().find(path); + if (spec == valueSpecs().end() || converts(value, spec->second.type)) + return; + + if (spec->second.fatal) + findings.push_back({kError, file, lineOf(value), + path + " is not " + typeName(spec->second.type) + + ". This one is read without a fallback, so the conversion throws and meshtasticd " + "refuses to start on this file"}); + else + findings.push_back({kWarn, file, lineOf(value), + path + " is not " + typeName(spec->second.type) + + ", so it is silently replaced by the default and the setting does nothing"}); +} + +// The PA gain table's two shapes fail differently: a bad list entry stops meshtasticd (no +// default), a bad scalar falls back to 0. Backed by uint16_t[22], so extras drop and values wrap. +void checkTxGain(const std::string &file, const YAML::Node &node, std::vector &findings) +{ + constexpr size_t kMaxPaPoints = 22; + + if (node.IsSequence()) { + if (node.size() > kMaxPaPoints) + findings.push_back({kWarn, file, lineOf(node), + "Lora.TX_GAIN_LORA lists " + std::to_string(node.size()) + " points but only the first " + + std::to_string(kMaxPaPoints) + " are stored; the rest are dropped"}); + for (const auto &point : node) { + if (!converts(point, kInt)) { + findings.push_back({kError, file, lineOf(point), + "Lora.TX_GAIN_LORA entry '" + point.as("") + + "' is not a whole number. List entries are read without a fallback, so this " + "throws and meshtasticd refuses to start on this file"}); + continue; + } + const int value = point.as(); + // Stored into a uint16_t, so anything outside the range silently wraps. + if (value < 0 || value > 65535) + findings.push_back({kError, file, lineOf(point), + "Lora.TX_GAIN_LORA entry " + std::to_string(value) + + " does not fit the 0-65535 range it is stored in, so it wraps to a different " + "gain than the one written"}); + } + return; + } + + if (node.IsMap()) { + findings.push_back({kError, file, lineOf(node), "Lora.TX_GAIN_LORA must be a list of gain points, or a single number"}); + return; + } + + if (!converts(node, kInt)) + findings.push_back({kWarn, file, lineOf(node), + "Lora.TX_GAIN_LORA is not a whole number, so it is silently read as 0 and no PA gain is applied"}); +} + +// Module names match exactly and are inconsistently cased (RF95 upper, sx1262 lower), +// and loadConfig() exits on an unknown name without printing the valid set, so name it here. +void checkLoraModule(const std::string &file, const YAML::Node &module, std::vector &findings) +{ + const std::string name = module.as(""); + if (name.empty()) + return; + + std::string valid; + std::string caseHint; + for (const auto &known : portduino_config.loraModules) { + if (name == known.second) + return; + valid += (valid.empty() ? "" : ", ") + known.second; + if (caseHint.empty() && name.size() == known.second.size() && + std::equal(name.begin(), name.end(), known.second.begin(), [](char a, char b) { + // Cast first: tolower() on a negative char is undefined, and a stray + // non-ASCII byte in a hand-edited config is exactly how that happens. + return std::tolower(static_cast(a)) == std::tolower(static_cast(b)); + })) + caseHint = known.second; + } + + std::string message = + "Lora.Module '" + name + "' is not a module meshtasticd knows, and it refuses to start. Valid: " + valid; + if (!caseHint.empty()) + message += ". The name is matched exactly -- did you mean '" + caseHint + "'?"; + findings.push_back({kError, file, lineOf(module), message}); +} + +// loadConfig() rejects a bad MACAddress or MACAddressSource silently, falling through to the +// BlueZ and LoRa-serial fallbacks; if those yield nothing, meshtasticd exits on a blank MAC. +void checkMacAddress(const std::string &file, const YAML::Node &general, std::vector &findings) +{ + const YAML::Node address = general["MACAddress"]; + const YAML::Node source = general["MACAddressSource"]; + const std::string addressText = address ? address.as("") : ""; + const std::string sourceText = source ? source.as("") : ""; + + if (!addressText.empty() && !sourceText.empty()) { + findings.push_back({kError, file, lineOf(source), + "General.MACAddress and General.MACAddressSource are both set. meshtasticd refuses to start " + "with both; keep whichever one you want the MAC to come from"}); + return; + } + + if (!addressText.empty()) { + std::string digits; + for (const char c : addressText) + if (c != ':' && c != '-') + digits += c; + const bool hex = digits.find_first_not_of("0123456789abcdefABCDEF") == std::string::npos; + // loadConfig() strips the colons and then requires more than 11 characters; + // anything shorter is dropped without a word and the MAC comes from elsewhere. + if (digits.size() < 12 || !hex) + findings.push_back({kError, file, lineOf(address), + "General.MACAddress '" + addressText + + "' is not 12 hex digits, so it is ignored and the MAC falls back to the Bluetooth " + "adapter or the LoRa device serial. If neither yields one, meshtasticd exits with " + "'Blank MAC Address not allowed!'"}); + } + + if (!sourceText.empty()) { + const std::string path = "/sys/class/net/" + sourceText + "/address"; + std::ifstream probe(path); + if (!probe.good()) + findings.push_back({kWarn, file, lineOf(source), + "General.MACAddressSource '" + sourceText + "' has no " + path + + " on this machine, so the MAC reads back empty and meshtasticd asks you to set one. " + "Expected if you are checking this config on a different host"}); + } +} + +void checkSection(const std::string &file, const std::string §ion, const YAML::Node &body, std::vector &findings) +{ + const auto &allowed = schema().at(section); + if (kFreeFormSections.count(section)) + return; + // An empty section (`Lora:` with no body) is null, not a map, and loadConfig() tests + // before reading it. Any other non-map shape is unreadable, so do not call the file clean. + if (body.IsNull()) + return; + if (!body.IsMap()) { + findings.push_back({kError, file, lineOf(body), "'" + section + "' is not a mapping, so nothing in it is read"}); + return; + } + + for (const auto &entry : body) { + const std::string key = entry.first.as(""); + const YAML::Node &value = entry.second; + + if (!allowed.count(key)) { + std::string message = "unknown key '" + section + "." + key + "', ignored by meshtasticd"; + const auto owner = keyOwners().find(key); + if (owner != keyOwners().end() && !owner->second.count(section)) + message += ". It is a valid key of " + joinSections(owner->second); + findings.push_back({kWarn, file, lineOf(entry.first), message}); + continue; + } + + checkValueType(file, section + "." + key, value, findings); + + if (section == "Lora" && key == "rfswitch_table") { + checkRfSwitchTable(file, value, findings); + } else if (section == "Lora" && key == "Module") { + checkLoraModule(file, value, findings); + } else if (section == "Lora" && key == "TX_GAIN_LORA") { + checkTxGain(file, value, findings); + } else if (section == "Display" && key == "HUB75") { + if (value.IsMap()) + for (const auto &hub : value) { + const std::string hubKey = hub.first.as(""); + if (!kHub75Keys.count(hubKey)) + findings.push_back( + {kWarn, file, lineOf(hub.first), "unknown key 'Display.HUB75." + hubKey + "', ignored"}); + else + checkValueType(file, "Display.HUB75." + hubKey, hub.second, findings); + } + } else if (key == "Enable_Pins" || key == "ExtraPins") { + if (value.IsSequence()) + for (const auto &pin : value) + checkPinNode(file, section + "." + key, pin, findings); + } else if (key == "JoystickButtons") { + // Free-form: any action name mapped to an evdev code. + } else if ((section == "Lora" && kLoraPinKeys.count(key)) || + (section == "Display" && + (key == "DC" || key == "CS" || key == "Backlight" || key == "BacklightPWMChannel" || key == "Reset")) || + (section == "Touchscreen" && (key == "CS" || key == "IRQ")) || + (section == "Input" && (key == "User" || key.rfind("Trackball", 0) == 0)) || + (section == "GPIO" && key == "User")) { + checkPinNode(file, section + "." + key, value, findings); + } + } +} + +// --------------------------------------------------------------------------- +// Cross-file overlap +// --------------------------------------------------------------------------- +// Every .yaml loads into the same portduino_config, so two files setting the same key +// are not merged: the one loaded LAST wins -- the opposite of the within-file rule. + +// path (below the top-level section) -> file -> line of its first appearance there +using PathIndex = std::map>; + +void collectPaths(const std::string &file, const YAML::Node &node, const std::string &prefix, int depth, PathIndex &index) +{ + if (!node.IsMap()) + return; + for (const auto &entry : node) { + const std::string key = entry.first.as(""); + const std::string path = prefix.empty() ? key : prefix + "." + key; + // depth 0 is the top-level section; several files each having a "Lora:" is + // normal, so only record what lives inside a section. + if (depth >= 1) + index[path].emplace(file, lineOf(entry.first)); + collectPaths(file, entry.second, path, depth + 1, index); + } +} + +void checkFile(const std::string &file, std::vector &findings, PathIndex &paths, + std::map> §ionOwners) +{ + YAML::Node doc; + try { + doc = YAML::LoadFile(file); + } catch (const std::exception &e) { + findings.push_back({kError, file, 0, std::string("could not be parsed, so it is being ignored entirely: ") + e.what()}); + return; + } + + if (doc.IsNull()) { + findings.push_back({kWarn, file, 0, "is empty"}); + return; + } + if (!doc.IsMap()) { + findings.push_back({kError, file, 0, "top level is not a mapping, so nothing in it is read"}); + return; + } + + checkDuplicateKeys(file, findings); + collectPaths(file, doc, "", 0, paths); + + for (const auto &entry : doc) { + const std::string sectionName = entry.first.as(""); + // A file that repeats a section still counts once here; the repeat itself is + // reported separately as a duplicate key. + auto &owners = sectionOwners[sectionName]; + if (schema().count(sectionName) && (owners.empty() || owners.back() != file)) + owners.push_back(file); + } + + for (const auto &entry : doc) { + const std::string section = entry.first.as(""); + if (schema().count(section)) { + checkSection(file, section, entry.second, findings); + if (section == "General" && entry.second.IsMap()) + checkMacAddress(file, entry.second, findings); + continue; + } + std::string message = "unknown top-level section '" + section + "', ignored by meshtasticd"; + const auto owner = keyOwners().find(section); + if (owner != keyOwners().end()) + message += ". '" + section + "' is a key of " + joinSections(owner->second) + + " -- indent it one level so it sits " + "inside that section"; + findings.push_back({kError, file, lineOf(entry.first), message}); + } +} + +std::string describeOwners(const std::map &owners) +{ + std::string out; + for (const auto &owner : owners) { + if (!out.empty()) + out += ", "; + out += owner.first; + if (owner.second) + out += " line " + std::to_string(owner.second); + } + return out; +} + +void checkCrossFileOverlap(const PathIndex &paths, const std::map> §ionOwners, + std::vector &findings) +{ + const std::string across = "(across configuration files)"; + + for (const auto &entry : paths) { + if (entry.second.size() < 2) + continue; + // If an ancestor also collides then the whole subtree is replaced together; + // reporting the parent once is clearer than repeating every leaf under it. + bool coveredByAncestor = false; + std::string ancestor = entry.first; + for (size_t dot = ancestor.rfind('.'); dot != std::string::npos; dot = ancestor.rfind('.')) { + ancestor.resize(dot); + const auto found = paths.find(ancestor); + if (found != paths.end() && found->second.size() >= 2) { + coveredByAncestor = true; + break; + } + } + if (coveredByAncestor) + continue; + + // The one place "last wins" is untrue: the loader only ever writes HIGH, so the effective + // table is the OR of every table loaded -- a switch state belonging to none of them. + if (entry.first == "Lora.rfswitch_table") { + findings.push_back({kError, across, 0, + "'Lora.rfswitch_table' is set in " + std::to_string(entry.second.size()) + " files (" + + describeOwners(entry.second) + + "). These do NOT override each other: the loader only ever writes HIGH, so a HIGH " + "from an earlier file survives a later file that sets LOW there, and the radio ends " + "up driving the OR of every table. Enable exactly one"}); + continue; + } + + findings.push_back({kInfo, across, 0, + "'" + entry.first + "' is set in " + std::to_string(entry.second.size()) + " files (" + + describeOwners(entry.second) + "). The file loaded last wins"}); + } + + // Sections do not merge, and these keys are assigned unconditionally with a default, so a + // later file with a Lora section that omits them silently resets them. + const auto loraOwners = sectionOwners.find("Lora"); + if (loraOwners != sectionOwners.end() && loraOwners->second.size() > 1) { + std::string files; + for (const auto &file : loraOwners->second) + files += (files.empty() ? "" : ", ") + file; + findings.push_back( + {kWarn, across, 0, + std::to_string(loraOwners->second.size()) + " files define a 'Lora:' section (" + files + + "). These keys are re-read with a default every time a Lora section is seen, so any of them not repeated " + "in the last file loaded is reset: spidev, spiSpeed, gpiochip, DIO2_AS_RF_SWITCH, DIO3_TCXO_VOLTAGE, " + "USB_PID, USB_VID, USB_Serialnum. Normally exactly one Lora config should be enabled at a time"}); + } +} + +// --------------------------------------------------------------------------- +// Semantic checks against the merged configuration +// --------------------------------------------------------------------------- + +bool isLR11xx(lora_module_enum module) +{ + return module == use_lr1110 || module == use_lr1120 || module == use_lr1121; +} + +std::string moduleName() +{ + const auto it = portduino_config.loraModules.find(portduino_config.lora_module); + return it == portduino_config.loraModules.end() ? "unknown" : it->second; +} + +// A pin whose value will not convert falls back to RADIOLIB_NC (-1) while still marked enabled, +// and initGPIOPin() then trips an assertion inside LinuxGPIOPin rather than failing cleanly. +void checkPinValues(std::vector &findings) +{ + const std::string merged = "(merged configuration)"; + + auto report = [&](const std::string &name) { + findings.push_back({kError, merged, 0, + name + " is set, but its value could not be read as a pin number so it resolves to -1. " + "meshtasticd aborts with an assertion when it tries to claim that line. Check for a " + "non-numeric value, or a stray line folded into it by YAML indentation"}); + }; + + for (const auto *pin : portduino_config.all_pins) + if (pin->enabled && pin->pin < 0) + report(pin->config_section + "." + pin->config_name); + for (const auto &pin : portduino_config.extra_pins) + if (pin.enabled && pin.pin < 0) + report(pin.config_section + "." + pin.config_name); +} + +void checkMergedConfig(const PathIndex &paths, std::vector &findings) +{ + const std::string merged = "(merged configuration)"; + + checkPinValues(findings); + + // portduinoSetup() skips initGPIOPin() for every Lora pin when spidev is ch341, so a + // gpiochip or line mapping written next to one is read, stored, and never used. + if (portduino_config.lora_spi_dev == "ch341") { + auto endsWith = [](const std::string &text, const std::string &suffix) { + return text.size() >= suffix.size() && text.compare(text.size() - suffix.size(), suffix.size(), suffix) == 0; + }; + std::string ignored; + for (const auto &entry : paths) { + if (entry.first.rfind("Lora.", 0) != 0) + continue; + if (entry.first == "Lora.gpiochip" || endsWith(entry.first, ".gpiochip") || endsWith(entry.first, ".line")) + ignored += (ignored.empty() ? "" : ", ") + entry.first; + } + if (!ignored.empty()) + findings.push_back({kWarn, merged, 0, + "Lora.spidev is ch341, so the Lora pins are indexes on the USB adapter and are driven by " + "the usermode driver rather than claimed from a gpiochip. " + + ignored + " are read but never used"}); + } + + if (isLR11xx(portduino_config.lora_module) && !portduino_config.has_rfswitch_table) + findings.push_back({kWarn, merged, 0, + "Module is " + moduleName() + + " but no Lora.rfswitch_table is set, so setRfSwitchTable() is never called. Most LR11xx " + "modules cannot transmit or receive without one"}); + + if (!isLR11xx(portduino_config.lora_module) && portduino_config.has_rfswitch_table) + findings.push_back( + {kWarn, merged, 0, + "a Lora.rfswitch_table is set but Module is " + moduleName() + ", and the table is only applied to LR11xx radios"}); + + // Either way -- the old uncaught filesystem_error abort or today's clean exit -- the files + // meant to configure the radio are not being loaded. + if (!portduino_config.config_directory.empty()) { + std::error_code error; + if (!std::filesystem::is_directory(portduino_config.config_directory, error)) + findings.push_back({kError, merged, 0, + "General.ConfigDirectory '" + portduino_config.config_directory + + "' is not a directory that can be read, so none of the files that were meant to be " + "loaded from it are being loaded and meshtasticd stops at startup"}); + } + + // strncpy into a char[80] that is then hard null-terminated: safe, but silently + // shortened, and the operator never sees the message they actually configured. + if (portduino_config.has_statusMessage && portduino_config.statusMessage.size() > 79) + findings.push_back({kWarn, merged, 0, + "Config.StatusMessage is " + std::to_string(portduino_config.statusMessage.size()) + + " characters and is truncated to 79 when it is stored"}); + + // DIO3_TCXO_VOLTAGE is in VOLTS while every other Meshtastic surface uses millivolts, so + // the natural "1800" silently asks for 1800V and nothing downstream range-checks it. + if (portduino_config.dio3_tcxo_voltage > 3600) + findings.push_back({kError, merged, 0, + "Lora.DIO3_TCXO_VOLTAGE resolves to " + std::to_string(portduino_config.dio3_tcxo_voltage) + + " mV, which no TCXO runs at. The value is in VOLTS and is multiplied by 1000, so write " + "1.8 (or true) for a 1.8V part, not 1800"}); + + // loadConfig() only adopts APIPort inside this range and otherwise keeps the + // default without a word, so an out-of-range port silently does nothing. + if (portduino_config.api_port != -1 && (portduino_config.api_port <= 1023 || portduino_config.api_port >= 65536)) + findings.push_back({kWarn, merged, 0, + "General.APIPort " + std::to_string(portduino_config.api_port) + + " is outside 1024-65535, so it is ignored and the default port is used instead"}); + + if (portduino_config.webserverport != -1 && (portduino_config.webserverport <= 0 || portduino_config.webserverport >= 65536)) + findings.push_back({kError, merged, 0, + "Webserver.Port " + std::to_string(portduino_config.webserverport) + " is not a usable TCP port"}); + + if (portduino_config.MaxNodes <= 0) + findings.push_back({kError, merged, 0, + "General.MaxNodes is " + std::to_string(portduino_config.MaxNodes) + + ", which leaves no room for even this node's own entry"}); + +#if !defined(HAS_HUB75_NATIVE) + // A build-time gap rather than a config error: the same file is valid on a + // meshtasticd built with rpi-rgb-led-matrix present. + if (portduino_config.displayPanel == hub75) + findings.push_back({kError, merged, 0, + "Display.Panel is HUB75 but this meshtasticd was built without HUB75 support, so it exits at " + "startup. Rebuild with hzeller/rpi-rgb-led-matrix installed (it provides rgbmatrix.pc)"}); +#endif + + if (portduino_config.lora_cs_pin.enabled && !portduino_config.lora_spi_dev.empty() && + portduino_config.lora_spi_dev != "ch341") + findings.push_back({kInfo, merged, 0, + "both Lora.spidev (" + portduino_config.lora_spi_dev + + ") and Lora.CS are set. If your device tree already assigns a chip select to that spidev " + "node, meshtasticd will fail to claim the CS line at startup; if it does not, CS must be " + "set here. Check with 'gpioinfo'"}); +} + +void printSummary() +{ + std::cout << "\nEffective radio configuration:\n"; + std::cout << " Module : " << moduleName() << "\n"; + if (!portduino_config.lora_spi_dev.empty()) + std::cout << " spidev : " << portduino_config.lora_spi_dev << "\n"; + std::cout << " SPI speed : " << portduino_config.spiSpeed << "\n"; + if (portduino_config.dio3_tcxo_voltage) + std::cout << " DIO3 TCXO voltage : " << portduino_config.dio3_tcxo_voltage << " mV\n"; + + // setRfSwitchTable() is only called for an LR11xx, so "not set" is no gap elsewhere; "auto" + // has not resolved to a module yet, so absence cannot be called either way. + const char *rfSwitch = "not needed for this module"; + if (portduino_config.has_rfswitch_table) + rfSwitch = "set"; + else if (isLR11xx(portduino_config.lora_module)) + rfSwitch = "not set"; + else if (portduino_config.lora_module == use_autoconf) + rfSwitch = "not set (module not resolved yet)"; + std::cout << " RF switch table : " << rfSwitch << "\n"; + + // A ch341 adapter's Lora pins are adapter indexes handed to Ch341Hal, not gpiochip lines + // (portduinoSetup() skips initGPIOPin()), so pointing the user at gpioinfo would be wrong. + const bool usbAdapter = portduino_config.lora_spi_dev == "ch341"; + + std::cout << (usbAdapter ? "\nCH341 adapter pins (driven over USB, not claimed from a gpiochip):\n" + : "\nResolved GPIO lines (what meshtasticd will try to claim):\n"); + bool any = false; + for (const auto *pin : portduino_config.all_pins) { + if (!pin->enabled || pin->config_section != "Lora") + continue; + any = true; + std::cout << " " << pin->config_name; + for (size_t i = pin->config_name.size(); i < 18; i++) + std::cout << ' '; + std::cout << ": pin " << pin->pin; + if (!usbAdapter) + std::cout << " gpiochip" << pin->gpiochip << " line " << pin->line; + std::cout << "\n"; + } + if (!any) + std::cout << " (none configured)\n"; + if (usbAdapter) + std::cout << "\n These are pin indexes on the CH341 itself, so 'gpiodetect' and 'gpioinfo' say\n" + " nothing about them. Lora.gpiochip and any per-pin gpiochip/line mapping are\n" + " ignored for a ch341 device.\n"; + else + std::cout << "\n Confirm these against 'gpiodetect' and 'gpioinfo' on this machine. A line that\n" + " exists on the wrong chip is claimed successfully and silently does nothing.\n"; +} + +} // namespace + +int runConfigCheck(const std::vector &configFiles) +{ + std::vector findings; + + std::cout << "meshtasticd configuration check\n"; + std::cout << "===============================\n\n"; + + if (configFiles.empty()) { + std::cout << "No configuration files were found.\n"; + return 1; + } + + std::cout << "Configuration files, in load order (later files override earlier ones):\n"; + for (size_t i = 0; i < configFiles.size(); i++) + std::cout << " " << (i + 1) << ". " << configFiles[i] << "\n"; + if (configFiles.size() > 1) + std::cout << "\n Files in the config directory are read in whatever order the filesystem\n" + " returns them, which is not necessarily alphabetical and can differ between\n" + " machines. Avoid relying on one file overriding another.\n"; + + PathIndex paths; + std::map> sectionOwners; + for (const auto &file : configFiles) + checkFile(file, findings, paths, sectionOwners); + checkCrossFileOverlap(paths, sectionOwners, findings); + checkMergedConfig(paths, findings); + + int errors = 0, warnings = 0; + std::string currentFile; + for (const auto &finding : findings) { + if (finding.level == kError) + errors++; + else if (finding.level == kWarn) + warnings++; + + if (finding.file != currentFile) { + currentFile = finding.file; + std::cout << "\n" << currentFile << "\n"; + } + std::cout << " " << levelName(finding.level) << ' '; + if (finding.line) + std::cout << "line " << finding.line << ": "; + std::cout << finding.message << "\n"; + } + + printSummary(); + + std::cout << "\nResult: " << errors << (errors == 1 ? " error, " : " errors, ") << warnings + << (warnings == 1 ? " warning" : " warnings") << "\n"; + if (errors == 0 && warnings == 0) + std::cout << "Configuration looks good.\n"; + + return errors ? 1 : 0; +} + +#endif // !ARCH_PORTDUINO_WASM diff --git a/src/platform/portduino/ConfigCheck.h b/src/platform/portduino/ConfigCheck.h new file mode 100644 index 000000000..77aeb53dc --- /dev/null +++ b/src/platform/portduino/ConfigCheck.h @@ -0,0 +1,12 @@ +#pragma once + +#ifndef ARCH_PORTDUINO_WASM + +#include +#include + +// Validates the loaded meshtasticd YAML, prints a report, and returns an exit code: 0 when +// nothing fatal was found, 1 otherwise. configFiles lists every attempted file in load order. +int runConfigCheck(const std::vector &configFiles); + +#endif // !ARCH_PORTDUINO_WASM diff --git a/src/platform/portduino/PortduinoGlue.cpp b/src/platform/portduino/PortduinoGlue.cpp index dd0f663f0..bc714fcbe 100644 --- a/src/platform/portduino/PortduinoGlue.cpp +++ b/src/platform/portduino/PortduinoGlue.cpp @@ -6,6 +6,7 @@ #include "sleep.h" #include "target_specific.h" +#include "ConfigCheck.h" #include "PortduinoGlue.h" #include "SHA256.h" #include "api/ServerAPI.h" @@ -65,6 +66,9 @@ char *configPath = nullptr; char *optionMac = nullptr; bool verboseEnabled = false; bool yamlOnly = false; +bool configCheck = false; +// Every config file we attempted to load, in load order, for --check to report on. +std::vector attemptedConfigFiles; const char *argp_program_version = optstr(APP_VERSION); @@ -86,9 +90,16 @@ void updateBatteryLevel(uint8_t level) NOT_IMPLEMENTED("updateBatteryLevel"); int TCPPort = SERVER_API_DEFAULT_PORT; bool checkConfigPort = true; +// Long-only option: argp treats any key above the printable ASCII range as having no +// single-character equivalent. +#define OPT_CONFIG_CHECK 1001 + static error_t parse_opt(int key, char *arg, struct argp_state *state) { switch (key) { + case OPT_CONFIG_CHECK: + configCheck = true; + break; case 'p': if (sscanf(arg, "%d", &TCPPort) < 1) { return ARGP_ERR_UNKNOWN; @@ -160,13 +171,15 @@ static void checkSpidevBufsiz() void portduinoCustomInit() { - static struct argp_option options[] = {{"port", 'p', "PORT", 0, "The TCP port to use."}, - {"config", 'c', "CONFIG_PATH", 0, "Full path of the .yaml config file to use."}, - {"hwid", 'h', "HWID", 0, "The mac address to assign to this virtual machine"}, - {"sim", 's', 0, 0, "Run in Simulated radio mode"}, - {"verbose", 'v', 0, 0, "Set log level to full debug"}, - {"output-yaml", 'y', 0, 0, "Output config yaml and exit"}, - {0}}; + static struct argp_option options[] = { + {"port", 'p', "PORT", 0, "The TCP port to use."}, + {"config", 'c', "CONFIG_PATH", 0, "Full path of the .yaml config file to use."}, + {"hwid", 'h', "HWID", 0, "The mac address to assign to this virtual machine"}, + {"sim", 's', 0, 0, "Run in Simulated radio mode"}, + {"verbose", 'v', 0, 0, "Set log level to full debug"}, + {"output-yaml", 'y', 0, 0, "Output config yaml and exit"}, + {"check", OPT_CONFIG_CHECK, 0, 0, "Check the configuration for problems, print a report, and exit"}, + {0}}; static void *childArguments; static char doc[] = "Meshtastic native build."; static char args_doc[] = "..."; @@ -310,40 +323,54 @@ void portduinoSetup() portduino_config.lora_module = use_simradio; } else if (configPath != nullptr) { if (loadConfig(configPath)) { - if (!yamlOnly) + if (!yamlOnly && !configCheck) std::cout << "Using " << configPath << " as config file" << std::endl; - } else { + } else if (!configCheck) { + // In check mode the path is already in attemptedConfigFiles, so fall through + // to runConfigCheck() and let it report the parse error with a file and line. std::cout << "Unable to use " << configPath << " as config file" << std::endl; exit(EXIT_FAILURE); } } else if (access("config.yaml", R_OK) == 0) { if (loadConfig("config.yaml")) { - if (!yamlOnly) + if (!yamlOnly && !configCheck) std::cout << "Using local config.yaml as config file" << std::endl; - } else { + } else if (!configCheck) { std::cout << "Unable to use local config.yaml as config file" << std::endl; exit(EXIT_FAILURE); } } else if (access("/etc/meshtasticd/config.yaml", R_OK) == 0) { if (loadConfig("/etc/meshtasticd/config.yaml")) { - if (!yamlOnly) + if (!yamlOnly && !configCheck) std::cout << "Using /etc/meshtasticd/config.yaml as config file" << std::endl; - } else { + } else if (!configCheck) { std::cout << "Unable to use /etc/meshtasticd/config.yaml as config file" << std::endl; exit(EXIT_FAILURE); } } else { - if (!yamlOnly) + if (!yamlOnly && !configCheck) std::cout << "No 'config.yaml' found..." << std::endl; portduino_config.lora_module = use_simradio; } if (portduino_config.config_directory != "") { - std::string filetype = ".yaml"; - for (const std::filesystem::directory_entry &entry : - std::filesystem::directory_iterator{portduino_config.config_directory}) { + // The throwing form of directory_iterator turns an unreadable ConfigDirectory into an + // uncaught filesystem_error and a SIGABRT, so take the error_code overload instead. + std::error_code dirError; + std::filesystem::directory_iterator entries{portduino_config.config_directory, dirError}; + if (dirError) { + // Half a configuration is worse than none. --check continues so the report can say + // so with the rest of the findings. + if (!configCheck) { + std::cout << "Unable to read ConfigDirectory " << portduino_config.config_directory << ": " << dirError.message() + << std::endl; + exit(EXIT_FAILURE); + } + } + for (const std::filesystem::directory_entry &entry : entries) { if (ends_with(entry.path().string(), ".yaml")) { - std::cout << "Also using " << entry << " as additional config file" << std::endl; + if (!configCheck) + std::cout << "Also using " << entry << " as additional config file" << std::endl; // .string() rather than .c_str(): path::value_type is wchar_t on // Windows, and loadConfig() takes a const char *. loadConfig(entry.path().string().c_str()); @@ -352,6 +379,11 @@ void portduinoSetup() } #ifndef ARCH_PORTDUINO_WASM + // --check wins over --output-yaml: asking for validation and getting a config dump + // with no report at all would be the more surprising of the two outcomes. + if (configCheck) + exit(runConfigCheck(attemptedConfigFiles)); + if (yamlOnly) { std::cout << portduino_config.emit_yaml() << std::endl; exit(EXIT_SUCCESS); @@ -828,6 +860,10 @@ bool loadConfig(const char *configPath) #else bool loadConfig(const char *configPath) { + // Recorded even when the load below fails: an unparseable config.d entry is skipped and its + // return value discarded by the caller, so --check needs to know it was attempted. + attemptedConfigFiles.push_back(configPath); + YAML::Node yamlConfig; try { yamlConfig = YAML::LoadFile(configPath); @@ -886,7 +922,9 @@ bool loadConfig(const char *configPath) break; } } - if (!found) { + if (!found && !configCheck) { + // --check names the valid modules in its report; exiting here would + // replace that with a bare one-liner. std::cerr << "Unknown Lora.Module: " << moduleName << std::endl; exit(EXIT_FAILURE); } @@ -1081,7 +1119,9 @@ bool loadConfig(const char *configPath) } } #if !defined(HAS_HUB75_NATIVE) - if (portduino_config.displayPanel == hub75) { + if (portduino_config.displayPanel == hub75 && !configCheck) { + // --check still validates the rest of the file and reports this as a + // finding, so it must not exit from inside the load. std::cerr << "HUB75 display panel selected, but this build does not support HUB75" << std::endl; exit(EXIT_FAILURE); } @@ -1225,8 +1265,12 @@ bool loadConfig(const char *configPath) (yamlConfig["General"]["AvailableDirectory"]).as("/etc/meshtasticd/available.d/"); if ((yamlConfig["General"]["MACAddress"]).as("") != "" && (yamlConfig["General"]["MACAddressSource"]).as("") != "") { - std::cout << "Cannot set both MACAddress and MACAddressSource!" << std::endl; - exit(EXIT_FAILURE); + // --check reports this as a finding against the file it came from, so + // exiting here would kill the report before it is printed. + if (!configCheck) { + std::cout << "Cannot set both MACAddress and MACAddressSource!" << std::endl; + exit(EXIT_FAILURE); + } } if (checkConfigPort) { portduino_config.api_port = (yamlConfig["General"]["APIPort"]).as(-1); @@ -1249,7 +1293,10 @@ bool loadConfig(const char *configPath) portduino_config.mac_address.end()); } } catch (YAML::Exception &e) { - std::cout << "*** Exception " << e.what() << std::endl; + // The check report repeats this against the file it came from, so printing it + // here too would only put a stray line above the report. + if (!configCheck) + std::cout << "*** Exception " << e.what() << std::endl; return false; } return true; diff --git a/test/README.md b/test/README.md index 68b271f1e..4961e36d7 100644 --- a/test/README.md +++ b/test/README.md @@ -386,6 +386,20 @@ A well-structured test suite follows this pattern: 4. **Lifecycle tests** - state persistence, startup from blank, restart recovery 5. **Summary test** (optional) - emits a scenario table into the log for quick CI review +## Not a Unity suite: `bin/test-config-check.sh` + +Portduino YAML validation is tested by driving a built `meshtasticd` rather than by a +Unity suite, because what it asserts - the exit status and printed report of +`meshtasticd --check`, and the fact that a normal run still refuses a bad config - are +properties of the process, not of a linkable function. Fixtures live in +`test/fixtures/portduino-config/` (see the README there); CI runs it in +`test_native.yml`. It is not counted in `native-suite-count`, which only tracks `test_*` +directories. + +```bash +pio run -e native && ./bin/test-config-check.sh +``` + ## Existing Test Suites | Suite | Module Under Test | diff --git a/test/fixtures/portduino-config/README.md b/test/fixtures/portduino-config/README.md new file mode 100644 index 000000000..3768f55b4 --- /dev/null +++ b/test/fixtures/portduino-config/README.md @@ -0,0 +1,158 @@ +# Portduino config fixtures + +Input files for `bin/test-config-check.sh`, which drives a built `meshtasticd` +binary and asserts what `meshtasticd --check` reports about each one. Every file +here is referenced by name from that script, so renaming one means editing it too. + +The theme is configuration that is accepted by the YAML parser but does not mean +what it looks like it means - the failures that otherwise only show up as a radio +that never transmits. + +Each file carries a comment header naming its planted fault and what the checker is +expected to say about it, so a fixture can be read on its own. One assertion, +`malformed-indent.yaml`'s reported line number, counts those header lines - editing +that file's comments means updating the expected line in the script. + +These files are exempt from `trunk fmt` (see `.trunk/trunk.yaml`): prettier rejects +the duplicate keys and bad indentation that are the entire point of them. + +## Valid, one per radio module family + +These must each report zero errors and zero warnings, and must resolve to the +module named in the file. A silent fallback to `sim` would otherwise pass. + +| File | Module | Family | +| -------------------- | -------- | ---------- | +| `module-rf95.yaml` | `RF95` | SX127x | +| `module-sx1262.yaml` | `sx1262` | SX126x | +| `module-sx1268.yaml` | `sx1268` | SX126x | +| `module-llcc68.yaml` | `LLCC68` | SX126x | +| `module-sx1280.yaml` | `sx1280` | SX128x | +| `module-lr1110.yaml` | `lr1110` | LR11xx | +| `module-lr1120.yaml` | `lr1120` | LR11xx | +| `module-lr1121.yaml` | `lr1121` | LR11xx | +| `module-sim.yaml` | `sim` | simulated | +| `module-auto.yaml` | `auto` | autodetect | + +`valid.yaml` is a minimal SX126x config, and `empty-sections.yaml` (`Lora:` with +no body) exists to prove the checker does not invent a finding for it. + +## Module naming + +Names are matched exactly and are not consistently cased - `RF95` and `LLCC68` +are upper, `sx1262` and `lr1121` lower. + +| File | Expected | +| ------------------------ | --------------------------------------------------- | +| `module-unknown.yaml` | `sx1263` is refused, and the valid set is listed. | +| `module-wrong-case.yaml` | `llcc68` is refused with a "did you mean" for case. | + +## LR11xx rfswitch table + +| File | Expected | +| ------------------------------ | ----------------------------------------------------------------- | +| `rfswitch-valid.yaml` | A full seven-mode table on an `lr1121` is clean. | +| `rfswitch-partial.yaml` | Legal, but the omitted modes are named - they are driven all-LOW. | +| `rfswitch-bad-pin.yaml` | `DIO9` is not one of DIO5/6/7/8/10. | +| `rfswitch-row-length.yaml` | Rows shorter and longer than the declared pin count. | +| `rfswitch-bad-level.yaml` | `high` and `On`: anything not exactly `HIGH` is silently LOW. | +| `rfswitch-no-pins.yaml` | No `pins` list, so no switch pin is ever driven. | +| `rfswitch-too-many-pins.yaml` | Six pins declared; only the first five are read. | +| `rfswitch-not-a-map.yaml` | `rfswitch_table` given a scalar. | +| `rfswitch-unknown-mode.yaml` | `MODE_TRANSMIT` is not a mode. | +| `rfswitch-stranded-modes.yaml` | A `MODE_` row one level out, sitting under `Lora:` doing nothing. | + +`rfswitch-partial.yaml` is legal but noted: the omitted modes are driven all-LOW. +`module-mismatch-lr11xx.yaml` (LR11xx with no table - cannot transmit) and +`module-mismatch-sx126x.yaml` (a table on a radio that never applies one) cover +the module/table disagreement in both directions. + +## PA gain table (`TX_GAIN_LORA`) + +Two shapes are accepted and they fail differently. A list is read element-by-element +with `.as()` and NO fallback, so one bad entry throws and meshtasticd will not +start. A bare scalar is read as `.as(0)` and merely falls back to 0. The table +is `uint16_t[22]`, so extra points are dropped and out-of-range values wrap. + +| File | Expected | +| ---------------------------- | ------------------------------------------------------------------------- | +| `txgain-scalar.yaml` | **Clean, and a regression guard** - an earlier checker called this fatal. | +| `value-type-fatal-list.yaml` | A non-numeric list entry: throws, so meshtasticd will not start. | +| `txgain-out-of-range.yaml` | `-5` and `70000` wrap to a different gain than written. | +| `txgain-too-many.yaml` | 25 points; everything past the 22nd is dropped. | + +## Value types, ranges and units + +| File | Expected | +| ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `value-type-fatal.yaml` | `Logging.AsciiLogs` is the other no-fallback read: a bad value stops meshtasticd starting. | +| `value-type-silent.yaml` | Wrong-typed values where the read has a default: silently replaced, so the setting does nothing. | +| `tcxo-millivolts.yaml` | `DIO3_TCXO_VOLTAGE` is in VOLTS and multiplied by 1000, so `1800` silently asks for 1800V. Write `1.8`. | +| `port-out-of-range.yaml` | `APIPort` outside 1024-65535 is silently ignored; `Webserver.Port` has no guard at all. | +| `statusmessage-long.yaml` | Copied into a `char[80]`, so it is safe but silently shortened to 79 characters. | +| `configdir-missing.yaml` | **Crash regression guard** - an unreadable `ConfigDirectory` used to abort meshtasticd (and `--check`) with SIGABRT via an uncaught `filesystem_error`. | + +## MAC address + +The MAC no longer determines NodeNum - that comes from the public key - but a MAC +that fails to apply still falls through to the BlueZ and LoRa-serial fallbacks, and +if those yield nothing meshtasticd exits with "Blank MAC Address not allowed!". + +| File | Expected | +| ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------- | +| `mac-conflict.yaml` | Both `MACAddress` and `MACAddressSource`; meshtasticd refuses. | +| `mac-malformed.yaml` | `AA:BB:CC` is under 12 hex digits, so it is silently dropped. | +| `mac-source-missing.yaml` | Names an interface with no `/sys/class/net//address`. Warning, not an error: it is machine-dependent and may be checked on another host. | + +## CH341 USB-SPI adapters + +`spidev: ch341` is a different hardware model, not a variant of the same one. The Lora +pins become indexes on the adapter and are driven by the usermode USB driver - +`portduinoSetup()` skips `initGPIOPin()` for every one of them - so nothing is claimed +from a gpiochip. This is also the only shape that works on Windows and macOS, which have +no gpiochip, `gpiodetect` or `gpioinfo` to check anything against. + +| File | Expected | +| --------------------- | ----------------------------------------------------------------------------- | +| `usb-ch341.yaml` | Clean, and the report lists adapter pins rather than resolved gpiochip lines. | +| `ch341-gpiochip.yaml` | A gpiochip and line mapping alongside `ch341`: read, stored, and never used. | + +## Structure + +| File | Expected | +| ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `duplicate-key.yaml` | yaml-cpp keeps the FIRST duplicate, so the later value is lost. | +| `nonmap-section.yaml` | `Lora: invalid` - a known section whose body cannot be read. | +| `unknown-section.yaml` | A top-level section meshtasticd never reads. | +| `stranded-key.yaml` | `spidev` left at the top level instead of inside a section. | +| `top-level-list.yaml` | Document root is a sequence. | +| `empty-file.yaml` | No content: comments only, which parse to a null document. A warning, not an error. | +| `malformed-indent.yaml` | Will not parse; the report must still name the file and line. | +| `pin-unknown-subkey.yaml` | A pin mapping accepts only `pin`, `gpiochip` and `line`. | +| `pin-unreadable.yaml` | A non-numeric pin resolves to -1 and trips an assertion at startup. | +| `hub75-unknown-key.yaml` | An unknown `Display.HUB75` option. On a build without rgbmatrix this also reports the missing HUB75 support, so the test asserts only the unknown key. | + +## Across a config directory + +`configd-conflict/` is a whole tree: a `config.yaml` pointing at a `config.d/` +holding two more `Lora:` sections. It covers the trap that a key not repeated in +the last-loaded file is reset to its default - here `config.yaml` sets +`DIO3_TCXO_VOLTAGE: 1800` and the effective configuration ends up without it. +The load order within `config.d/` comes from the filesystem, so the report warns +rather than assuming alphabetical order. + +`rfswitch-sticky/` covers the one place where "the file loaded last wins" is false, +and it documents a firmware bug rather than a configuration mistake. Its `config.d/` +holds two switch tables; the last one loaded sets `MODE_RX` LOW on both pins, but the +loader only ever writes HIGH and never writes LOW back, so the HIGH from the earlier +file survives and the effective table is the OR of both. Verified with +`meshtasticd --output-yaml`. Until the loader is fixed, `--check` reports this as an +error and tells you to enable exactly one. + +## Running these as a normal boot + +`malformed-indent.yaml`, `nonmap-section.yaml`, `module-unknown.yaml`, +`mac-conflict.yaml` and `hub75-unknown-key.yaml` are also run _without_ `--check`, +where each must be rejected with a non-zero exit. That is the guard on `--check` +mode not having quietly made the normal path permissive. No other fixture is run +that way: a config meshtasticd accepts makes it boot a node and block. diff --git a/test/fixtures/portduino-config/ch341-gpiochip.yaml b/test/fixtures/portduino-config/ch341-gpiochip.yaml new file mode 100644 index 000000000..f647ee714 --- /dev/null +++ b/test/fixtures/portduino-config/ch341-gpiochip.yaml @@ -0,0 +1,16 @@ +# FAULT: a CH341 config that also maps its pins onto a gpiochip. Every Lora pin is +# skipped by the GPIO claim loop when spidev is ch341, so the chip and line numbers below +# are read, stored, and never used - while reading as though they were authoritative. +Lora: + Module: sx1262 + spidev: ch341 + gpiochip: 4 + CS: + pin: 0 + gpiochip: 4 + line: 21 + IRQ: 6 + Reset: 2 + Busy: 4 + USB_PID: 0x5512 + USB_VID: 0x1A86 diff --git a/test/fixtures/portduino-config/configd-conflict/config.d/lora-a.yaml b/test/fixtures/portduino-config/configd-conflict/config.d/lora-a.yaml new file mode 100644 index 000000000..d0e548ae4 --- /dev/null +++ b/test/fixtures/portduino-config/configd-conflict/config.d/lora-a.yaml @@ -0,0 +1,6 @@ +# Second of three files. Re-opens "Lora:", which is what resets the keys set in +# ../config.yaml that are not repeated here. +Lora: + Module: sx1262 + CS: 21 + IRQ: 16 diff --git a/test/fixtures/portduino-config/configd-conflict/config.d/lora-b.yaml b/test/fixtures/portduino-config/configd-conflict/config.d/lora-b.yaml new file mode 100644 index 000000000..0377d2a83 --- /dev/null +++ b/test/fixtures/portduino-config/configd-conflict/config.d/lora-b.yaml @@ -0,0 +1,8 @@ +# Third of three files, and the one that wins -- but only because the filesystem +# happens to return it last. config.d/ load order is not guaranteed to be +# alphabetical and can differ between machines, which is why the report warns +# rather than assuming an order. +Lora: + Module: lr1121 + CS: 99 + IRQ: 16 diff --git a/test/fixtures/portduino-config/configd-conflict/config.yaml b/test/fixtures/portduino-config/configd-conflict/config.yaml new file mode 100644 index 000000000..b5a8b91d3 --- /dev/null +++ b/test/fixtures/portduino-config/configd-conflict/config.yaml @@ -0,0 +1,12 @@ +# The primary config of a three-file set (see config.d/ alongside it). +# FAULT, and it is invisible in this file alone: two more files in config.d/ each open +# their own "Lora:" section, and the keys below that they do not repeat -- notably +# DIO3_TCXO_VOLTAGE -- are reset to their defaults when the last of them is loaded. +# The effective configuration ends up with no TCXO voltage at all. +General: + ConfigDirectory: config.d/ +Lora: + Module: sx1262 + spidev: spidev0.0 + DIO3_TCXO_VOLTAGE: 1800 + CS: 21 diff --git a/test/fixtures/portduino-config/configdir-missing.yaml b/test/fixtures/portduino-config/configdir-missing.yaml new file mode 100644 index 000000000..17eaaaf22 --- /dev/null +++ b/test/fixtures/portduino-config/configdir-missing.yaml @@ -0,0 +1,8 @@ +# FAULT, and it used to be a hard crash: a ConfigDirectory that cannot be read made +# std::filesystem::directory_iterator throw an uncaught filesystem_error, aborting +# meshtasticd with SIGABRT -- and aborting --check along with it, so the one tool meant +# to diagnose the problem died on it. It now fails cleanly and is reported here. +Lora: + Module: sx1262 +General: + ConfigDirectory: /nonexistent/does-not-exist/ diff --git a/test/fixtures/portduino-config/duplicate-key.yaml b/test/fixtures/portduino-config/duplicate-key.yaml new file mode 100644 index 000000000..ec5bfbfe4 --- /dev/null +++ b/test/fixtures/portduino-config/duplicate-key.yaml @@ -0,0 +1,8 @@ +# FAULT: Lora.CS is defined twice. YAML forbids this but yaml-cpp does not complain: +# it keeps the FIRST occurrence and silently discards the rest, so the CS: 22 below +# looks applied and is not. Expect an ERROR naming both lines. +Lora: + Module: sx1262 + CS: 21 + CS: 22 + IRQ: 16 diff --git a/test/fixtures/portduino-config/empty-file.yaml b/test/fixtures/portduino-config/empty-file.yaml new file mode 100644 index 000000000..158da87e2 --- /dev/null +++ b/test/fixtures/portduino-config/empty-file.yaml @@ -0,0 +1,2 @@ +# FAULT: a zero-content file. Only comments here, which yaml-cpp parses to a null +# document -- the same as truly empty. Expect a WARNING, not an ERROR. diff --git a/test/fixtures/portduino-config/empty-sections.yaml b/test/fixtures/portduino-config/empty-sections.yaml new file mode 100644 index 000000000..fb374ce70 --- /dev/null +++ b/test/fixtures/portduino-config/empty-sections.yaml @@ -0,0 +1,5 @@ +# CLEAN, and deliberately so. A section with no body is a null node, not a map. +# loadConfig() tests the section node before reading it, so this is harmless -- the +# checker must not report it. Regression guard for the null-vs-non-map distinction. +Lora: +Display: diff --git a/test/fixtures/portduino-config/hub75-unknown-key.yaml b/test/fixtures/portduino-config/hub75-unknown-key.yaml new file mode 100644 index 000000000..fc5e9e474 --- /dev/null +++ b/test/fixtures/portduino-config/hub75-unknown-key.yaml @@ -0,0 +1,11 @@ +# FAULT: Colours is not a HUB75 option (the real ones are listed in ConfigCheck.cpp). +# On a meshtasticd built without rgbmatrix this also reports the missing HUB75 +# support, so the test asserts only the unknown key -- the second finding depends on +# how the binary was built. +Lora: + Module: sx1262 +Display: + Panel: HUB75 + HUB75: + Rows: 64 + Colours: 3 diff --git a/test/fixtures/portduino-config/mac-conflict.yaml b/test/fixtures/portduino-config/mac-conflict.yaml new file mode 100644 index 000000000..ed4431cb2 --- /dev/null +++ b/test/fixtures/portduino-config/mac-conflict.yaml @@ -0,0 +1,9 @@ +# FAULT: MACAddress and MACAddressSource are mutually exclusive; meshtasticd exits +# rather than pick one. That exit is inside loadConfig(), so --check has to report it +# without letting the exit kill the report. Also run without --check, where +# meshtasticd must refuse to start. +General: + MACAddress: AA:BB:CC:DD:EE:FF + MACAddressSource: eth0 +Lora: + Module: sx1262 diff --git a/test/fixtures/portduino-config/mac-malformed.yaml b/test/fixtures/portduino-config/mac-malformed.yaml new file mode 100644 index 000000000..3311fa7c0 --- /dev/null +++ b/test/fixtures/portduino-config/mac-malformed.yaml @@ -0,0 +1,9 @@ +# FAULT: AA:BB:CC is three octets, not six. loadConfig() strips the colons and then +# requires more than 11 characters, so this is dropped without a word and the MAC +# comes from the Bluetooth adapter or the LoRa serial instead. NodeNum is unaffected +# (that comes from the public key), but if no fallback yields a MAC, meshtasticd +# exits with "Blank MAC Address not allowed!". +General: + MACAddress: AA:BB:CC +Lora: + Module: sx1262 diff --git a/test/fixtures/portduino-config/mac-source-missing.yaml b/test/fixtures/portduino-config/mac-source-missing.yaml new file mode 100644 index 000000000..e7fc527c7 --- /dev/null +++ b/test/fixtures/portduino-config/mac-source-missing.yaml @@ -0,0 +1,7 @@ +# FAULT: names a network interface that does not exist, so /sys/class/net//address +# reads back empty and no MAC is set. A WARNING rather than an ERROR: which interfaces +# exist is a property of the machine, and a config is often checked on another host. +General: + MACAddressSource: nosuchiface99 +Lora: + Module: sx1262 diff --git a/test/fixtures/portduino-config/malformed-indent.yaml b/test/fixtures/portduino-config/malformed-indent.yaml new file mode 100644 index 000000000..03a1198d0 --- /dev/null +++ b/test/fixtures/portduino-config/malformed-indent.yaml @@ -0,0 +1,8 @@ +# FAULT: three-space indent on the CS line below, so the file will not parse at all. +# The point of this fixture is that --check still reaches its report and names the +# file and line, rather than exiting early with a bare "Unable to use ...". +# Also run without --check, where meshtasticd must refuse to start. +Lora: + Module: sx1262 + CS: 21 + IRQ: 16 diff --git a/test/fixtures/portduino-config/module-auto.yaml b/test/fixtures/portduino-config/module-auto.yaml new file mode 100644 index 000000000..eb5f51f1b --- /dev/null +++ b/test/fixtures/portduino-config/module-auto.yaml @@ -0,0 +1,3 @@ +# CLEAN. Autodetect: the module is probed at startup, so no pins are required here. +Lora: + Module: auto diff --git a/test/fixtures/portduino-config/module-llcc68.yaml b/test/fixtures/portduino-config/module-llcc68.yaml new file mode 100644 index 000000000..38dc23288 --- /dev/null +++ b/test/fixtures/portduino-config/module-llcc68.yaml @@ -0,0 +1,9 @@ +# CLEAN. Valid SX126x config -- must report no findings AND must resolve to this +# module in the effective configuration, so a silent fallback to sim cannot pass. +Lora: + Module: LLCC68 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 diff --git a/test/fixtures/portduino-config/module-lr1110.yaml b/test/fixtures/portduino-config/module-lr1110.yaml new file mode 100644 index 000000000..e3f5233fc --- /dev/null +++ b/test/fixtures/portduino-config/module-lr1110.yaml @@ -0,0 +1,18 @@ +# CLEAN. Valid LR11xx config -- must report no findings AND must resolve to this +# module in the effective configuration, so a silent fallback to sim cannot pass. +Lora: + Module: lr1110 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + rfswitch_table: + pins: [DIO5, DIO6, DIO7, DIO8] + MODE_STBY: [LOW, LOW, LOW, LOW] + MODE_RX: [HIGH, LOW, LOW, LOW] + MODE_TX: [HIGH, HIGH, LOW, LOW] + MODE_TX_HP: [LOW, HIGH, LOW, LOW] + MODE_TX_HF: [LOW, LOW, LOW, LOW] + MODE_GNSS: [LOW, LOW, HIGH, LOW] + MODE_WIFI: [LOW, LOW, LOW, HIGH] diff --git a/test/fixtures/portduino-config/module-lr1120.yaml b/test/fixtures/portduino-config/module-lr1120.yaml new file mode 100644 index 000000000..de6261184 --- /dev/null +++ b/test/fixtures/portduino-config/module-lr1120.yaml @@ -0,0 +1,18 @@ +# CLEAN. Valid LR11xx config -- must report no findings AND must resolve to this +# module in the effective configuration, so a silent fallback to sim cannot pass. +Lora: + Module: lr1120 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + rfswitch_table: + pins: [DIO5, DIO6, DIO7, DIO8] + MODE_STBY: [LOW, LOW, LOW, LOW] + MODE_RX: [HIGH, LOW, LOW, LOW] + MODE_TX: [HIGH, HIGH, LOW, LOW] + MODE_TX_HP: [LOW, HIGH, LOW, LOW] + MODE_TX_HF: [LOW, LOW, LOW, LOW] + MODE_GNSS: [LOW, LOW, HIGH, LOW] + MODE_WIFI: [LOW, LOW, LOW, HIGH] diff --git a/test/fixtures/portduino-config/module-lr1121.yaml b/test/fixtures/portduino-config/module-lr1121.yaml new file mode 100644 index 000000000..5beed4b59 --- /dev/null +++ b/test/fixtures/portduino-config/module-lr1121.yaml @@ -0,0 +1,18 @@ +# CLEAN. Valid LR11xx config -- must report no findings AND must resolve to this +# module in the effective configuration, so a silent fallback to sim cannot pass. +Lora: + Module: lr1121 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + rfswitch_table: + pins: [DIO5, DIO6, DIO7, DIO8] + MODE_STBY: [LOW, LOW, LOW, LOW] + MODE_RX: [HIGH, LOW, LOW, LOW] + MODE_TX: [HIGH, HIGH, LOW, LOW] + MODE_TX_HP: [LOW, HIGH, LOW, LOW] + MODE_TX_HF: [LOW, LOW, LOW, LOW] + MODE_GNSS: [LOW, LOW, HIGH, LOW] + MODE_WIFI: [LOW, LOW, LOW, HIGH] diff --git a/test/fixtures/portduino-config/module-mismatch-lr11xx.yaml b/test/fixtures/portduino-config/module-mismatch-lr11xx.yaml new file mode 100644 index 000000000..f2486297d --- /dev/null +++ b/test/fixtures/portduino-config/module-mismatch-lr11xx.yaml @@ -0,0 +1,7 @@ +# FAULT: an LR11xx with no rfswitch_table. setRfSwitchTable() is never called, and +# most LR11xx modules cannot transmit or receive without one. Expect a WARNING from +# the merged-configuration checks rather than from any single file. +Lora: + Module: lr1121 + CS: 21 + IRQ: 16 diff --git a/test/fixtures/portduino-config/module-mismatch-sx126x.yaml b/test/fixtures/portduino-config/module-mismatch-sx126x.yaml new file mode 100644 index 000000000..881afa827 --- /dev/null +++ b/test/fixtures/portduino-config/module-mismatch-sx126x.yaml @@ -0,0 +1,13 @@ +# FAULT: the opposite direction -- a switch table on a radio that never applies one. +# The table is only handed to LR11xx parts, so this one is silently inert. +Lora: + Module: sx1262 + rfswitch_table: + pins: [DIO5] + MODE_STBY: [LOW] + MODE_RX: [HIGH] + MODE_TX: [HIGH] + MODE_TX_HP: [HIGH] + MODE_TX_HF: [LOW] + MODE_GNSS: [LOW] + MODE_WIFI: [LOW] diff --git a/test/fixtures/portduino-config/module-rf95.yaml b/test/fixtures/portduino-config/module-rf95.yaml new file mode 100644 index 000000000..483ae7bc4 --- /dev/null +++ b/test/fixtures/portduino-config/module-rf95.yaml @@ -0,0 +1,10 @@ +# CLEAN. Valid SX127x config -- must report no findings AND must resolve to this +# module in the effective configuration, so a silent fallback to sim cannot pass. +Lora: + Module: RF95 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + RF95_MAX_POWER: 20 diff --git a/test/fixtures/portduino-config/module-sim.yaml b/test/fixtures/portduino-config/module-sim.yaml new file mode 100644 index 000000000..f4bc6be65 --- /dev/null +++ b/test/fixtures/portduino-config/module-sim.yaml @@ -0,0 +1,3 @@ +# CLEAN. The simulated radio takes no pins or SPI device at all. +Lora: + Module: sim diff --git a/test/fixtures/portduino-config/module-sx1262.yaml b/test/fixtures/portduino-config/module-sx1262.yaml new file mode 100644 index 000000000..9023f41a3 --- /dev/null +++ b/test/fixtures/portduino-config/module-sx1262.yaml @@ -0,0 +1,12 @@ +# CLEAN. Valid SX126x config -- must report no findings AND must resolve to this +# module in the effective configuration, so a silent fallback to sim cannot pass. +Lora: + Module: sx1262 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + DIO2_AS_RF_SWITCH: true + DIO3_TCXO_VOLTAGE: 1.8 + SX126X_MAX_POWER: 22 diff --git a/test/fixtures/portduino-config/module-sx1268.yaml b/test/fixtures/portduino-config/module-sx1268.yaml new file mode 100644 index 000000000..e4cb1651f --- /dev/null +++ b/test/fixtures/portduino-config/module-sx1268.yaml @@ -0,0 +1,9 @@ +# CLEAN. Valid SX126x config -- must report no findings AND must resolve to this +# module in the effective configuration, so a silent fallback to sim cannot pass. +Lora: + Module: sx1268 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 diff --git a/test/fixtures/portduino-config/module-sx1280.yaml b/test/fixtures/portduino-config/module-sx1280.yaml new file mode 100644 index 000000000..21d244a82 --- /dev/null +++ b/test/fixtures/portduino-config/module-sx1280.yaml @@ -0,0 +1,10 @@ +# CLEAN. Valid SX128x config -- must report no findings AND must resolve to this +# module in the effective configuration, so a silent fallback to sim cannot pass. +Lora: + Module: sx1280 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + SX128X_MAX_POWER: 13 diff --git a/test/fixtures/portduino-config/module-unknown.yaml b/test/fixtures/portduino-config/module-unknown.yaml new file mode 100644 index 000000000..b2026bc92 --- /dev/null +++ b/test/fixtures/portduino-config/module-unknown.yaml @@ -0,0 +1,4 @@ +# FAULT: sx1263 is not a module meshtasticd knows. loadConfig() exits on an unknown +# name, so --check has to report it instead and list the valid set. +Lora: + Module: sx1263 diff --git a/test/fixtures/portduino-config/module-wrong-case.yaml b/test/fixtures/portduino-config/module-wrong-case.yaml new file mode 100644 index 000000000..84b9bbafa --- /dev/null +++ b/test/fixtures/portduino-config/module-wrong-case.yaml @@ -0,0 +1,4 @@ +# FAULT: module names are matched EXACTLY and are not consistently cased -- the +# accepted spelling is LLCC68, not llcc68. Expect an ERROR with a "did you mean". +Lora: + Module: llcc68 diff --git a/test/fixtures/portduino-config/nonmap-section.yaml b/test/fixtures/portduino-config/nonmap-section.yaml new file mode 100644 index 000000000..aead4153d --- /dev/null +++ b/test/fixtures/portduino-config/nonmap-section.yaml @@ -0,0 +1,4 @@ +# FAULT: a known section whose body is a scalar. loadConfig() throws when it indexes +# into it ("operator[] call on a scalar"), so nothing in the file is read. Expect an +# ERROR. Also run without --check, where meshtasticd must refuse to start. +Lora: invalid diff --git a/test/fixtures/portduino-config/pin-unknown-subkey.yaml b/test/fixtures/portduino-config/pin-unknown-subkey.yaml new file mode 100644 index 000000000..1c5ffae96 --- /dev/null +++ b/test/fixtures/portduino-config/pin-unknown-subkey.yaml @@ -0,0 +1,8 @@ +# FAULT: chipline is not a pin-mapping sub-key -- only pin, gpiochip and line are +# read. The typo means the line number is never applied. Expect an ERROR. +Lora: + Module: sx1262 + CS: + pin: 21 + gpiochip: 0 + chipline: 4 diff --git a/test/fixtures/portduino-config/pin-unreadable.yaml b/test/fixtures/portduino-config/pin-unreadable.yaml new file mode 100644 index 000000000..4d87da5e2 --- /dev/null +++ b/test/fixtures/portduino-config/pin-unreadable.yaml @@ -0,0 +1,7 @@ +# FAULT: a pin value that is not a number. It is still marked enabled but resolves to +# -1 (RADIOLIB_NC), and initGPIOPin() then trips an assertion inside LinuxGPIOPin at +# startup. Expect an ERROR -- catching it here is the difference between a report and +# a stack trace from a library file. +Lora: + Module: sx1262 + CS: not-a-number diff --git a/test/fixtures/portduino-config/port-out-of-range.yaml b/test/fixtures/portduino-config/port-out-of-range.yaml new file mode 100644 index 000000000..db6914229 --- /dev/null +++ b/test/fixtures/portduino-config/port-out-of-range.yaml @@ -0,0 +1,9 @@ +# FAULT: loadConfig() only adopts APIPort when it is inside 1024-65535 and otherwise +# keeps the default without a word, so this is silently ignored. Webserver.Port has no +# such guard at all, so an unusable port there is an error rather than a warning. +Lora: + Module: sx1262 +General: + APIPort: 80 +Webserver: + Port: 99999 diff --git a/test/fixtures/portduino-config/rfswitch-bad-level.yaml b/test/fixtures/portduino-config/rfswitch-bad-level.yaml new file mode 100644 index 000000000..efcab98fa --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-bad-level.yaml @@ -0,0 +1,9 @@ +# FAULT: anything that is not the exact string "HIGH" is treated as LOW, so the +# lowercase "high" and the "On" below are silently switches that never close -- the +# quietest way to end up with a radio that cannot transmit. Expect an ERROR each. +Lora: + Module: lr1121 + rfswitch_table: + pins: [DIO5, DIO6] + MODE_STBY: [LOW, LOW] + MODE_RX: [high, On] diff --git a/test/fixtures/portduino-config/rfswitch-bad-pin.yaml b/test/fixtures/portduino-config/rfswitch-bad-pin.yaml new file mode 100644 index 000000000..110bbebe4 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-bad-pin.yaml @@ -0,0 +1,6 @@ +# FAULT: DIO9 is not a switch pin. Only DIO5, DIO6, DIO7, DIO8 and DIO10 exist. +Lora: + Module: lr1121 + rfswitch_table: + pins: [DIO5, DIO9, DIO7] + MODE_STBY: [LOW, LOW, LOW] diff --git a/test/fixtures/portduino-config/rfswitch-no-pins.yaml b/test/fixtures/portduino-config/rfswitch-no-pins.yaml new file mode 100644 index 000000000..fd6805ac9 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-no-pins.yaml @@ -0,0 +1,6 @@ +# FAULT: no pins list, so no switch pin is ever driven and the mode rows below have +# nothing to apply to. +Lora: + Module: lr1121 + rfswitch_table: + MODE_STBY: [LOW, LOW] diff --git a/test/fixtures/portduino-config/rfswitch-not-a-map.yaml b/test/fixtures/portduino-config/rfswitch-not-a-map.yaml new file mode 100644 index 000000000..c0bb8c454 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-not-a-map.yaml @@ -0,0 +1,4 @@ +# FAULT: rfswitch_table given a scalar instead of a mapping. +Lora: + Module: lr1121 + rfswitch_table: DIO5 diff --git a/test/fixtures/portduino-config/rfswitch-partial.yaml b/test/fixtures/portduino-config/rfswitch-partial.yaml new file mode 100644 index 000000000..244410bc7 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-partial.yaml @@ -0,0 +1,12 @@ +# CLEAN but NOTED. A partial table is legal. The modes left out (MODE_TX_HP, +# MODE_TX_HF, MODE_GNSS, MODE_WIFI) are driven all-LOW, which on most modules is the +# shutdown state -- so the report names them rather than leaving it to be found on air. +Lora: + Module: lr1121 + CS: 21 + IRQ: 16 + rfswitch_table: + pins: [DIO5, DIO6] + MODE_STBY: [LOW, LOW] + MODE_RX: [HIGH, LOW] + MODE_TX: [HIGH, HIGH] diff --git a/test/fixtures/portduino-config/rfswitch-row-length.yaml b/test/fixtures/portduino-config/rfswitch-row-length.yaml new file mode 100644 index 000000000..a56c1007b --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-row-length.yaml @@ -0,0 +1,8 @@ +# FAULT: three pins are declared, but MODE_STBY gives two values and MODE_RX gives +# four. Expect one ERROR per mismatched row. +Lora: + Module: lr1121 + rfswitch_table: + pins: [DIO5, DIO6, DIO7] + MODE_STBY: [LOW, LOW] + MODE_RX: [HIGH, LOW, LOW, LOW] diff --git a/test/fixtures/portduino-config/rfswitch-sticky/config.d/a-first.yaml b/test/fixtures/portduino-config/rfswitch-sticky/config.d/a-first.yaml new file mode 100644 index 000000000..2c60dfecc --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-sticky/config.d/a-first.yaml @@ -0,0 +1,6 @@ +# Loaded first. Sets MODE_RX HIGH on both pins. +Lora: + Module: lr1121 + rfswitch_table: + pins: [DIO5, DIO6] + MODE_RX: [HIGH, HIGH] diff --git a/test/fixtures/portduino-config/rfswitch-sticky/config.d/b-second.yaml b/test/fixtures/portduino-config/rfswitch-sticky/config.d/b-second.yaml new file mode 100644 index 000000000..1832c7625 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-sticky/config.d/b-second.yaml @@ -0,0 +1,9 @@ +# FAULT, and it is a firmware bug rather than a typo. This file is loaded LAST and says +# MODE_RX is LOW on both pins, but the loader only ever writes HIGH and never writes +# LOW back, so the HIGH from a-first.yaml survives. The effective table is the OR of +# both files -- a switch state that neither file asked for. Verified with --output-yaml. +Lora: + Module: lr1121 + rfswitch_table: + pins: [DIO5, DIO6] + MODE_RX: [LOW, LOW] diff --git a/test/fixtures/portduino-config/rfswitch-sticky/config.yaml b/test/fixtures/portduino-config/rfswitch-sticky/config.yaml new file mode 100644 index 000000000..ccc3ab988 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-sticky/config.yaml @@ -0,0 +1,4 @@ +# Primary config for the sticky-switch-table case. The two files in config.d/ each +# define a Lora.rfswitch_table. +General: + ConfigDirectory: config.d/ diff --git a/test/fixtures/portduino-config/rfswitch-stranded-modes.yaml b/test/fixtures/portduino-config/rfswitch-stranded-modes.yaml new file mode 100644 index 000000000..dcfeeb898 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-stranded-modes.yaml @@ -0,0 +1,9 @@ +# FAULT: MODE_RX below is indented one level short, so it sits under Lora: rather than +# inside rfswitch_table and does nothing at all. Expect a WARNING that says it is a +# valid key of Lora.rfswitch_table -- the hint is what makes this findable. +Lora: + Module: lr1121 + rfswitch_table: + pins: [DIO5] + MODE_STBY: [LOW] + MODE_RX: [HIGH] diff --git a/test/fixtures/portduino-config/rfswitch-too-many-pins.yaml b/test/fixtures/portduino-config/rfswitch-too-many-pins.yaml new file mode 100644 index 000000000..9a69e1977 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-too-many-pins.yaml @@ -0,0 +1,7 @@ +# FAULT: six pins declared but only the first five are read (rfswitch_dio_pins[5]), +# so the last one is silently dropped. +Lora: + Module: lr1121 + rfswitch_table: + pins: [DIO5, DIO6, DIO7, DIO8, DIO10, DIO5] + MODE_STBY: [LOW, LOW, LOW, LOW, LOW, LOW] diff --git a/test/fixtures/portduino-config/rfswitch-unknown-mode.yaml b/test/fixtures/portduino-config/rfswitch-unknown-mode.yaml new file mode 100644 index 000000000..5bb603704 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-unknown-mode.yaml @@ -0,0 +1,8 @@ +# FAULT: MODE_TRANSMIT is not a mode. The real ones are MODE_STBY, MODE_RX, MODE_TX, +# MODE_TX_HP, MODE_TX_HF, MODE_GNSS and MODE_WIFI. +Lora: + Module: lr1121 + rfswitch_table: + pins: [DIO5] + MODE_STBY: [LOW] + MODE_TRANSMIT: [HIGH] diff --git a/test/fixtures/portduino-config/rfswitch-valid.yaml b/test/fixtures/portduino-config/rfswitch-valid.yaml new file mode 100644 index 000000000..fad8c34f0 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-valid.yaml @@ -0,0 +1,17 @@ +# CLEAN. A full seven-mode table on an LR11xx: every mode present, every row the same +# length as the pins list, every value exactly HIGH or LOW. +Lora: + Module: lr1121 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + rfswitch_table: + pins: [DIO5, DIO6, DIO7, DIO8] + MODE_STBY: [LOW, LOW, LOW, LOW] + MODE_RX: [HIGH, LOW, LOW, LOW] + MODE_TX: [HIGH, HIGH, LOW, LOW] + MODE_TX_HP: [LOW, HIGH, LOW, LOW] + MODE_TX_HF: [LOW, LOW, LOW, LOW] + MODE_GNSS: [LOW, LOW, HIGH, LOW] + MODE_WIFI: [LOW, LOW, LOW, HIGH] diff --git a/test/fixtures/portduino-config/statusmessage-long.yaml b/test/fixtures/portduino-config/statusmessage-long.yaml new file mode 100644 index 000000000..8517c6ceb --- /dev/null +++ b/test/fixtures/portduino-config/statusmessage-long.yaml @@ -0,0 +1,7 @@ +# FAULT: StatusMessage is copied into a char[80] and hard null-terminated, so it is +# safe but silently shortened to 79 characters. The operator never sees the message +# they configured. Expect a WARNING. +Lora: + Module: sx1262 +Config: + StatusMessage: "This status message is comfortably longer than the eighty character buffer it is copied into, so it gets cut off" diff --git a/test/fixtures/portduino-config/stranded-key.yaml b/test/fixtures/portduino-config/stranded-key.yaml new file mode 100644 index 000000000..4f159ee80 --- /dev/null +++ b/test/fixtures/portduino-config/stranded-key.yaml @@ -0,0 +1,6 @@ +# FAULT: spidev left at the top level instead of inside a section, where it does +# nothing. Expect an ERROR that names the sections it IS valid in, so the fix is to +# indent it one level. +Lora: + Module: sx1262 +spidev: /dev/spidev0.0 diff --git a/test/fixtures/portduino-config/tcxo-millivolts.yaml b/test/fixtures/portduino-config/tcxo-millivolts.yaml new file mode 100644 index 000000000..3442bb911 --- /dev/null +++ b/test/fixtures/portduino-config/tcxo-millivolts.yaml @@ -0,0 +1,9 @@ +# FAULT: DIO3_TCXO_VOLTAGE is in VOLTS and is multiplied by 1000, but every other +# Meshtastic surface discusses this setting in millivolts -- so "1800" is the natural +# thing to write and silently asks for 1800V. Nothing downstream range-checks it. +# The correct spelling for a 1.8V part is 1.8, or just true. +Lora: + Module: sx1262 + CS: 21 + IRQ: 16 + DIO3_TCXO_VOLTAGE: 1800 diff --git a/test/fixtures/portduino-config/top-level-list.yaml b/test/fixtures/portduino-config/top-level-list.yaml new file mode 100644 index 000000000..1007d78d1 --- /dev/null +++ b/test/fixtures/portduino-config/top-level-list.yaml @@ -0,0 +1,3 @@ +# FAULT: the document root is a sequence, not a mapping, so nothing in it is read. +- Lora +- General diff --git a/test/fixtures/portduino-config/txgain-out-of-range.yaml b/test/fixtures/portduino-config/txgain-out-of-range.yaml new file mode 100644 index 000000000..ece546d91 --- /dev/null +++ b/test/fixtures/portduino-config/txgain-out-of-range.yaml @@ -0,0 +1,5 @@ +# FAULT: the gain table is uint16_t[22], so a negative or >65535 entry silently wraps +# to a completely different gain rather than being rejected. Expect an ERROR each. +Lora: + Module: sx1262 + TX_GAIN_LORA: [0, -5, 70000] diff --git a/test/fixtures/portduino-config/txgain-scalar.yaml b/test/fixtures/portduino-config/txgain-scalar.yaml new file mode 100644 index 000000000..93050386d --- /dev/null +++ b/test/fixtures/portduino-config/txgain-scalar.yaml @@ -0,0 +1,8 @@ +# CLEAN, and a regression guard. TX_GAIN_LORA accepts a bare scalar as well as a list: +# loadConfig() has an else branch reading .as(0). An earlier version of the checker +# treated the list shape as the only legal one and reported this working config as a +# fatal error -- a false positive is worse than a missing check, so this file must stay +# clean. +Lora: + Module: sx1262 + TX_GAIN_LORA: 5 diff --git a/test/fixtures/portduino-config/txgain-too-many.yaml b/test/fixtures/portduino-config/txgain-too-many.yaml new file mode 100644 index 000000000..4bff9062b --- /dev/null +++ b/test/fixtures/portduino-config/txgain-too-many.yaml @@ -0,0 +1,5 @@ +# FAULT: only the first 22 PA points are stored (num_pa_points is min(size, 22)), so +# everything past the 22nd is silently dropped. +Lora: + Module: sx1262 + TX_GAIN_LORA: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25] diff --git a/test/fixtures/portduino-config/unknown-key.yaml b/test/fixtures/portduino-config/unknown-key.yaml new file mode 100644 index 000000000..1ab2cc7de --- /dev/null +++ b/test/fixtures/portduino-config/unknown-key.yaml @@ -0,0 +1,9 @@ +# FAULT: Lora.Frequency is not a key meshtasticd reads, so setting it does nothing. +# Expect a WARNING and exit 0 -- warnings alone must not fail the run. +Lora: + Module: sx1262 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + Frequency: 868.0 diff --git a/test/fixtures/portduino-config/unknown-section.yaml b/test/fixtures/portduino-config/unknown-section.yaml new file mode 100644 index 000000000..7c0cb0154 --- /dev/null +++ b/test/fixtures/portduino-config/unknown-section.yaml @@ -0,0 +1,6 @@ +# FAULT: Telemetry is not a section meshtasticd reads, so the whole block is ignored. +# Expect an ERROR. +Lora: + Module: sx1262 +Telemetry: + Enabled: true diff --git a/test/fixtures/portduino-config/usb-ch341.yaml b/test/fixtures/portduino-config/usb-ch341.yaml new file mode 100644 index 000000000..0c75aecdd --- /dev/null +++ b/test/fixtures/portduino-config/usb-ch341.yaml @@ -0,0 +1,15 @@ +# CLEAN. A CH341 USB-SPI adapter, the meshstick/pinedio shape. The pin numbers here are +# indexes on the adapter, not gpiochip lines: portduinoSetup() skips initGPIOPin() for +# every Lora pin when spidev is ch341 and hands the raw numbers to Ch341Hal. Expect the +# report to say so and to stop citing 'gpiodetect'/'gpioinfo', which do not exist on the +# Windows and macOS hosts where this is the only shape that works. +Lora: + Module: sx1262 + CS: 0 + IRQ: 6 + Reset: 2 + Busy: 4 + spidev: ch341 + DIO3_TCXO_VOLTAGE: true + USB_PID: 0x5512 + USB_VID: 0x1A86 diff --git a/test/fixtures/portduino-config/valid.yaml b/test/fixtures/portduino-config/valid.yaml new file mode 100644 index 000000000..c867d4fbf --- /dev/null +++ b/test/fixtures/portduino-config/valid.yaml @@ -0,0 +1,8 @@ +# CLEAN. Minimal SX126x config, used as the baseline: the checker must not invent a +# finding for it. Also the fixture for --check winning over --output-yaml. +Lora: + Module: sx1262 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 diff --git a/test/fixtures/portduino-config/value-type-fatal-list.yaml b/test/fixtures/portduino-config/value-type-fatal-list.yaml new file mode 100644 index 000000000..77175004a --- /dev/null +++ b/test/fixtures/portduino-config/value-type-fatal-list.yaml @@ -0,0 +1,5 @@ +# FAULT: the other no-fallback read. Each TX_GAIN_LORA entry is .as() with no +# default, so one non-numeric entry throws and stops meshtasticd starting. +Lora: + Module: sx1262 + TX_GAIN_LORA: [0, high, 3] diff --git a/test/fixtures/portduino-config/value-type-fatal.yaml b/test/fixtures/portduino-config/value-type-fatal.yaml new file mode 100644 index 000000000..89ecb64e2 --- /dev/null +++ b/test/fixtures/portduino-config/value-type-fatal.yaml @@ -0,0 +1,8 @@ +# FAULT: Logging.AsciiLogs is one of only two settings read WITHOUT a fallback +# (.as() with no default), so a value that will not convert throws, loadConfig() +# returns false and meshtasticd refuses to start. Before this check existed --check +# called this file clean while the daemon would not boot on it. +Lora: + Module: sx1262 +Logging: + AsciiLogs: yes-please diff --git a/test/fixtures/portduino-config/value-type-silent.yaml b/test/fixtures/portduino-config/value-type-silent.yaml new file mode 100644 index 000000000..d0739893b --- /dev/null +++ b/test/fixtures/portduino-config/value-type-silent.yaml @@ -0,0 +1,10 @@ +# FAULT: values of the wrong type where loadConfig() reads .as(default). yaml-cpp +# hands back the default on a failed conversion, so meshtasticd starts happily and +# every one of these settings silently does nothing. Expect a WARNING each. +Lora: + Module: sx1262 + spiSpeed: fast + SX126X_MAX_POWER: high + DIO2_AS_RF_SWITCH: maybe +General: + MaxNodes: lots diff --git a/test/native-suite-count b/test/native-suite-count index 87523dd7a..d81cc0710 100644 --- a/test/native-suite-count +++ b/test/native-suite-count @@ -1 +1 @@ -41 +42 diff --git a/test/test_fuzz_config/test_main.cpp b/test/test_fuzz_config/test_main.cpp new file mode 100644 index 000000000..5919ebfe9 --- /dev/null +++ b/test/test_fuzz_config/test_main.cpp @@ -0,0 +1,267 @@ +// Adversarial fuzzing of `meshtasticd --check`, above all DuplicateKeyFinder, whose stack balance +// rests on yaml-cpp emitting matched start/end events. The contract is crash-freedom and +// termination only (what the report *says* is asserted by bin/test-config-check.sh); inputs come +// from a seeded LCG, so a failure reproduces from the printed seed. + +// configuration.h pulls in Arduino.h, whose Common.h declares setup()/loop() inside an extern "C" +// block. Must come BEFORE TestUtil.h, same as the other suites. +#include "configuration.h" + +#include "TestUtil.h" +#include + +#include "ConfigCheck.h" +#include "support/DeterministicRng.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +static constexpr uint64_t BASE_SEED = 0xC0FFEE01ULL; + +// Where the checked-in fixtures live, relative to the repo root that `pio test` runs from. +static const char *kFixtureDir = "test/fixtures/portduino-config"; + +// Thousands of full reports would bury the CI log, so stdout goes to /dev/null for the suite. +// Restored by duplicated fd: CI has no controlling terminal, so reopening /dev/tty would lose it. +class StdoutSilencer +{ + public: + StdoutSilencer() + { + fflush(stdout); + savedFd = dup(fileno(stdout)); + const int devNull = open("/dev/null", O_WRONLY); + if (devNull >= 0) { + dup2(devNull, fileno(stdout)); + close(devNull); + } + } + ~StdoutSilencer() + { + fflush(stdout); + if (savedFd >= 0) { + dup2(savedFd, fileno(stdout)); + close(savedFd); + } + } + + private: + int savedFd = -1; +}; + +static std::string tempPath(int n) +{ + return "/tmp/meshtastic-fuzz-config-" + std::to_string(n) + ".yaml"; +} + +/// Write `bytes` to a scratch file and hand it to the checker. Returns nothing: the assertion is +/// that we get here at all, without a crash, a hang, or an escaped exception. +static void runOn(const std::string &bytes, int slot) +{ + const std::string path = tempPath(slot); + { + std::ofstream out(path, std::ios::binary); + out.write(bytes.data(), (std::streamsize)bytes.size()); + } + try { + runConfigCheck({path}); + } catch (const std::exception &) { + // An escaped exception is a defect in its own right: portduinoSetup() calls this on the way + // to exit() with nothing above it to catch. + TEST_FAIL_MESSAGE("runConfigCheck() let an exception escape"); + } + std::error_code ec; + std::filesystem::remove(path, ec); +} + +static std::vector loadSeedCorpus() +{ + std::vector corpus; + std::error_code ec; + for (const auto &entry : std::filesystem::recursive_directory_iterator(kFixtureDir, ec)) { + if (!entry.is_regular_file() || entry.path().extension() != ".yaml") + continue; + std::ifstream in(entry.path(), std::ios::binary); + std::ostringstream buf; + buf << in.rdbuf(); + corpus.push_back(buf.str()); + } + return corpus; +} + +// --------------------------------------------------------------------------- +// C1 - the seed corpus itself +// --------------------------------------------------------------------------- + +void test_seed_corpus_survives(void) +{ + StdoutSilencer quiet; + const auto corpus = loadSeedCorpus(); + // A corpus that failed to load would make every later group vacuous, so prove it is there. + TEST_ASSERT_TRUE_MESSAGE(corpus.size() >= 20, "fixture corpus not found - is the test running from the repo root?"); + int slot = 0; + for (const auto &seed : corpus) + runOn(seed, slot++); +} + +// --------------------------------------------------------------------------- +// C2 - byte mutation of the corpus +// --------------------------------------------------------------------------- + +void test_mutated_corpus(void) +{ + StdoutSilencer quiet; + const auto corpus = loadSeedCorpus(); + TEST_ASSERT_TRUE(corpus.size() >= 20); + rngSeed(BASE_SEED + 1); + + for (int i = 0; i < 3000; i++) { + std::string s = corpus[rngRange((uint32_t)corpus.size())]; + if (s.empty()) + continue; + + switch (rngRange(5)) { + case 0: // flip a byte + s[rngRange((uint32_t)s.size())] = (char)rngByte(); + break; + case 1: // truncate + s.resize(rngRange((uint32_t)s.size())); + break; + case 2: // insert a run of one byte + s.insert(rngRange((uint32_t)s.size()), std::string(1 + rngRange(64), (char)rngByte())); + break; + case 3: // splice two fixtures together + s += corpus[rngRange((uint32_t)corpus.size())]; + break; + default: // delete a span + if (s.size() > 1) { + const uint32_t at = rngRange((uint32_t)s.size() - 1); + s.erase(at, 1 + rngRange((uint32_t)(s.size() - at))); + } + break; + } + runOn(s, i % 8); + } +} + +// --------------------------------------------------------------------------- +// C3 - structural torture +// --------------------------------------------------------------------------- +// Shapes chosen to stress the event-stream walker rather than the parser. + +void test_structural_torture(void) +{ + StdoutSilencer quiet; + rngSeed(BASE_SEED + 2); + int slot = 0; + + // Deep nesting, both flow and block style. + for (int depth : {1, 2, 8, 64, 512, 4096}) { + runOn("Lora:\n rfswitch_table: " + std::string(depth, '[') + std::string(depth, ']'), slot++); + std::string block = "Lora:\n"; + for (int i = 0; i < depth; i++) + block += std::string(2 + i * 2, ' ') + "k:\n"; + runOn(block, slot++); + } + + // Duplicate keys at every depth, which is what DuplicateKeyFinder's stack is for. + for (int repeat : {2, 16, 256}) { + std::string dup = "Lora:\n"; + for (int i = 0; i < repeat; i++) + dup += " CS: " + std::to_string(i) + "\n"; + runOn(dup, slot++); + std::string nested = "Lora:\n rfswitch_table:\n"; + for (int i = 0; i < repeat; i++) + nested += " MODE_RX: [HIGH]\n"; + runOn(nested, slot++); + } + + // Anchors and aliases: one node reachable from many paths, including a merge key. + runOn("Lora: &a\n Module: sx1262\nDisplay: *a\n", slot++); + runOn("a: &x [1, 2]\nLora:\n Enable_Pins: *x\n rfswitch_table: *x\n", slot++); + runOn("base: &b {CS: 1}\nLora:\n <<: *b\n IRQ: 2\n", slot++); + + // Collections where a scalar is expected and vice versa, across every known section. + for (const char *section : {"Lora", "General", "Display", "Logging", "Webserver", "Meta", "GPIO"}) { + runOn(std::string(section) + ": [1, 2, 3]\n", slot++); + runOn(std::string(section) + ": ~\n", slot++); + runOn(std::string(section) + ":\n ? [complex, key]\n : value\n", slot++); + } + + // Very long keys and scalars. + runOn("Lora:\n " + std::string(64 * 1024, 'k') + ": 1\n", slot++); + runOn("Lora:\n Module: " + std::string(256 * 1024, 'v') + "\n", slot++); + + // Multiple documents in one file, which HandleNextDocument() loops over. + runOn("Lora:\n CS: 1\n---\nLora:\n CS: 2\n---\n[]\n", slot++); +} + +// --------------------------------------------------------------------------- +// C4 - random bytes (DISABLED) +// --------------------------------------------------------------------------- +// Off for CI cost: half the inputs and half the runtime for the least return, since uniform noise +// is almost always rejected on the first token. Flip to 1 to restore it. +#define FUZZ_CONFIG_RANDOM_BYTES 0 + +#if FUZZ_CONFIG_RANDOM_BYTES +void test_random_bytes(void) +{ + StdoutSilencer quiet; + rngSeed(BASE_SEED + 3); + + for (int i = 0; i < 1500; i++) { + std::string s; + s.resize(rngRange(2048)); + for (auto &c : s) + c = (char)rngByte(); + runOn(s, i % 8); + } + + // Bytes drawn only from YAML's structural alphabet get much deeper into the parser than + // uniform random noise, which is usually rejected on the first token. + static const char kYamlChars[] = " \t\n-:[]{}#&*!|>'\"%@`,?abcLora0123"; + for (int i = 0; i < 1500; i++) { + std::string s; + s.resize(rngRange(2048)); + for (auto &c : s) + c = kYamlChars[rngRange(sizeof(kYamlChars) - 1)]; + runOn(s, i % 8); + } +} +#endif // FUZZ_CONFIG_RANDOM_BYTES + +void setUp(void) {} +void tearDown(void) {} + +// The portduino framework supplies main() and calls setup()/loop(), so the suite's entry point is +// setup() rather than main() -- defining main() here collides with the framework's. +void setup() +{ + initializeTestEnvironment(); + UNITY_BEGIN(); + + printf("\n=== Group C1: seed corpus ===\n"); + RUN_TEST(test_seed_corpus_survives); + + printf("\n=== Group C2: mutated corpus ===\n"); + RUN_TEST(test_mutated_corpus); + + printf("\n=== Group C3: structural torture ===\n"); + RUN_TEST(test_structural_torture); + +#if FUZZ_CONFIG_RANDOM_BYTES + printf("\n=== Group C4: random bytes ===\n"); + RUN_TEST(test_random_bytes); +#endif + + exit(UNITY_END()); +} + +void loop() {}