mirror of
https://github.com/meshtastic/firmware.git
synced 2026-10-09 14:41:19 -04:00
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<n>" 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.
This commit is contained in:
35 files changed
+1057
-205
No files matched your search
@@ -47,10 +47,15 @@ const std::map<std::string, std::set<std::string>> &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<std::string> kHub75Keys = {
|
||||
"ShowRefreshRate", "InverseColors", "RGBSequence", "PixelMapper", "PanelType", "LimitRefreshRateHz",
|
||||
"GPIOSlowdown"};
|
||||
|
||||
const std::set<std::string> 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<std::string> 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<std::string> &kRfSwitchModes()
|
||||
{
|
||||
static const std::set<std::string> s = [] {
|
||||
std::set<std::string> 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<std::string> 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<std::string> modesFor(lora_module_enum module)
|
||||
{
|
||||
if (module == use_autoconf)
|
||||
return kRfSwitchModes();
|
||||
std::set<std::string> 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<std::string> pinsFor(lora_module_enum module)
|
||||
{
|
||||
std::set<std::string> 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<std::string> &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<std::string, std::set<std::string>> &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<std::string> valid = pinsFor(portduino_config.lora_module);
|
||||
for (const auto &pin : pins) {
|
||||
const std::string name = pin.as<std::string>("");
|
||||
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<std::string> validModes = modesFor(portduino_config.lora_module);
|
||||
|
||||
for (const auto &entry : table) {
|
||||
const std::string key = entry.first.as<std::string>("");
|
||||
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<std::string> 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<std::string> 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<std::string, ValueSpec> &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<Finding> &findings)
|
||||
{
|
||||
if (!converts(value, kInt))
|
||||
return; // checkValueType() already reported it
|
||||
|
||||
const int dio = value.as<int>();
|
||||
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<Finding> &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<std::string, s
|
||||
if (coveredByAncestor)
|
||||
continue;
|
||||
|
||||
// The one place "last wins" is untrue: the loader only ever writes HIGH, so the effective
|
||||
// table is the OR of every table loaded -- a switch state belonging to none of them.
|
||||
if (entry.first == "Lora.rfswitch_table") {
|
||||
findings.push_back({kError, across, 0,
|
||||
"'Lora.rfswitch_table' is set in " + std::to_string(entry.second.size()) + " files (" +
|
||||
describeOwners(entry.second) +
|
||||
"). These do NOT override each other: the loader only ever writes HIGH, so a HIGH "
|
||||
"from an earlier file survives a later file that sets LOW there, and the radio ends "
|
||||
"up driving the OR of every table. Enable exactly one"});
|
||||
continue;
|
||||
}
|
||||
|
||||
findings.push_back({kInfo, across, 0,
|
||||
"'" + entry.first + "' is set in " + std::to_string(entry.second.size()) + " files (" +
|
||||
describeOwners(entry.second) + "). The file loaded last wins"});
|
||||
@@ -859,11 +984,6 @@ void checkCrossFileOverlap(const PathIndex &paths, const std::map<std::string, s
|
||||
// Semantic checks against the merged configuration
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
bool isLR11xx(lora_module_enum module)
|
||||
{
|
||||
return module == use_lr1110 || module == use_lr1120 || module == use_lr1121;
|
||||
}
|
||||
|
||||
std::string moduleName()
|
||||
{
|
||||
const auto it = portduino_config.loraModules.find(portduino_config.lora_module);
|
||||
@@ -917,16 +1037,69 @@ void checkMergedConfig(const PathIndex &paths, std::vector<Finding> &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";
|
||||
|
||||
Reference in new issue
Block a user