diff --git a/src/mesh/RadioInterface.cpp b/src/mesh/RadioInterface.cpp index 3eb32381f7..59847b4bb3 100644 --- a/src/mesh/RadioInterface.cpp +++ b/src/mesh/RadioInterface.cpp @@ -837,9 +837,11 @@ uint32_t RadioInterface::getTxDelayMsec(const meshtastic_MeshPacket *p) void RadioInterface::noteFrameEnd(uint32_t endMs, const char *what) { - if (lastFrameEndMs && (int32_t)(endMs - lastFrameEndMs) < 0) + // Both writers are the radio worker, so this read-modify-write races with no other write. + const uint32_t held = lastFrameEndMs.load(std::memory_order_relaxed); + if (held && (int32_t)(endMs - held) < 0) return; // a frame handed to us late, after one that ended later - lastFrameEndMs = Time::skipZero(endMs); + lastFrameEndMs.store(Time::skipZero(endMs), std::memory_order_relaxed); lastFrameEndWhat = what; } @@ -864,14 +866,15 @@ uint32_t RadioInterface::getAnchoredSlotDelayMsec(uint32_t slots, meshtastic_Mes // Two nodes that redraw after the same frame count their slots from its air end rather than from // their own handling of it, so their grids line up however differently they got there. Taking // only one parity's slots then rules out the same slot, so the two are always a slot time apart. - const uint32_t sinceEnd = lastFrameEndMs ? Time::getMillis() - lastFrameEndMs : 0; + const uint32_t frameEnd = lastFrameEndMs.load(std::memory_order_relaxed); + const uint32_t sinceEnd = frameEnd ? Time::getMillis() - frameEnd : 0; const uint32_t ownParitySlots = slots / 2 ? slots / 2 : 1; const uint32_t delay = anchoredSlotDelayMsec(sinceEnd, slotTimeMsec, random(0, ownParitySlots), parity); LOG_TRACE("TX slot anchor: %u ms into slot %u of %u, parity %u, %u ms after %s end", (unsigned)delay, (unsigned)((sinceEnd + delay) / slotTimeMsec), (unsigned)slots, (unsigned)(parity == meshtastic_MeshPacket_SlotParity_SLOT_PARITY_ODD ? 1 : 0), (unsigned)sinceEnd, - lastFrameEndMs ? lastFrameEndWhat : "no"); + frameEnd ? lastFrameEndWhat : "no"); return delay; } diff --git a/src/mesh/RadioInterface.h b/src/mesh/RadioInterface.h index fe01d36b37..310c19f27e 100644 --- a/src/mesh/RadioInterface.h +++ b/src/mesh/RadioInterface.h @@ -6,6 +6,7 @@ #include "PointerQueue.h" #include "airtime.h" #include "error.h" +#include #include #if HAS_LORA_FEM @@ -111,8 +112,12 @@ class RadioInterface /** millis() when the last frame this node sent or heard left the air, 0 before the first. * Only read by a draw that was asked for a slot parity; the driver keeps it up to date so any - * module can ask at any time. */ - uint32_t lastFrameEndMs = 0; + * module can ask at any time. + * + * Atomic because the radio worker writes it from onNotify() while a draw for a packet being + * queued reads it on whatever thread called send(). Relaxed is enough: it publishes nothing + * but itself, and losing the race just anchors that one draw to the frame before last. */ + std::atomic lastFrameEndMs{0}; /** "tx" or "rx" for the frame lastFrameEndMs came from, for the trace line */ const char *lastFrameEndWhat = "none"; uint16_t preambleLength = 16; // 8 is default, but we use longer to increase the amount of sleep time when receiving diff --git a/test/test_radio/test_main.cpp b/test/test_radio/test_main.cpp index 3464a3d3bd..ba37990b0f 100644 --- a/test/test_radio/test_main.cpp +++ b/test/test_radio/test_main.cpp @@ -80,7 +80,7 @@ class TestableRadioInterface : public RadioInterface size_t beginSendingPublic(meshtastic_MeshPacket *p) { return beginSending(p); } meshtastic_MeshPacket *getSendingPacket() const { return sendingPacket; } void clearSendingPacketForTest() { sendingPacket = nullptr; } - uint32_t getLastFrameEndMs() const { return lastFrameEndMs; } + uint32_t getLastFrameEndMs() const { return lastFrameEndMs.load(std::memory_order_relaxed); } // Override reconfigure to call the base which invokes applyModemConfig() bool reconfigure() override { return RadioInterface::reconfigure(); }