From 70bd2cc579a503ab4d0f1650f66ea488ec3e9628 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 04:07:10 +0000 Subject: [PATCH] TransparentRadio: verify what the device took, and don't rewrite it needlessly - Read-back compares the settings that decide what is on the air (region, preset or bandwidth/SF, coding rate, slot or frequency, tx_enabled), not just transparent_mode. A device that rejects a LoRa config restores its old one whole, transparent_mode included, so that is now "Refused": retried after 5 minutes or at once on this node's next config change. Only a device that took the settings and still reads transparent_mode false is old firmware. - The same comparison decides whether a device is already set up, so fields the device normalises no longer cause a push on every reconnect. coding_rate, spread_factor and bandwidth are clamped the way the device's AdminModule stores them. - Firmware that reports a release older than 2.8.3 is refused before any set_config reaches it. - With no LoRa region set here, nothing is pushed (it would set the device's region to UNSET and turn its TX off); setting a region pushes. - Frames held for their own radio settings (RadioTxHooks::holdsRadio, e.g. MeshBeacon on another preset) are dropped, since the device can't switch per packet, and the hooks hear about every released frame. - Reconnect attempts back off from 5 s to 60 s: opening a TCP link blocks the main loop. - hop_limit/hop_start are clamped as encodeRadioBuffer() does instead of being refused by the device; slot_parity is no longer forwarded. - Lora.Module transparent leaves spidev empty, so the SPI bus is untouched. - Queued frames are released on destruction; the late-window clamp only reschedules when the moved frame is the front. --- bin/config.d/lora-transparent.yaml | 1 + src/platform/portduino/PortduinoGlue.cpp | 4 +- src/platform/portduino/TransparentRadio.cpp | 138 +++++++++++++++---- src/platform/portduino/TransparentRadio.h | 29 ++-- test/test_transparent_radio/test_main.cpp | 140 +++++++++++++++----- 5 files changed, 237 insertions(+), 75 deletions(-) diff --git a/bin/config.d/lora-transparent.yaml b/bin/config.d/lora-transparent.yaml index bf097344cd..bf2b505a7d 100644 --- a/bin/config.d/lora-transparent.yaml +++ b/bin/config.d/lora-transparent.yaml @@ -7,3 +7,4 @@ Meta: Lora: Module: transparent TransparentDevice: /dev/ttyACM0 # the node's serial port, or host[:port] for its TCP API (port 4403 by default) + # TransparentDevice: "[fe80::1]:4403" # quote a bracketed IPv6 address, or YAML reads it as a list diff --git a/src/platform/portduino/PortduinoGlue.cpp b/src/platform/portduino/PortduinoGlue.cpp index 7e2ab6ae5e..a8cbedda26 100644 --- a/src/platform/portduino/PortduinoGlue.cpp +++ b/src/platform/portduino/PortduinoGlue.cpp @@ -1173,7 +1173,9 @@ bool loadConfig(const char *configPath) portduino_config.lora_usb_vid = yamlConfig["Lora"]["USB_VID"].as(0x1A86); portduino_config.lora_spi_dev = yamlConfig["Lora"]["spidev"].as("spidev0.0"); - if (portduino_config.lora_spi_dev != "ch341") { + if (portduino_config.lora_module == use_transparent) { + portduino_config.lora_spi_dev = ""; // the radio is on the other node, so leave the SPI bus alone + } else if (portduino_config.lora_spi_dev != "ch341") { portduino_config.lora_spi_dev = "/dev/" + portduino_config.lora_spi_dev; if (portduino_config.lora_spi_dev.length() == 14) { int x = portduino_config.lora_spi_dev.at(11) - '0'; diff --git a/src/platform/portduino/TransparentRadio.cpp b/src/platform/portduino/TransparentRadio.cpp index e763f1c03c..395c655031 100644 --- a/src/platform/portduino/TransparentRadio.cpp +++ b/src/platform/portduino/TransparentRadio.cpp @@ -2,13 +2,16 @@ #if HAS_TRANSPARENT_RADIO +#include "MeshRadio.h" #include "MeshService.h" #include "NodeDB.h" +#include "RadioTxHook.h" #include "Router.h" #include "Throttle.h" #include "UptimeClock.h" #include "airtime.h" #include +#include #include namespace @@ -17,12 +20,38 @@ constexpr uint8_t FRAME_START1 = 0x94; constexpr uint8_t FRAME_START2 = 0xc3; constexpr size_t WAKE_BYTES = 32; // a sleeping device's serial API wakes on a run of START2 -bool sameLoraConfig(const meshtastic_Config_LoRaConfig &a, const meshtastic_Config_LoRaConfig &b) +// First release with LoRaConfig.transparent_mode. Older firmware would apply our settings, then route on its own. +constexpr unsigned MIN_FIRMWARE[3] = {2, 8, 3}; + +bool firmwareSupportsTransparentMode(const char *version) { - uint8_t encA[meshtastic_Config_LoRaConfig_size], encB[meshtastic_Config_LoRaConfig_size]; - const size_t lenA = pb_encode_to_bytes(encA, sizeof(encA), &meshtastic_Config_LoRaConfig_msg, &a); - const size_t lenB = pb_encode_to_bytes(encB, sizeof(encB), &meshtastic_Config_LoRaConfig_msg, &b); - return lenA == lenB && memcmp(encA, encB, lenA) == 0; + unsigned v[3] = {0, 0, 0}; + if (sscanf(version, "%u.%u.%u", &v[0], &v[1], &v[2]) < 3) + return true; // unparseable: let the read-back decide + for (int i = 0; i < 3; i++) + if (v[i] != MIN_FIRMWARE[i]) + return v[i] > MIN_FIRMWARE[i]; + return true; +} + +/// The first setting that puts the two configs on different air, or nullptr if they agree. +const char *firstRfDifference(const meshtastic_Config_LoRaConfig &a, const meshtastic_Config_LoRaConfig &b, bool withPower) +{ + if (a.region != b.region) + return "region"; + if (a.use_preset != b.use_preset || (a.use_preset && a.modem_preset != b.modem_preset)) + return "modem preset"; + if (!a.use_preset && (a.bandwidth != b.bandwidth || a.spread_factor != b.spread_factor)) + return "bandwidth or spreading factor"; + if (a.coding_rate != b.coding_rate) + return "coding rate"; + if (a.override_frequency != b.override_frequency || (!a.override_frequency && a.channel_num != b.channel_num)) + return "frequency slot"; + if (a.tx_enabled != b.tx_enabled) + return "tx_enabled"; + if (withPower && a.tx_power != b.tx_power) // the device may clamp this to its own limits, so never fatal + return "tx_power"; + return nullptr; } } // namespace @@ -31,6 +60,12 @@ TransparentRadio::TransparentRadio(std::unique_ptr link) { } +TransparentRadio::~TransparentRadio() +{ + while (meshtastic_MeshPacket *p = txQueue.dequeue()) + releaseFrame(p); +} + bool TransparentRadio::init() { RadioInterface::init(); @@ -41,7 +76,7 @@ bool TransparentRadio::init() bool TransparentRadio::reconfigure() { RadioInterface::reconfigure(); - if (state == State::Pushing || state == State::Ready) + if (state == State::WaitingForRegion || state == State::Refused || state == State::Pushing || state == State::Ready) pushConfig(); return true; } @@ -87,7 +122,7 @@ bool TransparentRadio::cancelSending(NodeNum from, PacketId id) meshtastic_MeshPacket *p = txQueue.remove(from, id); if (!p) return false; - releaseRemoved(p); + releaseFrame(p); return true; } @@ -101,17 +136,10 @@ bool TransparentRadio::removePendingTXPacket(NodeNum from, PacketId id, uint32_t meshtastic_MeshPacket *p = txQueue.remove(from, id, true, true, hop_limit_lt); if (!p) return false; - releaseRemoved(p); + releaseFrame(p); return true; } -void TransparentRadio::releaseRemoved(meshtastic_MeshPacket *p) -{ - if (p == scheduledPacket) // its address can come back from the pool as a different packet - scheduledPacket = nullptr; - packetPool.release(p); -} - void TransparentRadio::clampToLateRebroadcastWindow(NodeNum from, PacketId id) { meshtastic_MeshPacket *p = txQueue.remove(from, id, true, false); @@ -119,26 +147,40 @@ void TransparentRadio::clampToLateRebroadcastWindow(NodeNum from, PacketId id) return; p->tx_after = Time::timerEndsAtMillis(getTxDelayMsecWeightedWorst(p->rx_snr)); bool dropped = false; - if (!txQueue.enqueue(p, &dropped)) - packetPool.release(p); + if (!txQueue.enqueue(p, &dropped)) { + txDrop++; + releaseFrame(p); + return; + } if (dropped) txDrop++; - scheduleTx(); // the moved packet may be the front, now with a later due time + if (txQueue.getFront() == p) // its due time moved; any other front is picked up by serviceTxQueue() + scheduleTx(); +} + +void TransparentRadio::releaseFrame(meshtastic_MeshPacket *p) +{ + if (p == scheduledPacket) // its address can come back from the pool as a different packet + scheduledPacket = nullptr; + RadioTxHooks::packetReleased(this, p); + packetPool.release(p); } int32_t TransparentRadio::runOnce() { - if (state == State::Disconnected && (!openAttempted || Throttle::hasElapsed(lastOpenAttemptMs, RECONNECT_INTERVAL_MS))) { + if (state == State::Disconnected && (!openAttempted || Throttle::hasElapsed(lastOpenAttemptMs, reconnectIntervalMs))) { openAttempted = true; lastOpenAttemptMs = Time::getMillis(); if (link->open()) { LOG_INFO("Transparent radio: connected to %s", link->describe().c_str()); openFailureLogged = false; + reconnectIntervalMs = RECONNECT_MIN_MS; startHandshake(); - } else if (!openFailureLogged) { - LOG_WARN("Transparent radio: can't open %s, retrying every %us", link->describe().c_str(), - (unsigned)(RECONNECT_INTERVAL_MS / 1000)); + } else { + if (!openFailureLogged) + LOG_WARN("Transparent radio: can't open %s, retrying", link->describe().c_str()); openFailureLogged = true; + reconnectIntervalMs = std::min(reconnectIntervalMs * 2, RECONNECT_MAX_MS); } } @@ -148,13 +190,15 @@ int32_t TransparentRadio::runOnce() if ((state == State::Configuring || state == State::Pushing) && Throttle::hasElapsed(stateSinceMs, HANDSHAKE_TIMEOUT_MS)) { LOG_WARN("Transparent radio: %s did not finish its handshake, asking again", link->describe().c_str()); startHandshake(); + } else if (state == State::Refused && Throttle::hasElapsed(stateSinceMs, REFUSED_RETRY_MS)) { + startHandshake(); } if (link->isOpen() && Throttle::hasElapsed(lastWriteMs, HEARTBEAT_INTERVAL_MS)) sendHeartbeat(); serviceTxQueue(); - return (link->isOpen() || scheduledPacket) ? POLL_INTERVAL_MS : RECONNECT_INTERVAL_MS; + return (link->isOpen() || scheduledPacket) ? POLL_INTERVAL_MS : reconnectIntervalMs; } void TransparentRadio::setState(State next) @@ -167,6 +211,7 @@ void TransparentRadio::startHandshake() { rxPtr = 0; deviceNodeNum = 0; + deviceFirmwareTooOld = false; haveDeviceLora = false; verifyRequestId = 0; deviceQueueFree = -1; @@ -241,6 +286,9 @@ void TransparentRadio::handleFromRadio() case meshtastic_FromRadio_my_info_tag: deviceNodeNum = fromRadio.my_info.my_node_num; break; + case meshtastic_FromRadio_metadata_tag: + deviceFirmwareTooOld = !firmwareSupportsTransparentMode(fromRadio.metadata.firmware_version); + break; case meshtastic_FromRadio_config_tag: if (fromRadio.config.which_payload_variant == meshtastic_Config_lora_tag) { deviceLora = fromRadio.config.payload_variant.lora; @@ -280,6 +328,11 @@ meshtastic_Config_LoRaConfig TransparentRadio::desiredLoraConfig() const { meshtastic_Config_LoRaConfig lora = config.lora; lora.transparent_mode = true; + // The device's AdminModule rewrites these when out of range; match it, or the read-back never agrees. + lora.coding_rate = clampCodingRate(lora.coding_rate); + lora.spread_factor = clampSpreadFactor(lora.spread_factor); + if (!lora.use_preset) + lora.bandwidth = clampBandwidthCode(lora.bandwidth); // The slot comes from the primary channel name, and the device's name is not ours: pin the slot we resolved. if (!lora.override_frequency) lora.channel_num = savedChannelNum + 1; @@ -300,8 +353,22 @@ void TransparentRadio::pushConfig() LOG_WARN("Transparent radio: %s sent no node number; waiting to retry", link->describe().c_str()); return; // the handshake timeout asks again } + if (deviceFirmwareTooOld) { + LOG_ERROR("Transparent radio: %s runs firmware older than %u.%u.%u, which has no transparent mode", + link->describe().c_str(), MIN_FIRMWARE[0], MIN_FIRMWARE[1], MIN_FIRMWARE[2]); + link->close(); + setState(State::Unsupported); + return; + } + if (config.lora.region == meshtastic_Config_LoRaConfig_RegionCode_UNSET) { + if (state != State::WaitingForRegion) + LOG_WARN("Transparent radio: no LoRa region set, so %s is left as it is", link->describe().c_str()); + setState(State::WaitingForRegion); + return; + } + const meshtastic_Config_LoRaConfig want = desiredLoraConfig(); - if (haveDeviceLora && sameLoraConfig(deviceLora, want)) { + if (haveDeviceLora && deviceLora.transparent_mode && !firstRfDifference(want, deviceLora, true)) { setState(State::Ready); LOG_INFO("Transparent radio: %s ready", link->describe().c_str()); return; @@ -313,6 +380,7 @@ void TransparentRadio::pushConfig() set.set_config.payload_variant.lora = want; if (!sendAdmin(set, false)) return; + pushedLora = want; meshtastic_AdminMessage get = meshtastic_AdminMessage_init_zero; get.which_payload_variant = meshtastic_AdminMessage_get_config_request_tag; @@ -333,6 +401,15 @@ void TransparentRadio::handleAdminReply(const meshtastic_MeshPacket &p) deviceLora = admin.get_config_response.payload_variant.lora; haveDeviceLora = true; + // A device that rejects a LoRa config restores its old one whole, transparent_mode included, so check the radio + // settings first: only a device that took them and still reads transparent_mode false lacks the feature. + if (const char *difference = firstRfDifference(pushedLora, deviceLora, false)) { + LOG_WARN("Transparent radio: %s did not take our LoRa settings (%s differs); retrying in %u min or on the next " + "config change", + link->describe().c_str(), difference, (unsigned)(REFUSED_RETRY_MS / 60000)); + setState(State::Refused); + return; + } if (!deviceLora.transparent_mode) { LOG_ERROR("Transparent radio: %s kept transparent_mode off; its firmware predates transparent mode", link->describe().c_str()); @@ -397,14 +474,13 @@ bool TransparentRadio::forward(const meshtastic_MeshPacket *p) out.to = p->to; out.id = p->id; out.channel = p->channel; - out.hop_limit = p->hop_limit; - out.hop_start = p->hop_start; + out.hop_limit = p->hop_limit > HOP_MAX ? HOP_RELIABLE : p->hop_limit; // as encodeRadioBuffer() does for a local chip + out.hop_start = p->hop_start & HOP_MAX; out.want_ack = p->want_ack; out.via_mqtt = p->via_mqtt; out.next_hop = p->next_hop; out.relay_node = p->relay_node; out.priority = p->priority; - out.slot_parity = p->slot_parity; out.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; out.encrypted = p->encrypted; return writeToRadio(); @@ -458,7 +534,11 @@ void TransparentRadio::serviceTxQueue() meshtastic_MeshPacket *p = txQueue.dequeue(); if (state != State::Ready) { - LOG_DEBUG("Transparent radio: drop 0x%08x, device not ready", p->id); + LOG_DEBUG_RADIO("Transparent radio: drop 0x%08x, device not ready", p->id); + txDrop++; + } else if (RadioTxHooks::holdsRadio(p)) { + LOG_WARN("Transparent radio: drop 0x%08x, it needs its own radio settings, which the device can't switch per packet", + p->id); txDrop++; } else if (p->which_payload_variant != meshtastic_MeshPacket_encrypted_tag) { LOG_WARN("Transparent radio: drop 0x%08x, not encrypted", p->id); @@ -476,7 +556,7 @@ void TransparentRadio::serviceTxQueue() } else { txDrop++; } - packetPool.release(p); + releaseFrame(p); scheduleTx(); } diff --git a/src/platform/portduino/TransparentRadio.h b/src/platform/portduino/TransparentRadio.h index c205f55ad2..9204b400b9 100644 --- a/src/platform/portduino/TransparentRadio.h +++ b/src/platform/portduino/TransparentRadio.h @@ -10,23 +10,23 @@ #include "concurrency/OSThread.h" #include "meshtastic/admin.pb.h" -/** - * meshtasticd's radio for Lora.Module "transparent": the LoRa chip belongs to another node, running - * LoRaConfig.transparent_mode and reached over its client API. That node puts this node's frames on the air - * unchanged and hands back every frame it hears, so routing, encryption and airtime accounting all happen here. - */ +/** meshtasticd's radio for Lora.Module "transparent": another node's LoRa chip, reached over its client API while + * that node runs LoRaConfig.transparent_mode. Routing, encryption and airtime accounting all stay here. */ class TransparentRadio : public RadioInterface, protected concurrency::OSThread { public: enum class State : uint8_t { - Disconnected, // waiting to (re)open the link - Configuring, // asked for the device's config dump - Pushing, // sent our LoRaConfig, waiting to read it back - Ready, // device confirmed transparent_mode with our settings - Unsupported, // device firmware ignored transparent_mode; stays here until restart + Disconnected, // waiting to (re)open the link + Configuring, // asked for the device's config dump + WaitingForRegion, // this node has no LoRa region yet, so there is nothing to push + Pushing, // sent our LoRaConfig, waiting to read it back + Refused, // the device kept other radio settings; retried later or on our next config change + Ready, // device confirmed transparent_mode with our settings + Unsupported, // device firmware predates transparent_mode; stays here until restart }; explicit TransparentRadio(std::unique_ptr link); + ~TransparentRadio() override; bool init() override; bool reconfigure() override; @@ -50,8 +50,10 @@ class TransparentRadio : public RadioInterface, protected concurrency::OSThread private: static constexpr uint32_t POLL_INTERVAL_MS = 10; - static constexpr uint32_t RECONNECT_INTERVAL_MS = 5000; + static constexpr uint32_t RECONNECT_MIN_MS = 5000; + static constexpr uint32_t RECONNECT_MAX_MS = 60000; // opening a TCP link blocks the main loop, so back off static constexpr uint32_t HANDSHAKE_TIMEOUT_MS = 15000; + static constexpr uint32_t REFUSED_RETRY_MS = 5 * 60 * 1000; // each push rewrites the device's flash static constexpr uint32_t HEARTBEAT_INTERVAL_MS = 5 * 60 * 1000; // the device drops an API client silent for 15 minutes static constexpr size_t FRAME_HEADER_LEN = 4; @@ -69,7 +71,7 @@ class TransparentRadio : public RadioInterface, protected concurrency::OSThread bool writeToRadio(); bool forward(const meshtastic_MeshPacket *p); void deliverReceived(const meshtastic_MeshPacket &p); - void releaseRemoved(meshtastic_MeshPacket *p); + void releaseFrame(meshtastic_MeshPacket *p); void scheduleTx(); void serviceTxQueue(); @@ -85,12 +87,15 @@ class TransparentRadio : public RadioInterface, protected concurrency::OSThread bool openAttempted = false; bool openFailureLogged = false; uint32_t lastOpenAttemptMs = 0; + uint32_t reconnectIntervalMs = RECONNECT_MIN_MS; uint32_t stateSinceMs = 0; uint32_t lastWriteMs = 0; NodeNum deviceNodeNum = 0; + bool deviceFirmwareTooOld = false; bool haveDeviceLora = false; meshtastic_Config_LoRaConfig deviceLora = meshtastic_Config_LoRaConfig_init_zero; + meshtastic_Config_LoRaConfig pushedLora = meshtastic_Config_LoRaConfig_init_zero; PacketId verifyRequestId = 0; int32_t deviceQueueFree = -1; // unknown until the device reports a QueueStatus diff --git a/test/test_transparent_radio/test_main.cpp b/test/test_transparent_radio/test_main.cpp index ac55b30bfd..f81921f88c 100644 --- a/test/test_transparent_radio/test_main.cpp +++ b/test/test_transparent_radio/test_main.cpp @@ -169,12 +169,16 @@ static void deviceSendsLora(const meshtastic_Config_LoRaConfig &lora) } /// The device's answer to want_config: its node number, its LoRa config, then config_complete_id. -static void deviceSendsConfigDump(const meshtastic_Config_LoRaConfig &lora) +static void deviceSendsConfigDump(const meshtastic_Config_LoRaConfig &lora, const char *firmwareVersion = "2.8.3.abcdef0") { meshtastic_FromRadio fr = meshtastic_FromRadio_init_zero; fr.which_payload_variant = meshtastic_FromRadio_my_info_tag; fr.my_info.my_node_num = DEVICE_NODE; deviceSends(fr); + fr = meshtastic_FromRadio_init_zero; + fr.which_payload_variant = meshtastic_FromRadio_metadata_tag; + strncpy(fr.metadata.firmware_version, firmwareVersion, sizeof(fr.metadata.firmware_version) - 1); + deviceSends(fr); deviceSendsLora(lora); fr = meshtastic_FromRadio_init_zero; fr.which_payload_variant = meshtastic_FromRadio_config_complete_id_tag; @@ -209,9 +213,30 @@ static meshtastic_Config_LoRaConfig expectedDeviceLora() meshtastic_Config_LoRaConfig lora = config.lora; lora.transparent_mode = true; lora.channel_num = radio->getChannelNum() + 1; + lora.coding_rate = clampCodingRate(lora.coding_rate); // as the device's AdminModule stores it + lora.spread_factor = clampSpreadFactor(lora.spread_factor); return lora; } +/// The device's answer to the read-back (get_config_request) the radio sent as `request`. +static void deviceAnswersReadBack(const meshtastic_ToRadio &request, const meshtastic_Config_LoRaConfig &lora) +{ + meshtastic_FromRadio reply = meshtastic_FromRadio_init_zero; + reply.which_payload_variant = meshtastic_FromRadio_packet_tag; + reply.packet.from = DEVICE_NODE; + reply.packet.to = DEVICE_NODE; + reply.packet.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + reply.packet.decoded.portnum = meshtastic_PortNum_ADMIN_APP; + reply.packet.decoded.request_id = request.packet.id; + meshtastic_AdminMessage readBack = meshtastic_AdminMessage_init_zero; + readBack.which_payload_variant = meshtastic_AdminMessage_get_config_response_tag; + readBack.get_config_response.which_payload_variant = meshtastic_Config_lora_tag; + readBack.get_config_response.payload_variant.lora = lora; + reply.packet.decoded.payload.size = pb_encode_to_bytes( + reply.packet.decoded.payload.bytes, sizeof(reply.packet.decoded.payload.bytes), &meshtastic_AdminMessage_msg, &readBack); + deviceSends(reply); +} + /// Run the handshake against a device already in the state we want, so no config is pushed. static void bringReady() { @@ -344,20 +369,7 @@ void test_handshake_pushes_our_lora_config_with_transparent_mode(void) TEST_ASSERT_TRUE(sent[1].packet.decoded.want_response); TEST_ASSERT_EQUAL((int)TransparentRadio::State::Pushing, (int)radio->getState()); - meshtastic_FromRadio reply = meshtastic_FromRadio_init_zero; - reply.which_payload_variant = meshtastic_FromRadio_packet_tag; - reply.packet.from = DEVICE_NODE; - reply.packet.to = DEVICE_NODE; - reply.packet.which_payload_variant = meshtastic_MeshPacket_decoded_tag; - reply.packet.decoded.portnum = meshtastic_PortNum_ADMIN_APP; - reply.packet.decoded.request_id = sent[1].packet.id; - meshtastic_AdminMessage readBack = meshtastic_AdminMessage_init_zero; - readBack.which_payload_variant = meshtastic_AdminMessage_get_config_response_tag; - readBack.get_config_response.which_payload_variant = meshtastic_Config_lora_tag; - readBack.get_config_response.payload_variant.lora = pushed; - reply.packet.decoded.payload.size = pb_encode_to_bytes( - reply.packet.decoded.payload.bytes, sizeof(reply.packet.decoded.payload.bytes), &meshtastic_AdminMessage_msg, &readBack); - deviceSends(reply); + deviceAnswersReadBack(sent[1], pushed); radio->runOnce(); TEST_ASSERT_EQUAL((int)TransparentRadio::State::Ready, (int)radio->getState()); @@ -370,8 +382,9 @@ void test_device_already_set_up_goes_ready_without_pushing(void) bringReady(); } -// Firmware that predates transparent_mode drops the unknown field, so the read-back comes back without it. -// The radio must refuse that device rather than hand frames to a node that will route them itself. +// Firmware that predates transparent_mode drops the unknown field: it takes our radio settings, but the +// read-back comes back without transparent_mode. The radio must refuse that device rather than hand frames +// to a node that will route them itself. void test_firmware_without_transparent_mode_is_refused(void) { radio->runOnce(); @@ -381,19 +394,9 @@ void test_firmware_without_transparent_mode_is_refused(void) std::vector sent = takeToRadio(); TEST_ASSERT_EQUAL(2, sent.size()); - meshtastic_FromRadio reply = meshtastic_FromRadio_init_zero; - reply.which_payload_variant = meshtastic_FromRadio_packet_tag; - reply.packet.which_payload_variant = meshtastic_MeshPacket_decoded_tag; - reply.packet.decoded.portnum = meshtastic_PortNum_ADMIN_APP; - reply.packet.decoded.request_id = sent[1].packet.id; - meshtastic_AdminMessage readBack = meshtastic_AdminMessage_init_zero; - readBack.which_payload_variant = meshtastic_AdminMessage_get_config_response_tag; - readBack.get_config_response.which_payload_variant = meshtastic_Config_lora_tag; - readBack.get_config_response.payload_variant.lora = adminIn(sent[0]).set_config.payload_variant.lora; - readBack.get_config_response.payload_variant.lora.transparent_mode = false; - reply.packet.decoded.payload.size = pb_encode_to_bytes( - reply.packet.decoded.payload.bytes, sizeof(reply.packet.decoded.payload.bytes), &meshtastic_AdminMessage_msg, &readBack); - deviceSends(reply); + meshtastic_Config_LoRaConfig tookItAll = adminIn(sent[0]).set_config.payload_variant.lora; + tookItAll.transparent_mode = false; + deviceAnswersReadBack(sent[1], tookItAll); radio->runOnce(); TEST_ASSERT_EQUAL((int)TransparentRadio::State::Unsupported, (int)radio->getState()); @@ -403,6 +406,76 @@ void test_firmware_without_transparent_mode_is_refused(void) TEST_ASSERT_EQUAL_MESSAGE(1, fakeLink->opens, "an unsupported device is not retried"); } +// A release the device reports as older than transparent mode is refused before anything is written to it: +// old firmware would apply and save our region and preset, and only then fail the read-back. +void test_old_firmware_version_is_refused_before_any_write(void) +{ + radio->runOnce(); + takeToRadio(); + deviceSendsConfigDump(meshtastic_Config_LoRaConfig_init_zero, "2.7.15.1234abc"); + radio->runOnce(); + + TEST_ASSERT_EQUAL((int)TransparentRadio::State::Unsupported, (int)radio->getState()); + TEST_ASSERT_EQUAL_MESSAGE(0, takeToRadio().size(), "no set_config may reach old firmware"); +} + +// A device that rejects a LoRa config restores its old one whole, so transparent_mode reads back false +// there too. That is not old firmware: the radio waits and retries, and retries at once when this node's +// config changes, instead of writing the device off until restart. +void test_device_refusing_our_settings_is_retried_not_written_off(void) +{ + radio->runOnce(); + takeToRadio(); + meshtastic_Config_LoRaConfig deviceNow = meshtastic_Config_LoRaConfig_init_zero; + deviceNow.region = meshtastic_Config_LoRaConfig_RegionCode_EU_868; + deviceNow.use_preset = true; + deviceSendsConfigDump(deviceNow); + radio->runOnce(); + std::vector sent = takeToRadio(); + TEST_ASSERT_EQUAL(2, sent.size()); + + deviceAnswersReadBack(sent[1], deviceNow); // kept its own settings + radio->runOnce(); + TEST_ASSERT_EQUAL((int)TransparentRadio::State::Refused, (int)radio->getState()); + TEST_ASSERT_TRUE_MESSAGE(fakeLink->isOpen(), "a refusal is not a reason to drop the link"); + + config.lora.modem_preset = meshtastic_Config_LoRaConfig_ModemPreset_MEDIUM_FAST; + TEST_ASSERT_TRUE(radio->reconfigure()); + sent = takeToRadio(); + TEST_ASSERT_EQUAL_MESSAGE(2, sent.size(), "a config change retries straight away"); + TEST_ASSERT_EQUAL(meshtastic_AdminMessage_set_config_tag, adminIn(sent[0]).which_payload_variant); + TEST_ASSERT_EQUAL((int)TransparentRadio::State::Pushing, (int)radio->getState()); +} + +// With no region set here, pushing would set the device's region to UNSET (and turn its TX off) and persist +// that. Nothing is pushed until this node has a region; setting one pushes at once. +void test_no_region_here_leaves_the_device_alone_until_one_is_set(void) +{ + config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_UNSET; + initRegion(); + TEST_ASSERT_TRUE(radio->reconfigure()); + radio->runOnce(); + takeToRadio(); + meshtastic_Config_LoRaConfig deviceNow = meshtastic_Config_LoRaConfig_init_zero; + deviceNow.region = meshtastic_Config_LoRaConfig_RegionCode_EU_868; + deviceSendsConfigDump(deviceNow); + radio->runOnce(); + TEST_ASSERT_EQUAL((int)TransparentRadio::State::WaitingForRegion, (int)radio->getState()); + TEST_ASSERT_EQUAL_MESSAGE(0, takeToRadio().size(), "no admin traffic without a region"); + + Time::advanceTestMillis(60000); + radio->runOnce(); + TEST_ASSERT_EQUAL_MESSAGE((int)TransparentRadio::State::WaitingForRegion, (int)radio->getState(), + "waiting for a region is not a stuck handshake"); + + config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_US; + initRegion(); + TEST_ASSERT_TRUE(radio->reconfigure()); + std::vector sent = takeToRadio(); + TEST_ASSERT_EQUAL(2, sent.size()); + TEST_ASSERT_EQUAL(meshtastic_Config_LoRaConfig_RegionCode_US, adminIn(sent[0]).set_config.payload_variant.lora.region); +} + // --------------------------------------------------------------------------- // TX // --------------------------------------------------------------------------- @@ -473,9 +546,7 @@ void test_full_device_queue_holds_frames_until_space_is_reported(void) deviceSendsQueueFree(3); Time::advanceTestMillis(100); - radio->runOnce(); // reads the QueueStatus - Time::advanceTestMillis(100); - radio->runOnce(); + radio->runOnce(); // reads the QueueStatus, then forwards the held frame in the same pass std::vector sent = takeToRadio(); TEST_ASSERT_EQUAL(1, sent.size()); TEST_ASSERT_EQUAL_HEX32(0x5002, sent[0].packet.id); @@ -628,6 +699,9 @@ void setup() RUN_TEST(test_handshake_pushes_our_lora_config_with_transparent_mode); RUN_TEST(test_device_already_set_up_goes_ready_without_pushing); RUN_TEST(test_firmware_without_transparent_mode_is_refused); + RUN_TEST(test_old_firmware_version_is_refused_before_any_write); + RUN_TEST(test_device_refusing_our_settings_is_retried_not_written_off); + RUN_TEST(test_no_region_here_leaves_the_device_alone_until_one_is_set); RUN_TEST(test_frame_goes_out_at_its_due_time_with_header_and_ciphertext_only); RUN_TEST(test_cancelled_frame_never_reaches_the_device); RUN_TEST(test_full_device_queue_holds_frames_until_space_is_reported);