From 2d6dad9ee9f24391c849206c0a7e73b1a48377d3 Mon Sep 17 00:00:00 2001 From: Tom <116762865+NomDeTom@users.noreply.github.com> Date: Wed, 16 Sep 2026 10:44:06 +0000 Subject: [PATCH] Portduino: Fix LR2021 switch tables, power ceilings and IRQ handling (#11382) * fix(portduino): recognise the LR2021 power ceilings in --check loadConfig() has read Lora.LR2021_MAX_POWER and Lora.LR2021_MAX_POWER_HF since LR2021 support landed, but neither was listed in the config checker's schema. --check therefore reported both as "unknown key ... ignored by meshtasticd" -- false, and actively misleading: it tells the user to delete a key that is doing exactly what they wanted. This breaks the contract stated above schema(), that a key taught to loadConfig() is added there too. CI enforces that by running --check over bin/config.d/**, but no shipped config sets either key -- or mentions lr2021 at all -- so nothing ever tripped over the omission. It could only surface for someone hand-writing an LR2021 config. LR20x0 is the only module with two power ceilings, one per band, selected at runtime by region; every other family expresses the split as separate module names and needs a single key. That is the likely reason the pair was missed while every other *_MAX_POWER key was added. Also adds both to valueSpecs(), so a wrong-typed value is reported rather than silently replaced by the default. * feat(portduino): configurable IRQ DIO and a chip-neutral RF switch table for LR20x0 Two gaps found bringing an LR2021 up under meshtasticd on a Luckfox Lyra Zero W. Both sit in the LR20x0 support added in #11252, and they interact: the switch table has to be written slightly wrong to pass validation, and the interrupt lands on a pin that table is driving. Every symptom is silent, because begin() only exercises SPI and BUSY -- the radio reports init success and then receives nothing. IRQ DIO could not be set on Portduino ------------------------------------- LR20x0Interface picked the IRQ DIO purely at compile time, and neither LR2021_IRQ_DIO_NUM nor IRQ_DIO_NUM exists for a Portduino target, so meshtasticd always fell through to RadioLib's default of DIO5 -- which is also the first RF switch line on carriers using the DIO5-DIO8 table. A variant says this with a #define (the pro-micro DIY board uses DIO9); a carrier has only the YAML, and had no way to say it. Adds Lora.IRQ_DIO_NUM, and an ARCH_PORTDUINO branch after the two existing #define branches, so a variant that already sets one still wins. The switch table was parsed as LR11xx-only ------------------------------------------ Pin names resolved to RADIOLIB_LR11X0_DIOn whatever the radio, and the mode set was the LR11xx's, so MODE_RX_HF -- a mode the LR20x0 really has -- was rejected as an unknown key and had to be omitted. The two families are not interchangeable: an LR11xx has no DIO9, so its fifth switch slot is DIO10, while an LR20x0's fifth slot is DIO9 and DIO10 is its sixth. A table naming DIO10 was therefore driving the wrong pin on an LR20x0. The YAML layer now stores what was written -- a DIO number and a neutral mode id -- and each interface supplies its own DIO constants and OpMode_t map to a shared builder. Neither family's constants are assumed to coincide with the other's. This also fixes a round trip in the config writer, which decoded pins by comparing against RADIOLIB_LR11X0_* and always emitted five values per mode row: for a four-pin table it produced YAML that --check would reject for mismatched row lengths. --check ------- Findings are now judged against the resolved module rather than a fixed list, so a mode or pin the part does have can no longer be rejected, and one it does not have is named instead of silently accepted. The claim that the table "is only applied to LR11xx radios" was stale and is corrected, and the missing-table warning now covers both families. "auto" is excluded throughout: the module has not been probed yet, so absence cannot be judged. The IRQ/switch-pin collision is reported in both directions, including the harder case where no key is set and the radio default collides -- nothing in the file looks wrong. Note that listing a pin is what breaks it, not driving it: setRfSwitchTable() reassigns the DIO function for every pin in the list whatever the levels say, so an all-LOW column is still a collision. Seven fixtures cover these, including a false-positive guard: DIO5 as the interrupt is normal, and must stay silent when the table is elsewhere. * feat(portduino): let the YAML ask for a TCXO probe, across every family that has one A variant declares "a TCXO may or may not be fitted" at compile time with TCXO_OPTIONAL, because the board is known when the image is built. A Portduino carrier cannot: the same meshtasticd binary runs on hardware populated either way, so the statement has to arrive as YAML and be answered at runtime. Adds Lora.TCXO_OPTIONAL, and TCXO_OPTIONAL_ENABLED in RadioLibInterface.h to unify the two, so each driver asks the question once rather than growing a second, Portduino-shaped code path. On an embedded target it stays a compile-time constant, so `if (TCXO_OPTIONAL_ENABLED)` folds away exactly as the old `#if` did: the nrf52_promicro_diy_tcxo image, which defines TCXO_OPTIONAL and so exercises the converted branches, still ends at 0xDF1D0 -- the same address as before this change. It is defined there rather than in a header of its own because InterfacesTemplates.cpp includes all three interface .cpp files into one translation unit, where a per-file definition would collide. Covers every family that has a TCXO reference to probe for: SX126x (sx1262/sx1268/LLCC68), LR11xx and LR20x0. With no DIO3_TCXO_VOLTAGE given, the TCXO attempt uses RadioLib's own 1.6 V default rather than being skipped -- otherwise there is nothing to fall back FROM and the flag would silently do nothing. This is also the FIXME that sat on the Portduino branch in LR20x0Interface: an unset voltage now means "no TCXO" explicitly. Two things are deliberately left alone: Each family keeps its own probe order. LR11xx tries XTAL first, because a TCXO-first attempt hangs RadioLib's unbounded calibration wait on a module with no TCXO fitted, whereas XTAL fails fast and cleanly on a module that has one; LR20x0 and SX126x try the TCXO first. A carrier therefore behaves the same way in a Portduino build as in an embedded one, and changing an order stays a hardware-behaviour decision rather than a tidying-up one. The SX126x retry is Portduino-only. An embedded TCXO_OPTIONAL board already gets this from initLoRa(), which constructs a second SX126x interface with no Vref when the first fails; retrying inside init() as well would leave that ladder step unreachable and change how every existing t-echo-class board reports its oscillator. A Portduino build has no ladder to fall through, because the module is named in YAML rather than probed. --check learns the key, reports which Vref will actually be tried, and warns when it is set on a radio with no TCXO reference, where it is read, stored and inert. * docs(portduino): condense the comments on this branch The repo asks for one or two lines and no multi-paragraph blocks, on the grounds that the diff and the commit message carry the rationale while the code carries the behaviour. What landed here was well past that: 163 added comment lines, including a 26-line block above a single macro. Removes the rhetoric, the issue numbers and the before-and-after asides, and the notes on where a thing used to live. No added block is longer than three lines now. Two facts needed stating and are stated once each rather than repeated at every use: the slot/DIO divergence between the families, in PortduinoGlue.h, and the per-family TCXO probe order, in RadioLibInterface.h. The longest surviving explanation is why an all-LOW switch column still collides with the interrupt, which sits in the fixtures README because without it that pair of fixtures reads as contradictory. Comments only; no functional change. * address CodeRabbit review on #11382 - SX126xInterface: distinguish an explicit DIO3_TCXO_VOLTAGE from the TCXO_OPTIONAL probing default in the debug log instead of always claiming the config field was set. - ConfigCheck: modesFor() now reports an unresolved use_autoconf against the union of both radio families' modes, not the LR11xx subset - fixes a false "not a mode this part has" warning for valid LR2021-only modes (e.g. RFSW_RX_HF) before autodetection resolves the module. - PortduinoGlue loadConfig: build rfswitch_mode_high[m] as a fresh per-row bitmask instead of OR-accumulating onto a stale value, so a config re-parse can clear a slot back to LOW. - PortduinoGlue YAML serialization: gate rfswitch_table emission on has_rfswitch_table rather than rfswitch_dio_num[0] >= 0 (missed sparse pin lists), and track each emitted pin's original slot so row values line up correctly instead of shifting when a low slot is absent. - config-dist.yaml: document the per-family TCXO/XTAL probe order (SX126x/LR20x0 TCXO-first, LR11x0 XTAL-first). - Trim three overlong comments per the coding-guideline nitpicks. Left the SX126x XTAL-retry-on-oscillator-failure nitpick alone - RadioLib's begin() already does its own XOSC_START_ERR recovery internally, and narrowing our wrapper's retry condition on top of that needs hardware to verify it doesn't regress a real failure path. Verified: bin/test-config-check.sh GREEN 69/69 against an isolated native build; pio test -e native -f test_rtc PASSED. * fix CI: cppcheck duplicateValueTernary, harden kRfSwitchModes init LR11x0Interface::init(): work around cppcheck's duplicateValueTernary on `TCXO_OPTIONAL_ENABLED ? 0 : tcxoVoltage` (both branches fold to 0 on a board with no ARCH_PORTDUINO, no TCXO_OPTIONAL, and no explicit Vref, since tcxoVoltage already reduces to 0 via the same macro chain) by splitting it into a plain assignment + if, rather than suppressing the warning. The other TCXO_OPTIONAL_ENABLED ternaries in this PR (LR11x0Interface.cpp:75, SX126xInterface.cpp:76, LR20x0Interface.cpp: 86,240) pick between TCXO_OPTIONAL_DEFAULT_VOLTAGE (1.6f) and 0, which can never coincide, so they're unaffected and left as-is. ConfigCheck.cpp: kRfSwitchModes was a namespace-scope global with dynamic initialization (a lambda IIFE) reading kRfSwitchModeNames, which is defined in a different translation unit (PortduinoGlue.cpp). Currently safe only because kRfSwitchModeNames's initializer is constant-expression-only (string literals + enum constants), which the standard guarantees completes before any TU's dynamic initializers - but that safety is silent and would break if PortduinoGlue.cpp's array initializer ever stopped being a constant expression, with nothing to warn a future editor. Converted to a function-local static (Meyers' singleton), which is correct by construction regardless of the other TU's initializer, updating all 4 call sites (definition + 3 uses) from kRfSwitchModes to kRfSwitchModes(). Verified: pio test -e native -f test_radio PASSED; bin/test-config- check.sh GREEN 69/69 against an isolated native build. * refactor: simplify TCXO voltage handling across interfaces and improve comments * fix rfswitch_table cross-file merge; drop now-stale checker warning Three CodeRabbit findings on 09e0d390c, addressed together since #2 and #3 are the same root cause: 1. PortduinoGlue.cpp: require an exact "DIO" match when parsing rfswitch_table.pins. sscanf's %d stops at the first non-digit, so "DIO5invalid" silently parsed as DIO5 at runtime even though ConfigCheck.cpp's static validator (exact match against kRfSwitchPins) already rejected it - checker and loader disagreed. 2. PortduinoGlue.cpp: reset all 5 pin slots and all 8 mode rows before applying a table, rather than only overwriting what the new table mentions. A later config.d file that omitted a mode a prior file had set (e.g. only redefining MODE_TX) let the earlier file's MODE_RX leak through, contradicting "last file wins" - the rule every other Lora: key already follows. 3. ConfigCheck.cpp: with #2 fixed, rfswitch_table behaves like any other cross-file key, so removed the special-cased ERROR in checkCrossFileOverlap ("These do NOT override each other... OR of every table") - it described the pre-fix OR-accumulation bug and is no longer accurate. Falls through to the generic "last file wins" INFO now. Renamed/repurposed the rfswitch-sticky fixture to rfswitch-last-wins and updated its assertion (was rc=1 asserting the old error text, now rc=0 asserting the generic info) and the fixtures README. Verified: bin/test-config-check.sh GREEN 69/69 against an isolated native build, including the renamed assertion. * Assert the effective rfswitch table, not just the overlap diagnostic The "last one wins" case checked that the cross-file info fires and that the result is clean. Neither observes the table the loader actually ended up with, so the merge bug ee9b5b81e fixed - a later table leaving an earlier file's pins and mode rows as carryover - would still have passed it. Raised by CodeRabbit. Asserting the value needs two things the existing case cannot supply. The winner has to be deterministic. Both files in rfswitch-last-wins/ sit in config.d/, which is walked with a bare directory_iterator and no sort, so which one lands last is up to the filesystem - the point configd-conflict/ exists to make, and the reason the checker warns rather than assuming alphabetical order. rfswitch-replace/ puts the losing table in config.yaml instead, which is always loaded before config.d/. The loser is the wider of the two, four pins and three all-HIGH mode rows against the winner's two pins and one all-LOW row, so carryover shows up as a surviving pin, a surviving mode row, or a HIGH that should be LOW. The table has to be observable. The check report says no more than "RF switch table : set", and check-yaml cannot help: it is --check --output-yaml, and --check wins and exits before the dump - which the case just below it asserts. emit_yaml() does serialise the effective table, so the assert helper grows a yaml mode that passes --output-yaml alone. Confirmed non-vacuous: with the reset loop in loadConfig() removed, the new assertion fails and the old one still passes. Config-check suite GREEN 70/70. Native suite GREEN 44/44, 968 cases. * Say nothing about rows the radio will never read Two of the RF-switch diagnostics judged a table against a family's mode list without first asking whether the module reads a table at all. modesFor() treated every module that was not an LR20x0 as LR11xx-like, so an sx1262 carrying a table was told which of its rows were "not a mode sx1262 has" and which modes it had omitted - alongside the correct warning that the whole table is inert. pinsFor() already returned an empty set for these parts and its caller already guarded on that; the mode path now matches. The missing-mode advice is dropped under "auto" as well. The module has not been probed, so the union of both families is all there is to compare against, and naming its absent modes would advise adding MODE_TX_HP, MODE_GNSS and MODE_WIFI rows to what may turn out to be an LR20x0. Fixtures for both silences, and an assert() needle prefixed with '!' to hold them: a line that is merely absent today is otherwise nobody's regression. Also corrects the kLr11x0SwitchDios/kLr20x0SwitchDios comments. They describe a slot mapping, but buildRfSwitchTable() searches them by value to find the parallel pin constant - and with 7 DIOs against 5 YAML pin slots, the LR20x0 array could not be positional. * Warn when the two TCXO keys ask for opposite things DIO3_TCXO_VOLTAGE written out as false or 0 asks for DIO3 to be left alone, and stores identically to the key being absent - so TCXO_OPTIONAL then probes DIO3 at the radio default anyway. Both keys behave exactly as documented; only together are they wrong, which is what makes the outcome surprising. loadConfig() now keeps the distinction that the store loses, and --check reports the contradiction and which key to drop. The flag is diagnostic only and is not serialized: an explicit false and an absent key both round-trip as absent, as they did before. Also fixes the Portduino TCXO log lines, which named the variant define SX126X_DIO3_TCXO_VOLTAGE on a path where the knob is the YAML key, and adds a TODO over the SX126x XTAL retry. RadioLib has autocorrected that case itself since 7.5.0 - SX126x::modSetup() retries config() on the XTAL when begin() fails with SPI_CMD_FAILED and XOSC_START_ERR - so the ordinary case never reaches our retry and what does is mostly invalid settings, logged as a TCXO fault. * feat(portduino): bound Lora.IRQ_DIO_NUM, accept the older spelling, document both An LR20x0 raises its interrupt on DIO5 through DIO11. Anything else was read straight out of the YAML and programmed into RadioLib, where it routes the IRQ nowhere: begin() touches only SPI and BUSY, so the radio reports init success and then never receives a packet - the same failure the switch-pin collision check already covers, reached by a typo instead. Refuse it in loadConfig() and again in the driver before it reaches RadioLib, warning both times. Because loadConfig() discards the value, the merged config cannot tell a rejected number from an absent key, so --check judges the range in its per-file pass where the offending line is still known. LR2021_IRQ_DIO_NUM, the spelling carried by the two earlier LR2021 branches, is read when IRQ_DIO_NUM is absent and reported as shadowed when it is not. Both keys, the DIO range and the collision that makes the setting matter are now described in config-dist.yaml. * fix(portduino): a rejected IRQ_DIO_NUM returns to the radio default loadConfig() runs once per file - the main config, then each file in config.d/ - and they all write the same portduino_config. An out-of-range Lora.IRQ_DIO_NUM warned that it was falling back to the radio default but left any valid value an earlier file had set, so LR20x0Interface went on programming that stale DIO. Reset it to -1, the unset sentinel every other reader already tests for. Pinned by a new fixture: DIO9 in the main config, out of range in config.d/. The main config is always read first, so the ordering is deterministic, unlike two files in config.d/ (see rfswitch-last-wins). The assertion requires the summary to name the radio default and not DIO9; with the reset removed and rebuilt, it is the only assertion that fails. --- bin/config-dist.yaml | 10 + bin/test-config-check.sh | 134 ++++++++- src/mesh/LR11x0Interface.cpp | 63 ++-- src/mesh/LR20x0Interface.cpp | 70 ++++- src/mesh/RadioLibInterface.h | 13 + src/mesh/SX126xInterface.cpp | 37 ++- src/platform/portduino/ConfigCheck.cpp | 277 +++++++++++++++--- src/platform/portduino/PortduinoGlue.cpp | 161 +++++++--- src/platform/portduino/PortduinoGlue.h | 117 +++++--- test/fixtures/portduino-config/README.md | 79 ++++- .../config.d/irq-override.yaml | 3 + .../irq-stale-override/config.yaml | 20 ++ .../rfswitch-auto-partial.yaml | 12 + .../portduino-config/rfswitch-bad-pin.yaml | 4 +- .../rfswitch-inert-table.yaml | 10 + .../config.d/a-first.yaml | 0 .../rfswitch-last-wins/config.d/b-second.yaml | 7 + .../rfswitch-last-wins/config.yaml | 4 + .../rfswitch-lr2021-irq-alias.yaml | 18 ++ .../rfswitch-lr2021-irq-all-low.yaml | 15 + .../rfswitch-lr2021-irq-clear.yaml | 18 ++ .../rfswitch-lr2021-irq-collision.yaml | 17 ++ .../rfswitch-lr2021-irq-default.yaml | 16 + .../rfswitch-lr2021-irq-out-of-range.yaml | 18 ++ .../rfswitch-lr2021-no-table.yaml | 11 + .../rfswitch-lr2021-wrong-mode.yaml | 17 ++ .../portduino-config/rfswitch-lr2021.yaml | 20 ++ .../rfswitch-replace/config.d/override.yaml | 6 + .../rfswitch-replace/config.yaml | 21 ++ .../rfswitch-sticky/config.d/b-second.yaml | 9 - .../rfswitch-sticky/config.yaml | 4 - .../tcxo-optional-contradiction.yaml | 12 + .../tcxo-optional-sx1262.yaml | 11 + .../tcxo-optional-unsupported.yaml | 9 + .../portduino-config/tcxo-optional.yaml | 19 ++ 35 files changed, 1057 insertions(+), 205 deletions(-) create mode 100644 test/fixtures/portduino-config/irq-stale-override/config.d/irq-override.yaml create mode 100644 test/fixtures/portduino-config/irq-stale-override/config.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-auto-partial.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-inert-table.yaml rename test/fixtures/portduino-config/{rfswitch-sticky => rfswitch-last-wins}/config.d/a-first.yaml (100%) create mode 100644 test/fixtures/portduino-config/rfswitch-last-wins/config.d/b-second.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-last-wins/config.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-lr2021-irq-alias.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-lr2021-irq-all-low.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-lr2021-irq-clear.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-lr2021-irq-collision.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-lr2021-irq-default.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-lr2021-irq-out-of-range.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-lr2021-no-table.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-lr2021-wrong-mode.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-lr2021.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-replace/config.d/override.yaml create mode 100644 test/fixtures/portduino-config/rfswitch-replace/config.yaml delete mode 100644 test/fixtures/portduino-config/rfswitch-sticky/config.d/b-second.yaml delete mode 100644 test/fixtures/portduino-config/rfswitch-sticky/config.yaml create mode 100644 test/fixtures/portduino-config/tcxo-optional-contradiction.yaml create mode 100644 test/fixtures/portduino-config/tcxo-optional-sx1262.yaml create mode 100644 test/fixtures/portduino-config/tcxo-optional-unsupported.yaml create mode 100644 test/fixtures/portduino-config/tcxo-optional.yaml diff --git a/bin/config-dist.yaml b/bin/config-dist.yaml index 46a044e8dd..cf0d56da26 100644 --- a/bin/config-dist.yaml +++ b/bin/config-dist.yaml @@ -81,6 +81,16 @@ Lora: # DIO3_TCXO_VOLTAGE: true # the Waveshare Core1262 and others are known to need this setting +# TCXO_OPTIONAL: true # Probe both oscillators (default Vref 1.6V); SX126x/LR20x0/LR11xx only. +# # SX126x and LR20x0 try TCXO first; LR11x0 tries the crystal first. + +### DIO the LR2021 raises its interrupt on, DIO5 to DIO11. Defaults to DIO5, which many carriers +### also drive as an RF switch line -- a pin cannot be both, and the radio will report init +### success and then receive nothing. Point it at a DIO rfswitch_table does not list. +### LR2021_IRQ_DIO_NUM is an older spelling of the same setting, read only if IRQ_DIO_NUM is absent. +# IRQ_DIO_NUM: 9 +# LR2021_IRQ_DIO_NUM: 9 + # TXen: x # TX and RX enable pins # RXen: x diff --git a/bin/test-config-check.sh b/bin/test-config-check.sh index b3027fe7ba..b39f19277a 100755 --- a/bin/test-config-check.sh +++ b/bin/test-config-check.sh @@ -59,11 +59,13 @@ PASS=0 FAIL=0 # assert [expected-substring...] -# mode: "check" adds --check, "check-yaml" adds --check --output-yaml, -# "normal" runs with neither. +# mode: "check" adds --check, "yaml" adds --output-yaml, "check-yaml" adds both +# (to assert that --check wins), "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. +# Each expected substring must appear in the output. One prefixed with '!' must not: +# an info line that is merely absent today would otherwise be nobody's regression. assert() { local desc="$1" want_rc="$2" fixture="$3" mode="$4" shift 4 @@ -77,6 +79,7 @@ assert() { local args=(--config "$config" -d "$WORKDIR/fs") case $mode in check) args+=(--check) ;; + yaml) args+=(--output-yaml) ;; check-yaml) args+=(--check --output-yaml) ;; esac @@ -90,7 +93,11 @@ assert() { [[ $rc -ne $want_rc ]] && problems+=("exit $rc, wanted $want_rc") local needle for needle in "$@"; do - grep -qF -- "$needle" <<<"$out" || problems+=("missing: $needle") + if [[ $needle == '!'* ]]; then + grep -qF -- "${needle#!}" <<<"$out" && problems+=("unexpected: ${needle#!}") + else + grep -qF -- "$needle" <<<"$out" || problems+=("missing: $needle") + fi done if [[ ${#problems[@]} -eq 0 ]]; then @@ -152,8 +159,8 @@ assert "wrong-case module suggests the right spelling" 1 module-wrong-case.yaml echo echo "LR11xx rfswitch table:" -assert "unrecognised switch pin" 1 rfswitch-bad-pin.yaml check \ - "'DIO9' is not a recognised pin" \ +assert "switch pin the module does not have" 1 rfswitch-bad-pin.yaml check \ + "'DIO9' is not an RF switch pin on lr1121" \ "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" \ @@ -190,14 +197,107 @@ 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" +# Under autodetect the part is not known yet, so neither is which modes it has. Naming them +# against the union of both families would advise adding rows an LR20x0 cannot use. +assert "omitted modes are not guessed at under auto" 0 rfswitch-auto-partial.yaml check \ + '!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" \ +assert "table on a radio that never applies one" 0 module-mismatch-sx126x.yaml check \ + "the table is only applied to LR11xx and LR20x0 radios" \ + "Result: 0 errors, 1 warning" +# ...and that is the only thing worth saying. Judging the rows against a family's mode list +# would report MODE_RX_HF as the ignored one, implying the others are applied. +assert "an inert table is not judged row by row" 0 rfswitch-inert-table.yaml check \ + "the table is only applied to LR11xx and LR20x0 radios" \ + '!is not a mode' \ + '!default to all pins LOW' \ + "Result: 0 errors, 1 warning" + +echo +echo "LR20x0 switch table and interrupt DIO:" +# MODE_RX_HF is a real mode on this part, so it must not be rejected as an unknown key. +assert "a correct LR20x0 table is accepted" 0 rfswitch-lr2021.yaml check \ + "Module : lr2021" \ + "RF switch table : set" \ + "IRQ DIO : DIO9" \ + "Result: 0 errors, 0 warnings" +# begin() needs only SPI and BUSY, so the radio reports init success either way. +assert "IRQ DIO collides with a switch pin" 1 rfswitch-lr2021-irq-collision.yaml check \ + "Lora.IRQ_DIO_NUM is DIO5, which Lora.rfswitch_table.pins also drives" \ + "Result: 1 error, 0 warnings" +# The same collision, reached by omitting the key: the radio default is DIO5. +assert "default IRQ DIO collides with a switch pin" 1 rfswitch-lr2021-irq-default.yaml check \ + "no Lora.IRQ_DIO_NUM is set" \ + "raises its interrupt on DIO5 by default" \ + "IRQ DIO : DIO5 (radio default)" \ + "Result: 1 error, 0 warnings" +# DIO5 is the radio's own default, so a check keyed on the DIO number rather than the pins +# list would fire on most working configs. +assert "IRQ on DIO5 with the table elsewhere is clean" 0 rfswitch-lr2021-irq-clear.yaml check \ + "IRQ DIO : DIO5" \ + "RF switch table : set" \ + "Result: 0 errors, 0 warnings" +# Listing the pin is what breaks it, not driving it: setRfSwitchTable() reassigns the DIO +# function for every pin in the list whatever the levels say. +assert "IRQ pin listed but never driven HIGH" 1 rfswitch-lr2021-irq-all-low.yaml check \ + "Lora.IRQ_DIO_NUM is DIO5, which Lora.rfswitch_table.pins also drives" \ + "Result: 1 error, 0 warnings" +# Out of range is discarded at load, so the merged view sees an absent key: only the per-file +# pass still knows a value was written at all. +assert "IRQ DIO outside DIO5-DIO11" 1 rfswitch-lr2021-irq-out-of-range.yaml check \ + "Lora.IRQ_DIO_NUM is 3, outside DIO5-DIO11" \ + "IRQ DIO : DIO5 (radio default)" \ + "Result: 1 error, 0 warnings" +# A rejected override must not leave the earlier file's value in place: the warning promises the +# radio default, so the merged view has to show it. +assert "out-of-range override drops back to the radio default" 1 irq-stale-override/config.yaml check \ + "Lora.IRQ_DIO_NUM is 3, outside DIO5-DIO11" \ + "IRQ DIO : DIO5 (radio default)" \ + "!IRQ DIO : DIO9" +# The older spelling is accepted, but never over the generic key. +assert "LR2021_IRQ_DIO_NUM is shadowed by IRQ_DIO_NUM" 0 rfswitch-lr2021-irq-alias.yaml check \ + "Lora.LR2021_IRQ_DIO_NUM is the older spelling" \ + "IRQ DIO : DIO9" \ + "Result: 0 errors, 1 warning" +# The collision check is gated on there being a table, so only the missing table is reported. +assert "LR20x0 without a table cannot transmit" 0 rfswitch-lr2021-no-table.yaml check \ + "Module is lr2021 but no Lora.rfswitch_table is set" \ + "RF switch table : not set" \ + "Result: 0 errors, 1 warning" +# A mode belonging to the other family is a dropped row, not a typo. +assert "modes the LR20x0 does not have" 0 rfswitch-lr2021-wrong-mode.yaml check \ + "Lora.rfswitch_table.MODE_TX_HP is not a mode lr2021 has" \ + "Lora.rfswitch_table.MODE_GNSS is not a mode lr2021 has" \ + "Result: 0 errors, 2 warnings" + +echo +echo "TCXO probing (Lora.TCXO_OPTIONAL):" +# With no explicit Vref the TCXO attempt uses the radio default rather than being skipped, +# otherwise there would be nothing to fall back from. +assert "probe with no Vref names the radio default" 0 tcxo-optional.yaml check \ + "TCXO probe : yes, 1600 mV (radio default) and XTAL" \ + "Result: 0 errors, 0 warnings" +# The same flag on another family, with an explicit Vref that must be the one reported. +assert "probe on an SX126x uses the given Vref" 0 tcxo-optional-sx1262.yaml check \ + "Module : sx1262" \ + "TCXO probe : yes, 1800 mV and XTAL" \ + "Result: 0 errors, 0 warnings" +# On a part with no TCXO reference the key is read, stored and inert. +assert "probe on a radio with no TCXO does nothing" 0 tcxo-optional-unsupported.yaml check \ + "Lora.TCXO_OPTIONAL is set but Module is sx1280" \ + "no TCXO reference to probe for" \ + "Result: 0 errors, 1 warning" +# A written-out false stores the same as an absent key, so the probe drives DIO3 regardless +# of what the user asked for. Both keys behave as documented; only together are they wrong. +assert "DIO3_TCXO_VOLTAGE off contradicts the probe" 0 tcxo-optional-contradiction.yaml check \ + "Lora.DIO3_TCXO_VOLTAGE is off, which asks for DIO3 not to be driven" \ + "The probe wins" \ "Result: 0 errors, 1 warning" echo @@ -344,12 +444,20 @@ assert "config.d overrides are reported" 0 configd-conflict/config.yaml check \ "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" +# rfswitch_table follows "last file wins" like every other Lora: key: no special-cased +# error, just the standard cross-file-overlap info. +assert "rfswitch tables across files: last one wins" 0 rfswitch-last-wins/config.yaml check \ + "'Lora.rfswitch_table' is set in 2 files" \ + "The file loaded last wins" \ + "Result: 0 errors," +# The case above proves the diagnostic fires; it cannot prove the table was replaced rather than +# merged, because which of two config.d/ files wins is up to the filesystem. This fixture puts the +# loser in config.yaml, which is always loaded first, so the effective table is deterministic and +# can be asserted by value. --output-yaml is the only output that reports it: the check report +# says no more than "set". +assert "a later rfswitch table replaces the earlier one" 0 rfswitch-replace/config.yaml yaml \ + "pins: [DIO5, DIO6]" \ + "MODE_RX: [LOW, LOW]" echo echo "--check takes precedence over --output-yaml:" diff --git a/src/mesh/LR11x0Interface.cpp b/src/mesh/LR11x0Interface.cpp index fdad36fca8..b6183aae39 100644 --- a/src/mesh/LR11x0Interface.cpp +++ b/src/mesh/LR11x0Interface.cpp @@ -27,8 +27,23 @@ #include "rfswitch.h" #elif ARCH_PORTDUINO #include "PortduinoGlue.h" -#define rfswitch_dio_pins portduino_config.rfswitch_dio_pins -#define rfswitch_table portduino_config.rfswitch_table + +// Switch-capable DIOs in slot order with this part's constants; no DIO9, so slot 4 is DIO10. +static const int8_t lr11x0_switch_dio_nums[] = {5, 6, 7, 8, 10}; +static const uint32_t lr11x0_switch_dio_consts[] = {RADIOLIB_LR11X0_DIO5, RADIOLIB_LR11X0_DIO6, RADIOLIB_LR11X0_DIO7, + RADIOLIB_LR11X0_DIO8, RADIOLIB_LR11X0_DIO10}; +static_assert(sizeof(lr11x0_switch_dio_nums) / sizeof(lr11x0_switch_dio_nums[0]) == + sizeof(lr11x0_switch_dio_consts) / sizeof(lr11x0_switch_dio_consts[0]), + "LR11x0 switch DIO numbers and constants must describe the same slots"); + +// This part has MODE_TX_HP/MODE_GNSS/MODE_WIFI and no MODE_RX_HF. +static const int32_t lr11x0_rfswitch_mode_map[RFSW_MODE_COUNT] = { + LR11x0::MODE_STBY, LR11x0::MODE_RX, LR11x0::MODE_TX, LR11x0::MODE_TX_HP, + LR11x0::MODE_TX_HF, RFSW_MODE_UNSUPPORTED, LR11x0::MODE_GNSS, LR11x0::MODE_WIFI, +}; + +static uint32_t rfswitch_dio_pins[Module::RFSWITCH_MAX_PINS]; +static Module::RfSwitchMode_t rfswitch_table[RFSW_MODE_COUNT + 1]; #else static const uint32_t rfswitch_dio_pins[] = {RADIOLIB_NC, RADIOLIB_NC, RADIOLIB_NC, RADIOLIB_NC, RADIOLIB_NC}; static const Module::RfSwitchMode_t rfswitch_table[] = { @@ -57,11 +72,12 @@ static const Module::RfSwitchMode_t rfswitch_table[] = { // Vref to assume for a board that declares a TCXO may be fitted without saying at what voltage. // "TCXO reference voltage to be set on DIO3. Defaults to 1.6 V, set to 0 to skip." per // https://github.com/jgromes/RadioLib/blob/690a050ebb46e6097c5d00c371e961c1caa3b52e/src/modules/LR11x0/LR11x0.h#L471C26-L471C104 -#if defined(TCXO_OPTIONAL) -#define LR11X0_TCXO_DEFAULT_VOLTAGE 1.6f -#else -#define LR11X0_TCXO_DEFAULT_VOLTAGE 0 -#endif +static inline float lr11x0TcxoDefaultVoltage() +{ + if (TCXO_OPTIONAL_ENABLED) + return TCXO_OPTIONAL_DEFAULT_VOLTAGE; + return 0; +} // A chip that never answers can surface either way depending on where RadioLib gave up: a bounded // per-command BUSY wait in Module::SPItransferStream() reports SPI_CMD_TIMEOUT rather than @@ -95,11 +111,11 @@ template bool LR11x0Interface::init() // Portduino leaves dio3_tcxo_voltage at 0 whenever the YAML omits DIO3_TCXO_VOLTAGE, which is the // "no explicit Vref" case, so the TCXO_OPTIONAL default still has to apply there float tcxoVoltage = - portduino_config.dio3_tcxo_voltage > 0 ? (float)portduino_config.dio3_tcxo_voltage / 1000 : LR11X0_TCXO_DEFAULT_VOLTAGE; + portduino_config.dio3_tcxo_voltage > 0 ? (float)portduino_config.dio3_tcxo_voltage / 1000 : lr11x0TcxoDefaultVoltage(); #elif defined(LR11X0_DIO3_TCXO_VOLTAGE) float tcxoVoltage = LR11X0_DIO3_TCXO_VOLTAGE; #else - float tcxoVoltage = LR11X0_TCXO_DEFAULT_VOLTAGE; + float tcxoVoltage = lr11x0TcxoDefaultVoltage(); #endif // DIO3 is free to be used as an IRQ only while no TCXO Vref is driven on it @@ -107,9 +123,8 @@ template bool LR11x0Interface::init() LOG_DEBUG("LR11x0 TCXO Vref %f V on DIO3 (DIO3 unavailable as IRQ)", tcxoVoltage); else LOG_DEBUG("LR11x0 no TCXO Vref, XTAL only (DIO3 free as IRQ)"); -#if defined(TCXO_OPTIONAL) - LOG_DEBUG("TCXO_OPTIONAL: osc type unknown, probe XTAL first, TCXO Vref as fallback"); -#endif + if (TCXO_OPTIONAL_ENABLED) + LOG_DEBUG("TCXO_OPTIONAL: osc type unknown, probe XTAL first, TCXO Vref as fallback"); RadioLibInterface::init(); @@ -143,26 +158,21 @@ template bool LR11x0Interface::init() return res; }; -#if defined(TCXO_OPTIONAL) - // 1. XTAL, because a TCXO-first attempt hangs RadioLib's unbounded calibration wait on a module - // with no TCXO fitted, whereas XTAL fails fast and cleanly on a module that does have one - float attemptVoltage = 0; -#else - // 1. Whatever Vref the variant configured, which it declared unconditionally + // 1. XTAL first when probing (see TCXO_OPTIONAL_ENABLED), else the configured Vref. Not a + // ternary: cppcheck sees both branches as 0 when tcxoVoltage above already folded to it. float attemptVoltage = tcxoVoltage; -#endif + if (TCXO_OPTIONAL_ENABLED) + attemptVoltage = 0; int res = tryBegin(1, attemptVoltage); -#if defined(TCXO_OPTIONAL) - // 2. XTAL failed with the chip present, so fall back to the TCXO if the variant configured one - if (res != RADIOLIB_ERR_NONE && res != RADIOLIB_ERR_CHIP_NOT_FOUND && tcxoVoltage > 0) { + // 2. XTAL failed with the chip present, so fall back to the TCXO if one was configured + if (TCXO_OPTIONAL_ENABLED && res != RADIOLIB_ERR_NONE && res != RADIOLIB_ERR_CHIP_NOT_FOUND && tcxoVoltage > 0) { LOG_WARN("LR11x0 XTAL init failed (err %d), retry with TCXO Vref %f V", res, tcxoVoltage); attemptVoltage = tcxoVoltage; res = tryBegin(2, attemptVoltage); if (res == RADIOLIB_ERR_NONE) LOG_INFO("LR11x0 init success with TCXO Vref %f V", tcxoVoltage); } -#endif // 3. Some units need extra settling time, so give whichever oscillator we settled on one retry. // After a step 2 fallback that is a second TCXO attempt, which is where settling actually matters. @@ -249,6 +259,10 @@ template bool LR11x0Interface::init() bool dioAsRfSwitch = true; #elif defined(ARCH_PORTDUINO) bool dioAsRfSwitch = portduino_config.has_rfswitch_table; + if (dioAsRfSwitch) + buildRfSwitchTable(rfswitch_dio_pins, rfswitch_table, RFSW_MODE_COUNT + 1, lr11x0_switch_dio_nums, + lr11x0_switch_dio_consts, sizeof(lr11x0_switch_dio_nums) / sizeof(lr11x0_switch_dio_nums[0]), + lr11x0_rfswitch_mode_map); #else bool dioAsRfSwitch = false; #endif @@ -583,7 +597,4 @@ template int16_t LR11x0Interface::getCurrentRSSI() return (int16_t)round(rssi); } -// Don't leak the aliases into the files InterfacesTemplates.cpp includes after this one. -#undef rfswitch_dio_pins -#undef rfswitch_table #endif diff --git a/src/mesh/LR20x0Interface.cpp b/src/mesh/LR20x0Interface.cpp index c3a4c12059..49ebd216ab 100644 --- a/src/mesh/LR20x0Interface.cpp +++ b/src/mesh/LR20x0Interface.cpp @@ -21,8 +21,24 @@ #include "rfswitch.h" #elif ARCH_PORTDUINO #include "PortduinoGlue.h" -#define lr20x0_rfswitch_dio_pins portduino_config.rfswitch_dio_pins -#define lr20x0_rfswitch_table portduino_config.rfswitch_table + +// Switch-capable DIOs in slot order with this part's constants. +static const int8_t lr20x0_switch_dio_nums[] = {5, 6, 7, 8, 9, 10, 11}; +static const uint32_t lr20x0_switch_dio_consts[] = {RADIOLIB_LR2021_DIO5, RADIOLIB_LR2021_DIO6, RADIOLIB_LR2021_DIO7, + RADIOLIB_LR2021_DIO8, RADIOLIB_LR2021_DIO9, RADIOLIB_LR2021_DIO10, + RADIOLIB_LR2021_DIO11}; +static_assert(sizeof(lr20x0_switch_dio_nums) / sizeof(lr20x0_switch_dio_nums[0]) == + sizeof(lr20x0_switch_dio_consts) / sizeof(lr20x0_switch_dio_consts[0]), + "LR20x0 switch DIO numbers and constants must describe the same slots"); + +// This part has MODE_RX_HF and no MODE_TX_HP/MODE_GNSS/MODE_WIFI. +static const int32_t lr20x0_rfswitch_mode_map[RFSW_MODE_COUNT] = { + LR20x0::MODE_STBY, LR20x0::MODE_RX, LR20x0::MODE_TX, RFSW_MODE_UNSUPPORTED, + LR20x0::MODE_TX_HF, LR20x0::MODE_RX_HF, RFSW_MODE_UNSUPPORTED, RFSW_MODE_UNSUPPORTED, +}; + +static uint32_t lr20x0_rfswitch_dio_pins[Module::RFSWITCH_MAX_PINS]; +static Module::RfSwitchMode_t lr20x0_rfswitch_table[RFSW_MODE_COUNT + 1]; #else static const uint32_t lr20x0_rfswitch_dio_pins[] = {RADIOLIB_NC, RADIOLIB_NC, RADIOLIB_NC, RADIOLIB_NC, RADIOLIB_NC}; static const Module::RfSwitchMode_t lr20x0_rfswitch_table[] = { @@ -81,8 +97,16 @@ template bool LR20x0Interface::init() #endif #if ARCH_PORTDUINO - float tcxoVoltage = (float)portduino_config.dio3_tcxo_voltage / 1000; -// FIXME: correct logic to default to not using TCXO if no voltage is specified for LR20x0_DIO3_TCXO_VOLTAGE + // An explicit Vref wins; probing with none given tries the radio default first. + float tcxoVoltage; + if (portduino_config.dio3_tcxo_voltage > 0) + tcxoVoltage = (float)portduino_config.dio3_tcxo_voltage / 1000; + else if (TCXO_OPTIONAL_ENABLED) + tcxoVoltage = TCXO_OPTIONAL_DEFAULT_VOLTAGE; + else + tcxoVoltage = 0; + if (portduino_config.dio3_tcxo_voltage <= 0 && TCXO_OPTIONAL_ENABLED) + LOG_DEBUG("TCXO_OPTIONAL: no Lora.DIO3_TCXO_VOLTAGE set, trying default TCXO Vref %f V first", tcxoVoltage); #elif defined(LR2021_DIO3_TCXO_VOLTAGE) float tcxoVoltage = LR2021_DIO3_TCXO_VOLTAGE; LOG_DEBUG("LR2021_DIO3_TCXO_VOLTAGE defined, DIO3 as TCXO Vref %f V", LR2021_DIO3_TCXO_VOLTAGE); @@ -106,6 +130,18 @@ template bool LR20x0Interface::init() #elif defined(IRQ_DIO_NUM) lora.irqDioNum = IRQ_DIO_NUM; LOG_DEBUG("Set irqDioNum %d", lora.irqDioNum); +#elif defined(ARCH_PORTDUINO) + // Unset keeps RadioLib's default of DIO5, which many carriers also drive as a switch line. The + // range is checked again here because a DIO the radio cannot drive is a silently dead receiver. + if (portduino_config.irq_dio_num < 0) { + LOG_DEBUG("Use default irqDioNum %d", lora.irqDioNum); + } else if (portduino_config.irq_dio_num >= kLr20x0IrqDioMin && portduino_config.irq_dio_num <= kLr20x0IrqDioMax) { + lora.irqDioNum = portduino_config.irq_dio_num; + LOG_DEBUG("Set irqDioNum %d from config", lora.irqDioNum); + } else { + LOG_WARN("Config irqDioNum %d outside DIO%d-DIO%d, using default irqDioNum %d", portduino_config.irq_dio_num, + kLr20x0IrqDioMin, kLr20x0IrqDioMax, lora.irqDioNum); + } #else LOG_DEBUG("Use default irqDioNum %d", lora.irqDioNum); #endif @@ -140,16 +176,14 @@ template bool LR20x0Interface::init() res = lora.begin(getFreq(), bw, sf, cr, syncWord, power, preambleLength, tcxoVoltage); } -#if defined(TCXO_OPTIONAL) // If init failed for any reason other than chip not found, retry without TCXO (XTAL mode) - if (res != RADIOLIB_ERR_NONE && res != RADIOLIB_ERR_CHIP_NOT_FOUND && tcxoVoltage > 0) { + if (TCXO_OPTIONAL_ENABLED && res != RADIOLIB_ERR_NONE && res != RADIOLIB_ERR_CHIP_NOT_FOUND && tcxoVoltage > 0) { LOG_WARN("LR20x0 init failed with TCXO Vref %f V (err %d), retry without TCXO", tcxoVoltage, res); tcxoVoltage = 0; res = lora.begin(getFreq(), bw, sf, cr, syncWord, power, preambleLength, tcxoVoltage); if (res == RADIOLIB_ERR_NONE) LOG_INFO("LR20x0 init success without TCXO (XTAL mode)"); } -#endif // \todo Display actual typename of the adapter, not just `LR20x0` LOG_INFO("LR20x0 init result %d", res); @@ -199,6 +233,10 @@ template bool LR20x0Interface::init() bool dioAsRfSwitch = true; #elif defined(ARCH_PORTDUINO) bool dioAsRfSwitch = portduino_config.has_rfswitch_table; + if (dioAsRfSwitch) + buildRfSwitchTable(lr20x0_rfswitch_dio_pins, lr20x0_rfswitch_table, RFSW_MODE_COUNT + 1, lr20x0_switch_dio_nums, + lr20x0_switch_dio_consts, sizeof(lr20x0_switch_dio_nums) / sizeof(lr20x0_switch_dio_nums[0]), + lr20x0_rfswitch_mode_map); #else bool dioAsRfSwitch = false; #endif @@ -356,11 +394,17 @@ template bool LR20x0Interface::fullBegin(float freq) #endif #if ARCH_PORTDUINO - float tcxoVoltage = (float)portduino_config.dio3_tcxo_voltage / 1000; + float tcxoVoltage; + if (portduino_config.dio3_tcxo_voltage > 0) + tcxoVoltage = (float)portduino_config.dio3_tcxo_voltage / 1000; + else if (TCXO_OPTIONAL_ENABLED) + tcxoVoltage = TCXO_OPTIONAL_DEFAULT_VOLTAGE; + else + tcxoVoltage = 0; #elif defined(LR2021_DIO3_TCXO_VOLTAGE) float tcxoVoltage = LR2021_DIO3_TCXO_VOLTAGE; #elif defined(TCXO_OPTIONAL) - float tcxoVoltage = 1.6f; + float tcxoVoltage = TCXO_OPTIONAL_DEFAULT_VOLTAGE; #else float tcxoVoltage = 0; #endif @@ -373,13 +417,11 @@ template bool LR20x0Interface::fullBegin(float freq) delay(100); res = lora.begin(freq, bw, sf, cr, syncWord, power, preambleLength, tcxoVoltage); } -#if defined(TCXO_OPTIONAL) - if (res != RADIOLIB_ERR_NONE && res != RADIOLIB_ERR_CHIP_NOT_FOUND && tcxoVoltage > 0) { + if (TCXO_OPTIONAL_ENABLED && res != RADIOLIB_ERR_NONE && res != RADIOLIB_ERR_CHIP_NOT_FOUND && tcxoVoltage > 0) { LOG_WARN("LR20x0 band-hop begin TCXO failed (%s%d), retry without TCXO", radioLibErr, res); tcxoVoltage = 0; res = lora.begin(freq, bw, sf, cr, syncWord, power, preambleLength, tcxoVoltage); } -#endif if (res != RADIOLIB_ERR_NONE) { LOG_ERROR("LR20x0 band-hop begin %s%d", radioLibErr, res); RECORD_CRITICALERROR(meshtastic_CriticalErrorCode_INVALID_RADIO_SETTING); @@ -682,8 +724,6 @@ template int16_t LR20x0Interface::getCurrentRSSI() return (int16_t)round(rssi); } -// Don't leak the aliases into the files InterfacesTemplates.cpp includes after this one. -#undef lr20x0_rfswitch_dio_pins -#undef lr20x0_rfswitch_table +// Don't leak the alias into the files InterfacesTemplates.cpp includes after this one. #undef LR20x0 #endif diff --git a/src/mesh/RadioLibInterface.h b/src/mesh/RadioLibInterface.h index 6dcd0876f7..a811b085b7 100644 --- a/src/mesh/RadioLibInterface.h +++ b/src/mesh/RadioLibInterface.h @@ -37,6 +37,19 @@ class LockingArduinoHal : public ArduinoHal #endif }; +// TCXO_OPTIONAL (variant define) or Lora.TCXO_OPTIONAL (Portduino YAML): probe for a TCXO and +// fall back to the XTAL. LR11x0 tries XTAL first - TCXO-first hangs RadioLib's calibration wait. +#if ARCH_PORTDUINO +#define TCXO_OPTIONAL_ENABLED (portduino_config.tcxo_optional) +#elif defined(TCXO_OPTIONAL) +#define TCXO_OPTIONAL_ENABLED true +#else +#define TCXO_OPTIONAL_ENABLED false +#endif + +// RadioLib's own default Vref, for a probe with no explicit voltage configured. +#define TCXO_OPTIONAL_DEFAULT_VOLTAGE 1.6f + #if defined(USE_STM32WLx) /** * A wrapper for the RadioLib STM32WLx_Module class, that doesn't connect any pins as they are virtual diff --git a/src/mesh/SX126xInterface.cpp b/src/mesh/SX126xInterface.cpp index ab1f5eb38b..58e808a104 100644 --- a/src/mesh/SX126xInterface.cpp +++ b/src/mesh/SX126xInterface.cpp @@ -70,16 +70,31 @@ template bool SX126xInterface::init() #endif #if ARCH_PORTDUINO - tcxoVoltage = (float)portduino_config.dio3_tcxo_voltage / 1000; + // An explicit Vref wins; probing with none given tries the radio default first. + bool tcxoVoltageExplicit = portduino_config.dio3_tcxo_voltage > 0; + if (tcxoVoltageExplicit) + tcxoVoltage = (float)portduino_config.dio3_tcxo_voltage / 1000; + else if (TCXO_OPTIONAL_ENABLED) + tcxoVoltage = TCXO_OPTIONAL_DEFAULT_VOLTAGE; + else + tcxoVoltage = 0; if (portduino_config.lora_sx126x_ant_sw_pin.pin != RADIOLIB_NC) { digitalWrite(portduino_config.lora_sx126x_ant_sw_pin.pin, HIGH); pinMode(portduino_config.lora_sx126x_ant_sw_pin.pin, OUTPUT); } -#endif + // The knob here is the YAML key, not the variant define the other branch reports. + if (tcxoVoltage == 0.0) + LOG_DEBUG("Lora.DIO3_TCXO_VOLTAGE not set, DIO3 not used as TCXO Vref"); + else if (!tcxoVoltageExplicit) + LOG_DEBUG("TCXO_OPTIONAL: no Vref configured, probing default TCXO Vref %f V on DIO3", tcxoVoltage); + else + LOG_DEBUG("Lora.DIO3_TCXO_VOLTAGE set, DIO3 as TCXO Vref %f V", tcxoVoltage); +#else if (tcxoVoltage == 0.0) LOG_DEBUG("SX126X_DIO3_TCXO_VOLTAGE not defined, DIO3 not used as TCXO Vref"); else LOG_DEBUG("SX126X_DIO3_TCXO_VOLTAGE defined, DIO3 as TCXO Vref %f V", tcxoVoltage); +#endif setTransmitEnable(false); RadioLibInterface::init(); @@ -109,6 +124,24 @@ template bool SX126xInterface::reinitChip() int res = lora.begin(getFreq(), bw, sf, cr, syncWord, power, preambleLength, tcxoVoltage, useRegulatorLDO); + // Chip answered but would not start on the TCXO: retry on the XTAL. CHIP_NOT_FOUND is a + // wiring or SPI fault, where a second attempt only hides it. Portduino only - an embedded + // board gets this from the second interface instance in initLoRa()'s ladder. + // TODO: consider deferring to RadioLib, which has autocorrected this itself since 7.5.0: + // SX126x::modSetup() retries config() on the XTAL when begin() fails with SPI_CMD_FAILED and + // XOSC_START_ERR, so the ordinary "TCXO configured, XTAL fitted" case never reaches here and + // what does is mostly invalid settings. Narrowing this to SPI_CMD_TIMEOUT - the oscillator + // symptom RadioLib's condition misses - would keep the cover and drop the misdiagnosis. +#if ARCH_PORTDUINO + if (TCXO_OPTIONAL_ENABLED && res != RADIOLIB_ERR_NONE && res != RADIOLIB_ERR_CHIP_NOT_FOUND && tcxoVoltage > 0) { + LOG_WARN("SX126x init failed with TCXO Vref %f V (err %d), retrying without TCXO", tcxoVoltage, res); + tcxoVoltage = 0; + res = lora.begin(getFreq(), bw, sf, cr, syncWord, power, preambleLength, tcxoVoltage, useRegulatorLDO); + if (res == RADIOLIB_ERR_NONE) + LOG_INFO("SX126x init success without TCXO (XTAL mode)"); + } +#endif + #ifdef SX126X_PA_RAMP_US // Set custom PA ramp time for boards requiring longer stabilization (e.g., T-Beam 1W needs >800us) if (res == RADIOLIB_ERR_NONE) { diff --git a/src/platform/portduino/ConfigCheck.cpp b/src/platform/portduino/ConfigCheck.cpp index 54037bf359..d03ca7c09a 100644 --- a/src/platform/portduino/ConfigCheck.cpp +++ b/src/platform/portduino/ConfigCheck.cpp @@ -47,10 +47,15 @@ const std::map> &schema() "spiSpeed", "DIO2_AS_RF_SWITCH", "DIO3_TCXO_VOLTAGE", + "TCXO_OPTIONAL", "Enable_Pins", "rfswitch_table", + "IRQ_DIO_NUM", + "LR2021_IRQ_DIO_NUM", "LR1110_MAX_POWER", "LR1120_MAX_POWER", + "LR2021_MAX_POWER", + "LR2021_MAX_POWER_HF", "RF95_MAX_POWER", "SX126X_MAX_POWER", "SX128X_MAX_POWER", @@ -100,10 +105,89 @@ const std::set kHub75Keys = { "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"}; +// Names the radio a finding is judged against; defined with the merged-config checks. +std::string moduleName(); -const std::set kRfSwitchPins = {"DIO5", "DIO6", "DIO7", "DIO8", "DIO10"}; +// Union of every family's mode names: membership means the name is spelled correctly. +// Whether this radio can act on it is a separate, module-aware question - see modesFor(). +// Function-local static, not a namespace-scope global: kRfSwitchModeNames lives in another TU, +// and lazy first-use init sidesteps any cross-TU static-initialization-order question. +const std::set &kRfSwitchModes() +{ + static const std::set s = [] { + std::set s; + for (int m = 0; m < RFSW_MODE_COUNT; m++) + s.insert(kRfSwitchModeNames[m].name); + return s; + }(); + return s; +} + +// Likewise the union of both families' switch-capable DIOs. +const std::set kRfSwitchPins = {"DIO5", "DIO6", "DIO7", "DIO8", "DIO9", "DIO10", "DIO11"}; + +// RadioLib's default when nothing sets LR2021::irqDioNum. Mirrored rather than included, to +// keep the radio headers out of this file. +const int kLr20x0DefaultIrqDio = 5; + +// Mode names this module can apply, empty if it never applies a table at all - there is then +// nothing module-specific to say about the rows, only that the whole table is inert. +// An unresolved "auto" is reported against the union of both families' modes, since narrowing +// to either subset would flag the other's valid modes. +std::set modesFor(lora_module_enum module) +{ + if (module == use_autoconf) + return kRfSwitchModes(); + std::set s; + if (!moduleUsesRfSwitchTable(module)) + return s; + if (module == use_lr2021) { + for (int m = 0; m < RFSW_MODE_COUNT; m++) + if (m != RFSW_TX_HP && m != RFSW_GNSS && m != RFSW_WIFI) + s.insert(kRfSwitchModeNames[m].name); + } else { + for (int m = 0; m < RFSW_MODE_COUNT; m++) + if (m != RFSW_RX_HF) + s.insert(kRfSwitchModeNames[m].name); + } + return s; +} + +std::set pinsFor(lora_module_enum module) +{ + std::set s; + size_t count = 0; + const int8_t *dios = rfSwitchDiosFor(module, &count); + for (size_t i = 0; i < count; i++) + s.insert("DIO" + std::to_string(dios[i])); + return s; +} + +// Families whose driver can probe for a TCXO. RF95, SX128x and the simulated radio have no +// TCXO reference, so Lora.TCXO_OPTIONAL is inert on those rather than merely unnecessary. +bool moduleSupportsTcxoProbe(lora_module_enum module) +{ + switch (module) { + case use_sx1262: + case use_sx1268: + case use_llcc68: + case use_lr1110: + case use_lr1120: + case use_lr1121: + case use_lr2021: + return true; + default: + return false; + } +} + +std::string joinNames(const std::set &names) +{ + std::string out; + for (const auto &name : names) + out += (out.empty() ? "" : ", ") + name; + return out; +} // 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. @@ -116,7 +200,7 @@ const std::map> &keyOwners() 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) + for (const auto &mode : kRfSwitchModes()) m[mode].insert("Lora.rfswitch_table"); m["pins"].insert("Lora.rfswitch_table"); return m; @@ -295,12 +379,20 @@ void checkRfSwitchTable(const std::string &file, const YAML::Node &table, std::v findings.push_back({kError, file, lineOf(pins), "Lora.rfswitch_table.pins must be a list"}); } else { pinCount = pins.size(); + const std::set valid = pinsFor(portduino_config.lora_module); for (const auto &pin : pins) { const std::string name = pin.as(""); - if (!kRfSwitchPins.count(name)) + 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"}); + "Lora.rfswitch_table.pins: '" + name + "' is not a recognised pin. Valid values are " + + joinNames(kRfSwitchPins)}); + } else if (!valid.empty() && !valid.count(name)) { + // Spelled correctly, but not a switch control here: the slot is left + // unconnected and every level in its column dropped. + findings.push_back({kError, file, lineOf(pin), + "Lora.rfswitch_table.pins: '" + name + "' is not an RF switch pin on " + moduleName() + + ", so that column is never driven. It has " + joinNames(valid)}); + } } if (pinCount > 5) findings.push_back( @@ -311,14 +403,21 @@ void checkRfSwitchTable(const std::string &file, const YAML::Node &table, std::v findings.push_back({kError, file, lineOf(table), "Lora.rfswitch_table has no 'pins' list, so no switch pins are driven"}); } + const std::set validModes = modesFor(portduino_config.lora_module); + for (const auto &entry : table) { const std::string key = entry.first.as(""); if (key == "pins") continue; - if (!kRfSwitchModes.count(key)) { + if (!kRfSwitchModes().count(key)) { findings.push_back({kError, file, lineOf(entry.first), "unknown key 'Lora.rfswitch_table." + key + "'"}); continue; } + // A real mode name, but not one this part has, so the row is never applied. Empty means + // the module applies no table, and singling out one row would imply the rest are used. + if (!validModes.empty() && !validModes.count(key)) + findings.push_back({kWarn, file, lineOf(entry.first), + "Lora.rfswitch_table." + key + " is not a mode " + moduleName() + " has, so the row is ignored"}); const YAML::Node &row = entry.second; if (!row.IsSequence()) { findings.push_back({kError, file, lineOf(row), "Lora.rfswitch_table." + key + " must be a list"}); @@ -339,17 +438,18 @@ void checkRfSwitchTable(const std::string &file, const YAML::Node &table, std::v // 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) + // Only worth saying once the radio is known: under "auto" the union would advise adding + // rows for modes the part turns out not to have, and a module that applies no table needs + // no rows at all. Either way the advice would be wrong, so say nothing. + if (validModes.empty() || portduino_config.lora_module == use_autoconf) + return; + std::set missing; + for (const auto &mode : validModes) 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"}); - } + missing.insert(mode); + if (!missing.empty()) + findings.push_back({kInfo, file, lineOf(table), + "Lora.rfswitch_table omits " + joinNames(missing) + "; those modes default to all pins LOW"}); } // --------------------------------------------------------------------------- @@ -375,8 +475,13 @@ const std::map &valueSpecs() {"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.TCXO_OPTIONAL", {kBool, false}}, {"Lora.LR1110_MAX_POWER", {kInt, false}}, {"Lora.LR1120_MAX_POWER", {kInt, false}}, + {"Lora.LR2021_MAX_POWER", {kInt, false}}, + {"Lora.LR2021_MAX_POWER_HF", {kInt, false}}, + {"Lora.IRQ_DIO_NUM", {kInt, false}}, + {"Lora.LR2021_IRQ_DIO_NUM", {kInt, false}}, {"Lora.RF95_MAX_POWER", {kInt, false}}, {"Lora.SX126X_MAX_POWER", {kInt, false}}, {"Lora.SX128X_MAX_POWER", {kInt, false}}, @@ -537,6 +642,32 @@ void checkValueType(const std::string &file, const std::string &path, const YAML ", so it is silently replaced by the default and the setting does nothing"}); } +// The spelling the earlier LR2021 branches used, read only when IRQ_DIO_NUM is absent. Held as a +// constant so no comparison puts the name next to a variable called `key`, which reads to secret +// scanners as an assignment of a credential. +const char kLegacyIrqDioName[] = "LR2021_IRQ_DIO_NUM"; + +bool isIrqDioName(const std::string &name) +{ + return name == "IRQ_DIO_NUM" || name == kLegacyIrqDioName; +} + +// Out of range is discarded when the config is read, so the merged view cannot tell a typo from +// an absent key. Judged here, per file, where the offending line is still known. +void checkIrqDioNum(const std::string &file, const std::string &key, const YAML::Node &value, std::vector &findings) +{ + if (!converts(value, kInt)) + return; // checkValueType() already reported it + + const int dio = value.as(); + if (dio < kLr20x0IrqDioMin || dio > kLr20x0IrqDioMax) + findings.push_back({kError, file, lineOf(value), + "Lora." + key + " is " + std::to_string(dio) + ", outside DIO" + std::to_string(kLr20x0IrqDioMin) + + "-DIO" + std::to_string(kLr20x0IrqDioMax) + + ". It is ignored, and the radio raises its interrupt on DIO" + + std::to_string(kLr20x0DefaultIrqDio) + " instead"}); +} + // 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) @@ -685,6 +816,12 @@ void checkSection(const std::string &file, const std::string §ion, const YAM checkLoraModule(file, value, findings); } else if (section == "Lora" && key == "TX_GAIN_LORA") { checkTxGain(file, value, findings); + } else if (section == "Lora" && isIrqDioName(key)) { + checkIrqDioNum(file, key, value, findings); + if (key == kLegacyIrqDioName && body["IRQ_DIO_NUM"]) + findings.push_back({kWarn, file, lineOf(entry.first), + "Lora.LR2021_IRQ_DIO_NUM is the older spelling of Lora.IRQ_DIO_NUM and is only read " + "when that key is absent, so this line does nothing"}); } else if (section == "Display" && key == "HUB75") { if (value.IsMap()) for (const auto &hub : value) { @@ -822,18 +959,6 @@ void checkCrossFileOverlap(const PathIndex &paths, const std::map &findings) ignored + " are read but never used"}); } - if (isLR11xx(portduino_config.lora_module) && !portduino_config.has_rfswitch_table) + // "auto" is excluded by moduleUsesRfSwitchTable(): the module has not been probed yet, so + // there is nothing to judge the absence against. + if (moduleUsesRfSwitchTable(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"}); + " but no Lora.rfswitch_table is set, so setRfSwitchTable() is never called. Most modules " + "of this family 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"}); + if (!moduleUsesRfSwitchTable(portduino_config.lora_module) && portduino_config.lora_module != use_autoconf && + 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 and LR20x0 radios"}); + + // One pin cannot be both the interrupt output and a switch control. begin() exercises only + // SPI and BUSY, so the radio reports init success and then never receives anything. + bool irqDioCollides = false; + if (portduino_config.lora_module == use_lr2021 && portduino_config.has_rfswitch_table) { + const int irqDio = portduino_config.irq_dio_num >= 0 ? portduino_config.irq_dio_num : kLr20x0DefaultIrqDio; + for (int i = 0; i < 5; i++) { + if (portduino_config.rfswitch_dio_num[i] != irqDio) + continue; + const std::string dio = "DIO" + std::to_string(irqDio); + irqDioCollides = true; + if (portduino_config.irq_dio_num >= 0) + findings.push_back({kError, merged, 0, + "Lora.IRQ_DIO_NUM is " + dio + + ", which Lora.rfswitch_table.pins also drives as an RF switch control. One pin " + "cannot be both the interrupt output and a switch line"}); + else + findings.push_back({kError, merged, 0, + "no Lora.IRQ_DIO_NUM is set, so the radio raises its interrupt on " + dio + + " by default -- but Lora.rfswitch_table.pins drives " + dio + + " as an RF switch control. Set Lora.IRQ_DIO_NUM to a DIO the table does not use " + "(the radio will otherwise report init success and receive nothing)"}); + break; + } + } + + // Leaving it unset is legitimate as long as nothing else wants DIO5, but the default is worth + // stating: the collision above is what it turns into once a switch table arrives. + if (portduino_config.lora_module == use_lr2021 && portduino_config.irq_dio_num < 0 && !irqDioCollides) + findings.push_back({kInfo, merged, 0, + "no Lora.IRQ_DIO_NUM is set, so the radio raises its interrupt on DIO" + + std::to_string(kLr20x0DefaultIrqDio) + " (the RadioLib default)"}); + + // The probe belongs to the radio driver, so on a part with no TCXO reference the key is + // read, stored and inert. + if (portduino_config.tcxo_optional && portduino_config.lora_module != use_autoconf && + !moduleSupportsTcxoProbe(portduino_config.lora_module)) + findings.push_back({kWarn, merged, 0, + "Lora.TCXO_OPTIONAL is set but Module is " + moduleName() + + ", which has no TCXO reference to probe for, so the setting does nothing"}); + + // Writing the voltage out as false asks for DIO3 to be left alone; the probe drives it + // anyway. Both keys are honoured exactly as documented, which is what makes it confusing. + if (portduino_config.tcxo_optional && portduino_config.dio3_tcxo_voltage_disabled && + moduleSupportsTcxoProbe(portduino_config.lora_module)) + findings.push_back({kWarn, merged, 0, + "Lora.DIO3_TCXO_VOLTAGE is off, which asks for DIO3 not to be driven, but " + "Lora.TCXO_OPTIONAL probes DIO3 at the radio default before falling back to the " + "crystal. The probe wins. Drop TCXO_OPTIONAL to keep DIO3 idle, or drop " + "DIO3_TCXO_VOLTAGE to let the probe pick"}); // Either way -- the old uncaught filesystem_error abort or today's clean exit -- the files // meant to configure the radio are not being loaded. @@ -1004,18 +1177,32 @@ void printSummary() 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"; + if (portduino_config.tcxo_optional) { + // Name the Vref actually tried: with no explicit voltage the driver uses the radio + // default rather than skipping the TCXO attempt. + std::cout << " TCXO probe : yes, " + << (portduino_config.dio3_tcxo_voltage ? std::to_string(portduino_config.dio3_tcxo_voltage) + " mV" + : std::string("1600 mV (radio default)")) + << " and XTAL\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. + // setRfSwitchTable() is called for an LR11xx and an LR20x0, so "not set" is no gap on any + // other radio; "auto" has not resolved 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)) + else if (moduleUsesRfSwitchTable(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"; + if (portduino_config.lora_module == use_lr2021) + std::cout << " IRQ DIO : " + << (portduino_config.irq_dio_num >= 0 ? "DIO" + std::to_string(portduino_config.irq_dio_num) + : "DIO" + std::to_string(kLr20x0DefaultIrqDio) + " (radio default)") + << "\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"; diff --git a/src/platform/portduino/PortduinoGlue.cpp b/src/platform/portduino/PortduinoGlue.cpp index f7aa2726ae..8fe452aa01 100644 --- a/src/platform/portduino/PortduinoGlue.cpp +++ b/src/platform/portduino/PortduinoGlue.cpp @@ -15,6 +15,8 @@ #include #include #include +#include +#include #include #include #include @@ -60,6 +62,7 @@ bool portduinoWindowsPrimaryMac(uint8_t *dmac); portduino_config_struct portduino_config; portduino_status_struct portduino_status; + std::ofstream traceFile; std::ofstream JSONFile; std::unique_ptr ch341Hal; @@ -71,6 +74,77 @@ bool configCheck = false; // Every config file we attempted to load, in load order, for --check to report on. std::vector attemptedConfigFiles; +// --------------------------------------------------------------------------- +// RF switch table: chip-neutral storage, per-part translation +// --------------------------------------------------------------------------- + +const RfSwitchModeName kRfSwitchModeNames[RFSW_MODE_COUNT] = { + {"MODE_STBY", RFSW_STBY}, {"MODE_RX", RFSW_RX}, {"MODE_TX", RFSW_TX}, {"MODE_TX_HP", RFSW_TX_HP}, + {"MODE_TX_HF", RFSW_TX_HF}, {"MODE_RX_HF", RFSW_RX_HF}, {"MODE_GNSS", RFSW_GNSS}, {"MODE_WIFI", RFSW_WIFI}, +}; + +const int8_t kLr11x0SwitchDios[5] = {5, 6, 7, 8, 10}; +const int8_t kLr20x0SwitchDios[7] = {5, 6, 7, 8, 9, 10, 11}; + +const int8_t *rfSwitchDiosFor(lora_module_enum module, size_t *count) +{ + switch (module) { + case use_lr1110: + case use_lr1120: + case use_lr1121: + *count = sizeof(kLr11x0SwitchDios) / sizeof(kLr11x0SwitchDios[0]); + return kLr11x0SwitchDios; + case use_lr2021: + *count = sizeof(kLr20x0SwitchDios) / sizeof(kLr20x0SwitchDios[0]); + return kLr20x0SwitchDios; + default: + *count = 0; + return nullptr; + } +} + +bool moduleUsesRfSwitchTable(lora_module_enum module) +{ + size_t count = 0; + return rfSwitchDiosFor(module, &count) != nullptr; +} + +size_t buildRfSwitchTable(uint32_t (&pins)[Module::RFSWITCH_MAX_PINS], Module::RfSwitchMode_t *table, size_t tableCapacity, + const int8_t *dioNumbers, const uint32_t *pinConsts, size_t dioCount, const int32_t *modeMap) +{ + for (size_t i = 0; i < Module::RFSWITCH_MAX_PINS; i++) + pins[i] = RADIOLIB_NC; + + // A DIO this part cannot use leaves the slot at RADIOLIB_NC, so mode rows still line up. + uint8_t usableSlots = 0; + for (size_t i = 0; i < Module::RFSWITCH_MAX_PINS; i++) { + const int8_t dio = portduino_config.rfswitch_dio_num[i]; + if (dio < 0) + continue; + for (size_t s = 0; s < dioCount; s++) { + if (dioNumbers[s] == dio) { + pins[i] = pinConsts[s]; + usableSlots |= (uint8_t)(1u << i); + break; + } + } + } + + size_t rows = 0; + for (int m = 0; m < RFSW_MODE_COUNT && rows + 1 < tableCapacity; m++) { + if (modeMap[m] == RFSW_MODE_UNSUPPORTED) + continue; + table[rows].mode = (uint32_t)modeMap[m]; + const uint8_t high = (uint8_t)(portduino_config.rfswitch_mode_high[m] & usableSlots); + for (size_t i = 0; i < Module::RFSWITCH_MAX_PINS; i++) + table[rows].values[i] = (high & (1u << i)) ? HIGH : LOW; + rows++; + } + if (rows < tableCapacity) + table[rows++] = END_OF_MODE_TABLE; + return rows; +} + const char *argp_program_version = optstr(APP_VERSION); char stdoutBuffer[512]; @@ -984,6 +1058,12 @@ bool loadConfig(const char *configPath) if (portduino_config.dio3_tcxo_voltage == 0 && yamlConfig["Lora"]["DIO3_TCXO_VOLTAGE"].as(false)) { portduino_config.dio3_tcxo_voltage = 1800; // default millivolts for "true" } + // A written-out false or 0 asks for DIO3 to be left alone, which stores the same as + // an absent key. Kept apart so --check can see it contradict TCXO_OPTIONAL. + portduino_config.dio3_tcxo_voltage_disabled = + yamlConfig["Lora"]["DIO3_TCXO_VOLTAGE"] && portduino_config.dio3_tcxo_voltage == 0; + // Try both oscillators rather than requiring the user to know which is fitted. + portduino_config.tcxo_optional = yamlConfig["Lora"]["TCXO_OPTIONAL"].as(false); // backwards API compatibility and to globally set gpiochip once portduino_config.lora_default_gpiochip = yamlConfig["Lora"]["gpiochip"].as(0); @@ -1027,44 +1107,55 @@ bool loadConfig(const char *configPath) } if (yamlConfig["Lora"]["rfswitch_table"]) { portduino_config.has_rfswitch_table = true; - portduino_config.rfswitch_table[0].mode = LR11x0::MODE_STBY; - portduino_config.rfswitch_table[1].mode = LR11x0::MODE_RX; - portduino_config.rfswitch_table[2].mode = LR11x0::MODE_TX; - portduino_config.rfswitch_table[3].mode = LR11x0::MODE_TX_HP; - portduino_config.rfswitch_table[4].mode = LR11x0::MODE_TX_HF; - portduino_config.rfswitch_table[5].mode = LR11x0::MODE_GNSS; - portduino_config.rfswitch_table[6].mode = LR11x0::MODE_WIFI; - portduino_config.rfswitch_table[7] = END_OF_MODE_TABLE; + // A later file's table fully replaces an earlier one, matching "last file wins" + // for every other Lora: key, rather than leaving omitted pins/modes as carryover. + for (int i = 0; i < 5; i++) + portduino_config.rfswitch_dio_num[i] = -1; + for (int m = 0; m < RFSW_MODE_COUNT; m++) { + portduino_config.rfswitch_mode_present[m] = false; + portduino_config.rfswitch_mode_high[m] = 0; + } + const YAML::Node table = yamlConfig["Lora"]["rfswitch_table"]; + // Store the DIO number as written; the slot it maps to is per-radio. Anything + // not spelled exactly "DIO" (trailing junk included) leaves the slot unused. for (int i = 0; i < 5; i++) { + const std::string name = table["pins"][i].as(""); + int dioNum = 0; + if (sscanf(name.c_str(), "DIO%d", &dioNum) == 1 && dioNum >= 0 && dioNum <= INT8_MAX && + name == "DIO" + std::to_string(dioNum)) + portduino_config.rfswitch_dio_num[i] = (int8_t)dioNum; + } - // set up the pin array first - if (yamlConfig["Lora"]["rfswitch_table"]["pins"][i].as("") == "DIO5") - portduino_config.rfswitch_dio_pins[i] = RADIOLIB_LR11X0_DIO5; - if (yamlConfig["Lora"]["rfswitch_table"]["pins"][i].as("") == "DIO6") - portduino_config.rfswitch_dio_pins[i] = RADIOLIB_LR11X0_DIO6; - if (yamlConfig["Lora"]["rfswitch_table"]["pins"][i].as("") == "DIO7") - portduino_config.rfswitch_dio_pins[i] = RADIOLIB_LR11X0_DIO7; - if (yamlConfig["Lora"]["rfswitch_table"]["pins"][i].as("") == "DIO8") - portduino_config.rfswitch_dio_pins[i] = RADIOLIB_LR11X0_DIO8; - if (yamlConfig["Lora"]["rfswitch_table"]["pins"][i].as("") == "DIO10") - portduino_config.rfswitch_dio_pins[i] = RADIOLIB_LR11X0_DIO10; - - // now fill in the table - if (yamlConfig["Lora"]["rfswitch_table"]["MODE_STBY"][i].as("") == "HIGH") - portduino_config.rfswitch_table[0].values[i] = HIGH; - if (yamlConfig["Lora"]["rfswitch_table"]["MODE_RX"][i].as("") == "HIGH") - portduino_config.rfswitch_table[1].values[i] = HIGH; - if (yamlConfig["Lora"]["rfswitch_table"]["MODE_TX"][i].as("") == "HIGH") - portduino_config.rfswitch_table[2].values[i] = HIGH; - if (yamlConfig["Lora"]["rfswitch_table"]["MODE_TX_HP"][i].as("") == "HIGH") - portduino_config.rfswitch_table[3].values[i] = HIGH; - if (yamlConfig["Lora"]["rfswitch_table"]["MODE_TX_HF"][i].as("") == "HIGH") - portduino_config.rfswitch_table[4].values[i] = HIGH; - if (yamlConfig["Lora"]["rfswitch_table"]["MODE_GNSS"][i].as("") == "HIGH") - portduino_config.rfswitch_table[5].values[i] = HIGH; - if (yamlConfig["Lora"]["rfswitch_table"]["MODE_WIFI"][i].as("") == "HIGH") - portduino_config.rfswitch_table[6].values[i] = HIGH; + for (int m = 0; m < RFSW_MODE_COUNT; m++) { + const YAML::Node row = table[kRfSwitchModeNames[m].name]; + if (!row) + continue; + portduino_config.rfswitch_mode_present[m] = true; + // Fresh mask per row, not OR'd onto whatever was there - a re-parse must be able + // to clear a slot back to LOW, not just add HIGH bits. + uint8_t high = 0; + for (int i = 0; i < 5; i++) + if (row[i].as("") == "HIGH") + high |= (uint8_t)(1u << i); + portduino_config.rfswitch_mode_high[m] = high; + } + } + // IRQ DIO for the LR20x0 driver; unset leaves RadioLib's default of DIO5. LR2021_IRQ_DIO_NUM + // is the older spelling, read only when the generic key is absent. + const char *irqDioKey = yamlConfig["Lora"]["IRQ_DIO_NUM"] + ? "IRQ_DIO_NUM" + : (yamlConfig["Lora"]["LR2021_IRQ_DIO_NUM"] ? "LR2021_IRQ_DIO_NUM" : nullptr); + if (irqDioKey) { + const int irqDio = yamlConfig["Lora"][irqDioKey].as(-1); + if (irqDio >= kLr20x0IrqDioMin && irqDio <= kLr20x0IrqDioMax) + portduino_config.irq_dio_num = irqDio; + else { + // Back to unset, or a valid value from an earlier config.d file would survive the + // warning and be used in place of the default it promises. + portduino_config.irq_dio_num = -1; + LOG_WARN("Lora.%s is %d, outside DIO%d-DIO%d; ignoring it and using the radio default", irqDioKey, irqDio, + kLr20x0IrqDioMin, kLr20x0IrqDioMax); } } } diff --git a/src/platform/portduino/PortduinoGlue.h b/src/platform/portduino/PortduinoGlue.h index 2072455671..3e77894f57 100644 --- a/src/platform/portduino/PortduinoGlue.h +++ b/src/platform/portduino/PortduinoGlue.h @@ -52,6 +52,44 @@ enum lora_module_enum { use_lr2021 }; +// RF switch modes as the YAML names them; each family supports a different subset. The neutral +// id lets one parser serve both, each interface translating to its own OpMode_t. +enum RfSwitchModeId { RFSW_STBY, RFSW_RX, RFSW_TX, RFSW_TX_HP, RFSW_TX_HF, RFSW_RX_HF, RFSW_GNSS, RFSW_WIFI, RFSW_MODE_COUNT }; + +// A mode the part does not have, for buildRfSwitchTable()'s modeMap. +#define RFSW_MODE_UNSUPPORTED (-1) + +struct RfSwitchModeName { + const char *name; + RfSwitchModeId id; +}; + +// YAML spelling of every mode, in RfSwitchModeId order. +extern const RfSwitchModeName kRfSwitchModeNames[RFSW_MODE_COUNT]; + +// The switch-capable DIO numbers of each family, parallel to that interface's pin-constant +// array: a configured DIO is looked up by value here, and the index found selects the +// constant. Not a slot mapping - Lora.rfswitch_table has 5 pin slots, the LR20x0 has 7 DIOs. +extern const int8_t kLr11x0SwitchDios[5]; +extern const int8_t kLr20x0SwitchDios[7]; + +// DIOs an LR20x0 can raise its interrupt on. Anything outside this is refused when the config is +// read and again before it reaches RadioLib, which would otherwise route the IRQ into the void. +constexpr int kLr20x0IrqDioMin = 5; +constexpr int kLr20x0IrqDioMax = 11; + +// A module's switch-capable DIO numbers, nullptr if it applies no table; count gets the length. +const int8_t *rfSwitchDiosFor(lora_module_enum module, size_t *count); + +// True if this module is handed the parsed table via setRfSwitchTable(). +bool moduleUsesRfSwitchTable(lora_module_enum module); + +// Build RadioLib's pin array and mode table from the parsed YAML, resolving each configured DIO +// through the parallel dioNumbers/pinConsts and taking modeMap[i]'s OpMode_t for RfSwitchModeId +// i. Returns rows written. +size_t buildRfSwitchTable(uint32_t (&pins)[Module::RFSWITCH_MAX_PINS], Module::RfSwitchMode_t *table, size_t tableCapacity, + const int8_t *dioNumbers, const uint32_t *pinConsts, size_t dioCount, const int32_t *modeMap); + struct pinMapping { std::string config_section; std::string config_name; @@ -90,8 +128,13 @@ extern struct portduino_config_struct { lora_module_enum lora_module; bool has_rfswitch_table = false; - uint32_t rfswitch_dio_pins[5] = {RADIOLIB_NC, RADIOLIB_NC, RADIOLIB_NC, RADIOLIB_NC, RADIOLIB_NC}; - Module::RfSwitchMode_t rfswitch_table[8]; + // The table as written: rfswitch_dio_num[i] is the DIO number for pin slot i (-1 = none), + // rfswitch_mode_high[m] has bit i set when slot i is HIGH in mode m. + int8_t rfswitch_dio_num[5] = {-1, -1, -1, -1, -1}; + uint8_t rfswitch_mode_high[RFSW_MODE_COUNT] = {0}; + bool rfswitch_mode_present[RFSW_MODE_COUNT] = {false}; + // DIO carrying the radio's interrupt; -1 keeps RadioLib's default (DIO5 on an LR2021). + int irq_dio_num = -1; bool force_simradio = false; bool has_device_id = false; uint8_t device_id[16] = {0}; @@ -108,6 +151,11 @@ extern struct portduino_config_struct { int rf95_max_power = 20; bool dio2_as_rf_switch = false; int dio3_tcxo_voltage = 0; + // DIO3_TCXO_VOLTAGE was written out as false or 0 rather than left absent. Diagnostic only, + // and so not serialized - the key it came from round-trips as absent either way. + bool dio3_tcxo_voltage_disabled = false; + // Probe for a TCXO and fall back to the XTAL; runtime twin of the TCXO_OPTIONAL define. + bool tcxo_optional = false; int lora_usb_pid = 0x5512; int lora_usb_vid = 0x1A86; int spiSpeed = 2000000; @@ -313,6 +361,8 @@ extern struct portduino_config_struct { out << YAML::Key << "DIO2_AS_RF_SWITCH" << YAML::Value << dio2_as_rf_switch; if (dio3_tcxo_voltage != 0) out << YAML::Key << "DIO3_TCXO_VOLTAGE" << YAML::Value << YAML::Precision(3) << (float)dio3_tcxo_voltage / 1000; + if (tcxo_optional) + out << YAML::Key << "TCXO_OPTIONAL" << YAML::Value << tcxo_optional; if (lora_usb_pid != 0x5512) out << YAML::Key << "USB_PID" << YAML::Value << YAML::Hex << lora_usb_pid; if (lora_usb_vid != 0x1A86) @@ -326,60 +376,33 @@ extern struct portduino_config_struct { out << YAML::Key << "USB_Serialnum" << YAML::Value << lora_usb_serial_num; if (spiSpeed != 2000000) out << YAML::Key << "spiSpeed" << YAML::Value << spiSpeed; - if (rfswitch_dio_pins[0] != RADIOLIB_NC) { + if (irq_dio_num >= 0) + out << YAML::Key << "IRQ_DIO_NUM" << YAML::Value << irq_dio_num; + if (has_rfswitch_table) { out << YAML::Key << "rfswitch_table" << YAML::Value << YAML::BeginMap; + // DIO numbers as written; a slot can be absent (sparse config), so remember its + // original index - row values below key off that, not position in this sequence. out << YAML::Key << "pins"; out << YAML::Value << YAML::Flow << YAML::BeginSeq; - + int emittedSlots[5]; + size_t pinCount = 0; for (int i = 0; i < 5; i++) { - // set up the pin array first - if (rfswitch_dio_pins[i] == RADIOLIB_LR11X0_DIO5) - out << "DIO5"; - if (rfswitch_dio_pins[i] == RADIOLIB_LR11X0_DIO6) - out << "DIO6"; - if (rfswitch_dio_pins[i] == RADIOLIB_LR11X0_DIO7) - out << "DIO7"; - if (rfswitch_dio_pins[i] == RADIOLIB_LR11X0_DIO8) - out << "DIO8"; - if (rfswitch_dio_pins[i] == RADIOLIB_LR11X0_DIO10) - out << "DIO10"; + if (rfswitch_dio_num[i] < 0) + continue; + out << ("DIO" + std::to_string(rfswitch_dio_num[i])); + emittedSlots[pinCount++] = i; } out << YAML::EndSeq; - for (int i = 0; i < 7; i++) { - switch (i) { - case 0: - out << YAML::Key << "MODE_STBY"; - break; - case 1: - out << YAML::Key << "MODE_RX"; - break; - case 2: - out << YAML::Key << "MODE_TX"; - break; - case 3: - out << YAML::Key << "MODE_TX_HP"; - break; - case 4: - out << YAML::Key << "MODE_TX_HF"; - break; - case 5: - out << YAML::Key << "MODE_GNSS"; - break; - case 6: - out << YAML::Key << "MODE_WIFI"; - break; - } - + // Only the modes the config carried, so a round trip invents no rows. + for (int m = 0; m < RFSW_MODE_COUNT; m++) { + if (!rfswitch_mode_present[m]) + continue; + out << YAML::Key << kRfSwitchModeNames[m].name; out << YAML::Value << YAML::Flow << YAML::BeginSeq; - for (int j = 0; j < 5; j++) { - if (rfswitch_table[i].values[j] == HIGH) { - out << "HIGH"; - } else { - out << "LOW"; - } - } + for (size_t j = 0; j < pinCount; j++) + out << ((rfswitch_mode_high[m] & (1u << emittedSlots[j])) ? "HIGH" : "LOW"); out << YAML::EndSeq; } out << YAML::EndMap; // rfswitch_table diff --git a/test/fixtures/portduino-config/README.md b/test/fixtures/portduino-config/README.md index 3768f55b43..729d794fa0 100644 --- a/test/fixtures/portduino-config/README.md +++ b/test/fixtures/portduino-config/README.md @@ -53,7 +53,7 @@ are upper, `sx1262` and `lr1121` lower. | ------------------------------ | ----------------------------------------------------------------- | | `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-bad-pin.yaml` | `DIO9` is a real pin name but not a switch pin on an `lr1121`. | | `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. | @@ -61,12 +61,69 @@ are upper, `sx1262` and `lr1121` lower. | `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-auto-partial.yaml` | **Silence guard** - under `auto` the omitted modes are not named. | +| `rfswitch-inert-table.yaml` | **Silence guard** - an sx1262's rows are not judged individually. | `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. +Both silence guards exist because a mode list is only meaningful once the radio is known. +Under `Module: auto` the part has not been probed, so naming the omitted modes against the +union of both families would advise adding `MODE_TX_HP`, `MODE_GNSS` and `MODE_WIFI` rows to +what may turn out to be an LR20x0. On a module that applies no table at all, singling out one +row as ignored would imply the others are used, when the single `module-mismatch-sx126x.yaml` +warning already says the whole table is inert. + +## LR20x0 rfswitch table and interrupt DIO + +The table is handed to an LR20x0 as well as an LR11xx, and the two parts have neither the +same modes nor the same switch pins, so which findings are correct depends on the module. + +| File | Expected | +| --------------------------------------- | ------------------------------------------------------------------------------- | +| `rfswitch-lr2021.yaml` | Clean. `MODE_RX_HF` is a real mode here, and `IRQ_DIO_NUM` keeps the IRQ clear. | +| `rfswitch-lr2021-irq-collision.yaml` | `IRQ_DIO_NUM: 5` names a pin the table also drives as a switch line. | +| `rfswitch-lr2021-irq-default.yaml` | The same collision reached by omitting the key: the radio default is DIO5. | +| `rfswitch-lr2021-irq-clear.yaml` | **False-positive guard** - DIO5 as the IRQ, table on DIO6/7/8, is clean. | +| `rfswitch-lr2021-irq-all-low.yaml` | DIO5 listed in `pins` but driven LOW everywhere: still a collision. | +| `rfswitch-lr2021-irq-out-of-range.yaml` | `IRQ_DIO_NUM: 3` is outside DIO5-DIO11, so it is discarded and DIO5 is used. | +| `rfswitch-lr2021-irq-alias.yaml` | `LR2021_IRQ_DIO_NUM` alongside `IRQ_DIO_NUM`: the older spelling does nothing. | +| `rfswitch-lr2021-no-table.yaml` | An LR20x0 with no table cannot transmit, same as an LR11xx without one. | +| `rfswitch-lr2021-wrong-mode.yaml` | `MODE_TX_HP` and `MODE_GNSS` are LR11xx modes an LR20x0 does not have. | + +`begin()` needs only SPI and BUSY, so a radio whose interrupt lands on a switch pin still +reports init success and then never receives a packet. + +An out-of-range `IRQ_DIO_NUM` is refused twice, in `loadConfig()` and again in the driver before +it reaches RadioLib, so the checker judges it per file: by the time the merged config is built a +rejected value and an absent key look the same. + +Why `-irq-all-low` is a fault and `-irq-clear` is not: `LR2021::config()` (from `begin()`) +points the IRQ DIO at `FUNCTION_IRQ`, then `setRfSwitchTable()` calls `setDioFunction(..., +FUNCTION_RF_SWITCH)` for every non-NC pin in the list whatever the levels are, and nothing +re-asserts the IRQ function afterwards. Listing the pin is what breaks it, not driving it. + +`rfswitch-lr2021.yaml` also carries `LR2021_MAX_POWER` and `LR2021_MAX_POWER_HF` in a case +that must stay at zero warnings, so dropping either from the checker's schema fails it. + +## TCXO probing (`Lora.TCXO_OPTIONAL`) + +A variant declares "a TCXO may or may not be fitted" at compile time with `TCXO_OPTIONAL`. +A Portduino carrier cannot: the same `meshtasticd` binary runs on hardware populated either +way, so the statement arrives as YAML and is answered at runtime. + +| File | Expected | +| ---------------------------------- | -------------------------------------------------------------------------------------- | +| `tcxo-optional.yaml` | Clean. No Vref given, so the TCXO attempt uses the 1.6 V radio default. | +| `tcxo-optional-sx1262.yaml` | Clean. Another family, explicit Vref, which is the one reported. | +| `tcxo-optional-unsupported.yaml` | An SX128x has no TCXO reference to probe for, so the key is inert. | +| `tcxo-optional-contradiction.yaml` | `DIO3_TCXO_VOLTAGE: false` asks for DIO3 to be left alone; the probe drives it anyway. | + +With the probe asked for and no voltage given, the driver tries the radio default rather +than skipping the TCXO attempt - otherwise there is nothing to fall back _from_. + ## PA gain table (`TX_GAIN_LORA`) Two shapes are accepted and they fail differently. A list is read element-by-element @@ -141,13 +198,19 @@ the last-loaded file is reset to its default - here `config.yaml` sets 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. +`rfswitch-last-wins/` covers `Lora.rfswitch_table` across two `config.d/` files. It +follows the same "last file loaded wins" rule as every other `Lora:` key - the loader +resets a table's pins and mode rows before applying a replacement, so an earlier +file's `MODE_RX` setting cannot leak through a later file that omits it. `--check` +reports this as the standard cross-file-overlap info, not a special-cased error. + +`rfswitch-replace/` pins the replacement itself rather than the diagnostic. Which of +two `config.d/` files wins is up to the filesystem, so the fixture above cannot assert +the effective table by value; here the losing table sits in `config.yaml`, which is +always loaded before `config.d/`, and the winner is therefore deterministic. The loser +is the wider of the two - four pins and three mode rows, all `HIGH` - so any carryover +appears as a surviving pin, a surviving mode row, or a `HIGH` that should be `LOW`. +`--output-yaml` is what reports it: the `--check` report says no more than `set`. ## Running these as a normal boot diff --git a/test/fixtures/portduino-config/irq-stale-override/config.d/irq-override.yaml b/test/fixtures/portduino-config/irq-stale-override/config.d/irq-override.yaml new file mode 100644 index 0000000000..70eb43922c --- /dev/null +++ b/test/fixtures/portduino-config/irq-stale-override/config.d/irq-override.yaml @@ -0,0 +1,3 @@ +# DIO3 is out of range for the LR20x0 interrupt, so this overrides DIO9 with nothing usable. +Lora: + IRQ_DIO_NUM: 3 diff --git a/test/fixtures/portduino-config/irq-stale-override/config.yaml b/test/fixtures/portduino-config/irq-stale-override/config.yaml new file mode 100644 index 0000000000..fedbf5d35c --- /dev/null +++ b/test/fixtures/portduino-config/irq-stale-override/config.yaml @@ -0,0 +1,20 @@ +# The main config is always read before ConfigDirectory, so this sets a valid DIO9 and the +# file in config.d/ then supplies an out-of-range one. The rejected value must take the radio +# back to its own default, not leave DIO9 standing behind a warning that promises the default. +Lora: + Module: lr2021 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + IRQ_DIO_NUM: 9 + rfswitch_table: + pins: [DIO6, DIO7, DIO8] + MODE_STBY: [LOW, LOW, LOW] + MODE_RX: [HIGH, LOW, HIGH] + MODE_TX: [HIGH, HIGH, HIGH] + MODE_RX_HF: [LOW, LOW, LOW] + MODE_TX_HF: [LOW, LOW, LOW] +General: + ConfigDirectory: config.d/ diff --git a/test/fixtures/portduino-config/rfswitch-auto-partial.yaml b/test/fixtures/portduino-config/rfswitch-auto-partial.yaml new file mode 100644 index 0000000000..e201444b2e --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-auto-partial.yaml @@ -0,0 +1,12 @@ +# CLEAN. A partial table under autodetect. Which modes are missing cannot be known before +# the radio is probed -- naming them against the union of both families would advise adding +# MODE_TX_HP, MODE_GNSS and MODE_WIFI rows to what may turn out to be an LR20x0. +Lora: + Module: auto + 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-bad-pin.yaml b/test/fixtures/portduino-config/rfswitch-bad-pin.yaml index 110bbebe4f..37d5db77a8 100644 --- a/test/fixtures/portduino-config/rfswitch-bad-pin.yaml +++ b/test/fixtures/portduino-config/rfswitch-bad-pin.yaml @@ -1,4 +1,6 @@ -# FAULT: DIO9 is not a switch pin. Only DIO5, DIO6, DIO7, DIO8 and DIO10 exist. +# FAULT: DIO9 is not a switch pin on an LR11xx, whose slots are DIO5, DIO6, DIO7, DIO8 and +# DIO10. The name is valid on an LR20x0, so the finding is judged against the module rather +# than a fixed list. Lora: Module: lr1121 rfswitch_table: diff --git a/test/fixtures/portduino-config/rfswitch-inert-table.yaml b/test/fixtures/portduino-config/rfswitch-inert-table.yaml new file mode 100644 index 0000000000..b912586461 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-inert-table.yaml @@ -0,0 +1,10 @@ +# FAULT, reported once. A partial table on a radio that never applies one. The single +# useful finding is that the whole table is inert -- rows are not judged against a +# family's mode list, since that would imply the rest of them are used. +Lora: + Module: sx1262 + rfswitch_table: + pins: [DIO5] + MODE_STBY: [LOW] + MODE_RX: [HIGH] + MODE_RX_HF: [HIGH] diff --git a/test/fixtures/portduino-config/rfswitch-sticky/config.d/a-first.yaml b/test/fixtures/portduino-config/rfswitch-last-wins/config.d/a-first.yaml similarity index 100% rename from test/fixtures/portduino-config/rfswitch-sticky/config.d/a-first.yaml rename to test/fixtures/portduino-config/rfswitch-last-wins/config.d/a-first.yaml diff --git a/test/fixtures/portduino-config/rfswitch-last-wins/config.d/b-second.yaml b/test/fixtures/portduino-config/rfswitch-last-wins/config.d/b-second.yaml new file mode 100644 index 0000000000..59597b33a2 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-last-wins/config.d/b-second.yaml @@ -0,0 +1,7 @@ +# Sets MODE_RX LOW on both pins. Whichever of this file and a-first.yaml the filesystem +# returns last should fully replace the other's table, per "last file wins". +Lora: + Module: lr1121 + rfswitch_table: + pins: [DIO5, DIO6] + MODE_RX: [LOW, LOW] diff --git a/test/fixtures/portduino-config/rfswitch-last-wins/config.yaml b/test/fixtures/portduino-config/rfswitch-last-wins/config.yaml new file mode 100644 index 0000000000..1781b359ff --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-last-wins/config.yaml @@ -0,0 +1,4 @@ +# Primary config for the rfswitch_table last-wins case. The two files in config.d/ +# each define a Lora.rfswitch_table; the one loaded last should fully replace it. +General: + ConfigDirectory: config.d/ diff --git a/test/fixtures/portduino-config/rfswitch-lr2021-irq-alias.yaml b/test/fixtures/portduino-config/rfswitch-lr2021-irq-alias.yaml new file mode 100644 index 0000000000..e536254df0 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-lr2021-irq-alias.yaml @@ -0,0 +1,18 @@ +# LR2021_IRQ_DIO_NUM is the older spelling, read only when IRQ_DIO_NUM is absent. Both are set +# here, so the alias line does nothing and DIO9 - not DIO11 - is the interrupt. +Lora: + Module: lr2021 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + IRQ_DIO_NUM: 9 + LR2021_IRQ_DIO_NUM: 11 + rfswitch_table: + pins: [DIO6, DIO7, DIO8] + MODE_STBY: [LOW, LOW, LOW] + MODE_RX: [HIGH, LOW, HIGH] + MODE_TX: [HIGH, HIGH, HIGH] + MODE_RX_HF: [LOW, LOW, LOW] + MODE_TX_HF: [LOW, LOW, LOW] diff --git a/test/fixtures/portduino-config/rfswitch-lr2021-irq-all-low.yaml b/test/fixtures/portduino-config/rfswitch-lr2021-irq-all-low.yaml new file mode 100644 index 0000000000..a778ce03a1 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-lr2021-irq-all-low.yaml @@ -0,0 +1,15 @@ +# FAULT: DIO5 is listed in pins but driven LOW in every mode. setRfSwitchTable() assigns +# FUNCTION_RF_SWITCH to every non-NC pin in the list whatever the levels, so listing DIO5 +# takes it away from FUNCTION_IRQ. Reported exactly like the HIGH case. +Lora: + Module: lr2021 + CS: 21 + IRQ: 16 + IRQ_DIO_NUM: 5 + rfswitch_table: + pins: [DIO5, DIO6] + MODE_STBY: [LOW, LOW] + MODE_RX: [LOW, HIGH] + MODE_TX: [LOW, HIGH] + MODE_RX_HF: [LOW, LOW] + MODE_TX_HF: [LOW, LOW] diff --git a/test/fixtures/portduino-config/rfswitch-lr2021-irq-clear.yaml b/test/fixtures/portduino-config/rfswitch-lr2021-irq-clear.yaml new file mode 100644 index 0000000000..768cfc66ec --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-lr2021-irq-clear.yaml @@ -0,0 +1,18 @@ +# CLEAN, and the false-positive guard for the collision check. DIO5 is the interrupt and also +# the radio default, but the table drives DIO6/DIO7/DIO8, so nothing collides. The check keys +# on the pins list, not on the DIO number. +Lora: + Module: lr2021 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + IRQ_DIO_NUM: 5 + rfswitch_table: + pins: [DIO6, DIO7, DIO8] + MODE_STBY: [LOW, LOW, LOW] + MODE_RX: [HIGH, LOW, HIGH] + MODE_TX: [HIGH, HIGH, HIGH] + MODE_RX_HF: [LOW, LOW, LOW] + MODE_TX_HF: [LOW, LOW, LOW] diff --git a/test/fixtures/portduino-config/rfswitch-lr2021-irq-collision.yaml b/test/fixtures/portduino-config/rfswitch-lr2021-irq-collision.yaml new file mode 100644 index 0000000000..1982487f75 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-lr2021-irq-collision.yaml @@ -0,0 +1,17 @@ +# FAULT: IRQ_DIO_NUM names DIO5, which the switch table also drives. One pin cannot be both +# the interrupt output and an RF switch control. begin() exercises only SPI and BUSY, so the +# radio reports init success and then never receives a packet. +Lora: + Module: lr2021 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + IRQ_DIO_NUM: 5 + 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_RX_HF: [LOW, LOW, HIGH, LOW] + MODE_TX_HF: [LOW, LOW, LOW, HIGH] diff --git a/test/fixtures/portduino-config/rfswitch-lr2021-irq-default.yaml b/test/fixtures/portduino-config/rfswitch-lr2021-irq-default.yaml new file mode 100644 index 0000000000..d7621a2152 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-lr2021-irq-default.yaml @@ -0,0 +1,16 @@ +# FAULT: the same collision, with no IRQ_DIO_NUM set. The radio keeps RadioLib's default of +# DIO5, which this table drives as a switch line, so the collision exists because the key is +# absent rather than because of anything written here. +Lora: + Module: lr2021 + 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_RX_HF: [LOW, LOW, HIGH, LOW] + MODE_TX_HF: [LOW, LOW, LOW, HIGH] diff --git a/test/fixtures/portduino-config/rfswitch-lr2021-irq-out-of-range.yaml b/test/fixtures/portduino-config/rfswitch-lr2021-irq-out-of-range.yaml new file mode 100644 index 0000000000..a49d6e3031 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-lr2021-irq-out-of-range.yaml @@ -0,0 +1,18 @@ +# DIO3 is a real pin on the part but not one the LR20x0 can raise its interrupt on, so the value +# is discarded when the config is read and the radio falls back to DIO5. The merged view cannot +# tell that from an absent key, which is why the range is judged per file. +Lora: + Module: lr2021 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + IRQ_DIO_NUM: 3 + rfswitch_table: + pins: [DIO6, DIO7, DIO8] + MODE_STBY: [LOW, LOW, LOW] + MODE_RX: [HIGH, LOW, HIGH] + MODE_TX: [HIGH, HIGH, HIGH] + MODE_RX_HF: [LOW, LOW, LOW] + MODE_TX_HF: [LOW, LOW, LOW] diff --git a/test/fixtures/portduino-config/rfswitch-lr2021-no-table.yaml b/test/fixtures/portduino-config/rfswitch-lr2021-no-table.yaml new file mode 100644 index 0000000000..1b49b36e33 --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-lr2021-no-table.yaml @@ -0,0 +1,11 @@ +# FAULT: an LR20x0 with no rfswitch_table, so setRfSwitchTable() is never called. Expect a +# WARNING from the merged-configuration checks. DIO5 is left as the interrupt to show the +# collision check stays silent when there is no table. +Lora: + Module: lr2021 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + IRQ_DIO_NUM: 5 diff --git a/test/fixtures/portduino-config/rfswitch-lr2021-wrong-mode.yaml b/test/fixtures/portduino-config/rfswitch-lr2021-wrong-mode.yaml new file mode 100644 index 0000000000..d221933ffa --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-lr2021-wrong-mode.yaml @@ -0,0 +1,17 @@ +# FAULT: MODE_GNSS and MODE_TX_HP are LR11xx modes an LR20x0 does not have, so both rows are +# dropped. Expect a WARNING naming the module rather than the "unknown key" a misspelling +# gets. +Lora: + Module: lr2021 + CS: 21 + IRQ: 16 + IRQ_DIO_NUM: 11 + rfswitch_table: + pins: [DIO5, DIO6] + MODE_STBY: [LOW, LOW] + MODE_RX: [HIGH, LOW] + MODE_TX: [HIGH, HIGH] + MODE_RX_HF: [LOW, HIGH] + MODE_TX_HF: [HIGH, LOW] + MODE_TX_HP: [LOW, HIGH] + MODE_GNSS: [HIGH, HIGH] diff --git a/test/fixtures/portduino-config/rfswitch-lr2021.yaml b/test/fixtures/portduino-config/rfswitch-lr2021.yaml new file mode 100644 index 0000000000..e9055de9fb --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-lr2021.yaml @@ -0,0 +1,20 @@ +# CLEAN. A correct LR20x0 table: MODE_RX_HF is a mode this part has, and IRQ_DIO_NUM keeps +# the interrupt off DIO5, which the table drives as a switch line. Also carries both LR2021 +# power ceilings, so dropping either from the checker's schema fails this case. +Lora: + Module: lr2021 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + IRQ_DIO_NUM: 9 + LR2021_MAX_POWER: 20 + LR2021_MAX_POWER_HF: 10 + 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_RX_HF: [LOW, LOW, HIGH, LOW] + MODE_TX_HF: [LOW, LOW, LOW, HIGH] diff --git a/test/fixtures/portduino-config/rfswitch-replace/config.d/override.yaml b/test/fixtures/portduino-config/rfswitch-replace/config.d/override.yaml new file mode 100644 index 0000000000..564b0a805f --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-replace/config.d/override.yaml @@ -0,0 +1,6 @@ +# Loaded after config.yaml, so this is the effective table: two pins, one mode row, all LOW. +Lora: + Module: lr2021 + rfswitch_table: + pins: [DIO5, DIO6] + MODE_RX: [LOW, LOW] diff --git a/test/fixtures/portduino-config/rfswitch-replace/config.yaml b/test/fixtures/portduino-config/rfswitch-replace/config.yaml new file mode 100644 index 0000000000..a5f8cab22c --- /dev/null +++ b/test/fixtures/portduino-config/rfswitch-replace/config.yaml @@ -0,0 +1,21 @@ +# The other half of the "last file wins" contract: that the winning table *replaces* the loser +# rather than merging over it. config.yaml is always loaded before config.d/, so unlike two files +# inside config.d/ - whose order the filesystem decides - the winner here is deterministic, which +# is what lets the effective table be asserted by value. +# +# This table is the loser, and is deliberately the wider of the two: four pins where the winner +# has two, and every mode row HIGH where the winner sets LOW. Any carryover therefore shows up in +# the emitted table as a surviving pin, a surviving mode row, or a HIGH that should be LOW. +General: + ConfigDirectory: config.d/ +Lora: + Module: lr2021 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + rfswitch_table: + pins: [DIO5, DIO6, DIO7, DIO8] + MODE_STBY: [HIGH, HIGH, HIGH, HIGH] + MODE_RX: [HIGH, HIGH, HIGH, HIGH] + MODE_TX: [HIGH, HIGH, 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 deleted file mode 100644 index 1832c76256..0000000000 --- a/test/fixtures/portduino-config/rfswitch-sticky/config.d/b-second.yaml +++ /dev/null @@ -1,9 +0,0 @@ -# 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 deleted file mode 100644 index ccc3ab9883..0000000000 --- a/test/fixtures/portduino-config/rfswitch-sticky/config.yaml +++ /dev/null @@ -1,4 +0,0 @@ -# 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/tcxo-optional-contradiction.yaml b/test/fixtures/portduino-config/tcxo-optional-contradiction.yaml new file mode 100644 index 0000000000..fef6fdfe49 --- /dev/null +++ b/test/fixtures/portduino-config/tcxo-optional-contradiction.yaml @@ -0,0 +1,12 @@ +# FAULT: the two TCXO keys ask for opposite things. Writing the voltage out as false says +# leave DIO3 alone; TCXO_OPTIONAL drives it at the radio default before trying the crystal. +# Both are honoured as documented, which is exactly why the outcome surprises people. +Lora: + Module: sx1262 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + DIO3_TCXO_VOLTAGE: false + TCXO_OPTIONAL: true diff --git a/test/fixtures/portduino-config/tcxo-optional-sx1262.yaml b/test/fixtures/portduino-config/tcxo-optional-sx1262.yaml new file mode 100644 index 0000000000..06d1a0b4eb --- /dev/null +++ b/test/fixtures/portduino-config/tcxo-optional-sx1262.yaml @@ -0,0 +1,11 @@ +# CLEAN. The same flag on another family, with an explicit Vref. That voltage is what gets +# tried first, so the report must name it rather than the 1.6 V default. +Lora: + Module: sx1262 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + DIO3_TCXO_VOLTAGE: 1.8 + TCXO_OPTIONAL: true diff --git a/test/fixtures/portduino-config/tcxo-optional-unsupported.yaml b/test/fixtures/portduino-config/tcxo-optional-unsupported.yaml new file mode 100644 index 0000000000..90bb4ba727 --- /dev/null +++ b/test/fixtures/portduino-config/tcxo-optional-unsupported.yaml @@ -0,0 +1,9 @@ +# FAULT: TCXO_OPTIONAL on an SX128x, which has no TCXO reference to probe for. The key is +# read, stored and inert. +Lora: + Module: sx1280 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + TCXO_OPTIONAL: true diff --git a/test/fixtures/portduino-config/tcxo-optional.yaml b/test/fixtures/portduino-config/tcxo-optional.yaml new file mode 100644 index 0000000000..0d054a3892 --- /dev/null +++ b/test/fixtures/portduino-config/tcxo-optional.yaml @@ -0,0 +1,19 @@ +# CLEAN. The carrier ships populated either way, so the driver probes: try the TCXO, fall +# back to the XTAL. No DIO3_TCXO_VOLTAGE, so the TCXO attempt uses the radio default of +# 1.6 V rather than being skipped, and the report must name that Vref. +Lora: + Module: lr2021 + spidev: spidev0.0 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 + IRQ_DIO_NUM: 9 + TCXO_OPTIONAL: true + 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_RX_HF: [LOW, LOW, HIGH, LOW] + MODE_TX_HF: [LOW, LOW, LOW, HIGH]