refactor: simplify TCXO voltage handling across interfaces and improve comments

bin/config-dist.yaml

@@ -81,10 +81,8 @@ Lora:
 
 #  DIO3_TCXO_VOLTAGE: true  # the Waveshare Core1262 and others are known to need this setting
 
-#  TCXO_OPTIONAL: true  # Probe both oscillators; without DIO3_TCXO_VOLTAGE the TCXO attempt uses
-#                       # the radio default of 1.6 V. SX126x, LR11xx and LR20x0 only. Init order is
-#                       # per-family: SX126x and LR20x0 try the TCXO first, then fall back to the
-#                       # crystal; LR11x0 tries the crystal first, then falls back to the TCXO.
+#  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.
 
 #  TXen: x  # TX and RX enable pins
 #  RXen: x

src/mesh/LR11x0Interface.cpp

@@ -72,7 +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
-#define LR11X0_TCXO_DEFAULT_VOLTAGE (TCXO_OPTIONAL_ENABLED ? TCXO_OPTIONAL_DEFAULT_VOLTAGE : 0)
+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
@@ -106,11 +111,11 @@ template <typename T> bool LR11x0Interface<T>::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
@@ -154,8 +159,7 @@ template <typename T> bool LR11x0Interface<T>::init()
     };
 
     // 1. XTAL first when probing (see TCXO_OPTIONAL_ENABLED), else the configured Vref. Not a
-    // ternary: when TCXO_OPTIONAL_ENABLED is false, tcxoVoltage above already folds to 0 on a
-    // board with no explicit Vref, which reads to cppcheck as both branches yielding 0.
+    // ternary: cppcheck sees both branches as 0 when tcxoVoltage above already folded to it.
     float attemptVoltage = tcxoVoltage;
     if (TCXO_OPTIONAL_ENABLED)
         attemptVoltage = 0;

src/mesh/LR20x0Interface.cpp

@@ -82,8 +82,13 @@ template <typename T> bool LR20x0Interface<T>::init()
 
 #if ARCH_PORTDUINO
     // An explicit Vref wins; probing with none given tries the radio default first.
-    float tcxoVoltage = portduino_config.dio3_tcxo_voltage > 0 ? (float)portduino_config.dio3_tcxo_voltage / 1000
-                                                               : (TCXO_OPTIONAL_ENABLED ? TCXO_OPTIONAL_DEFAULT_VOLTAGE : 0);
+    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)
@@ -236,8 +241,13 @@ template <typename T> bool LR20x0Interface<T>::reconfigure()
 #endif
 
 #if ARCH_PORTDUINO
-        float tcxoVoltage = portduino_config.dio3_tcxo_voltage > 0 ? (float)portduino_config.dio3_tcxo_voltage / 1000
-                                                                   : (TCXO_OPTIONAL_ENABLED ? TCXO_OPTIONAL_DEFAULT_VOLTAGE : 0);
+        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)

src/mesh/SX126xInterface.cpp

@@ -72,8 +72,12 @@ template <typename T> bool SX126xInterface<T>::init()
 #if ARCH_PORTDUINO
     // An explicit Vref wins; probing with none given tries the radio default first.
     bool tcxoVoltageExplicit = portduino_config.dio3_tcxo_voltage > 0;
-    tcxoVoltage = tcxoVoltageExplicit ? (float)portduino_config.dio3_tcxo_voltage / 1000
-                                      : (TCXO_OPTIONAL_ENABLED ? TCXO_OPTIONAL_DEFAULT_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);

src/platform/portduino/ConfigCheck.cpp

@@ -109,10 +109,8 @@ std::string moduleName();
 
 // 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 rather than a namespace-scope global: it reads kRfSwitchModeNames, which
-// is defined in a different translation unit (PortduinoGlue.cpp), and lazy first-use init sidesteps
-// any cross-TU static-initialization-order question rather than relying on that array staying
-// constant-initializable.
+// 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 = [] {
@@ -131,9 +129,8 @@ const std::set<std::string> kRfSwitchPins = {"DIO5", "DIO6", "DIO7", "DIO8", "DI
 // keep the radio headers out of this file.
 const int kLr20x0DefaultIrqDio = 5;
 
-// Mode names this module can apply. Only the LR20x0 differs from the LR11xx set. An unresolved
-// "auto" hasn't picked a family yet, so it is reported against the union of both - narrowing to
-// either family's subset would flag the other family's valid modes as unsupported.
+// Mode names this module can apply. 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)

src/platform/portduino/PortduinoGlue.h

@@ -370,10 +370,8 @@ extern struct portduino_config_struct {
         if (has_rfswitch_table) {
             out << YAML::Key << "rfswitch_table" << YAML::Value << YAML::BeginMap;
 
-            // DIO numbers as written: the slot a RADIOLIB_* constant belongs to cannot be
-            // decoded without knowing the part. A slot can be absent (sparse config), so
-            // remember which original slot each emitted pin came from - the row values below
-            // read rfswitch_mode_high by that original slot, not by position in this sequence.
+            // 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];