From bf2338ce4ec52f5d346a7fa87a2681943a0cc6f3 Mon Sep 17 00:00:00 2001 From: Jeremiah K <17190268+jeremiah-k@users.noreply.github.com> Date: Sun, 21 Jun 2026 17:36:57 -0500 Subject: [PATCH] fix(network): preserve TCP reconnect backoff on short sessions (#5893) --- .../core/network/transport/TcpTransport.kt | 32 ++++++++++-- .../transport/ShouldResetBackoffTest.kt | 50 +++++++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 core/network/src/jvmTest/kotlin/org/meshtastic/core/network/transport/ShouldResetBackoffTest.kt diff --git a/core/network/src/jvmAndroidMain/kotlin/org/meshtastic/core/network/transport/TcpTransport.kt b/core/network/src/jvmAndroidMain/kotlin/org/meshtastic/core/network/transport/TcpTransport.kt index ea32c7474..df858ea90 100644 --- a/core/network/src/jvmAndroidMain/kotlin/org/meshtastic/core/network/transport/TcpTransport.kt +++ b/core/network/src/jvmAndroidMain/kotlin/org/meshtastic/core/network/transport/TcpTransport.kt @@ -35,6 +35,16 @@ import java.net.SocketTimeoutException import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicInteger +/** + * Decides whether to reset the reconnect backoff based on session data and uptime. + * + * Only sessions that lasted at least [thresholdMs] with actual data exchange are considered stable enough to warrant a + * backoff reset. Short sessions — e.g., an ESP32 dumping config then closing the socket — keep the growing backoff so + * the radio has time to recover between attempts. + */ +internal fun shouldResetBackoff(hadData: Boolean, sessionUptimeMs: Long, thresholdMs: Long): Boolean = + hadData && sessionUptimeMs >= thresholdMs + /** * Shared JVM TCP transport for Meshtastic radios. * @@ -76,6 +86,14 @@ class TcpTransport( const val SOCKET_RETRIES = 18 // 18 * 5s = 90s inactivity before disconnect const val TIMEOUT_LOG_INTERVAL = 5 private const val MILLIS_PER_SECOND = 1_000L + + /** + * Minimum session duration for backoff to reset. Sessions shorter than this that ended in peer-EOF are treated + * as transient firmware-side disconnects (e.g., ESP32 light sleep closing the TCP PhoneAPI session after a + * config dump) and do NOT reset the backoff — the growing delay gives the radio time to recover between + * attempts instead of hammering it at 1 Hz. + */ + const val SHORT_SESSION_THRESHOLD_MS = 30_000L } private val codec = @@ -176,12 +194,18 @@ class TcpTransport( false } - // Reset backoff after a connection that successfully exchanged data, - // so transient firmware-side disconnects recover quickly. - if (hadData) { - Logger.d { "$logTag: [$address] Resetting backoff after successful data exchange" } + // Reset backoff only after a session that lasted long enough to indicate a real connection, + // not a short config-dump-then-EOF from a sleeping radio. Short sessions keep the backoff + // growing so the radio has time to recover between reconnect attempts. + val sessionUptime = if (connectionStartTime > 0) nowMillis - connectionStartTime else 0 + if (shouldResetBackoff(hadData, sessionUptime, SHORT_SESSION_THRESHOLD_MS)) { + Logger.d { "$logTag: [$address] Resetting backoff after successful data exchange (${sessionUptime}ms)" } retryCount = 1 backoff = MIN_BACKOFF_MILLIS + } else if (hadData) { + Logger.d { + "$logTag: [$address] Short session (${sessionUptime}ms) — keeping backoff at ${backoff / MILLIS_PER_SECOND}s" + } } val delaySec = backoff / MILLIS_PER_SECOND diff --git a/core/network/src/jvmTest/kotlin/org/meshtastic/core/network/transport/ShouldResetBackoffTest.kt b/core/network/src/jvmTest/kotlin/org/meshtastic/core/network/transport/ShouldResetBackoffTest.kt new file mode 100644 index 000000000..906e2893c --- /dev/null +++ b/core/network/src/jvmTest/kotlin/org/meshtastic/core/network/transport/ShouldResetBackoffTest.kt @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.network.transport + +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class ShouldResetBackoffTest { + + private val threshold = 30_000L + + @Test + fun `no data never resets backoff`() { + assertFalse(shouldResetBackoff(hadData = false, sessionUptimeMs = 0, thresholdMs = threshold)) + assertFalse(shouldResetBackoff(hadData = false, sessionUptimeMs = 60_000, thresholdMs = threshold)) + } + + @Test + fun `data below threshold does not reset backoff`() { + assertFalse(shouldResetBackoff(hadData = true, sessionUptimeMs = 0, thresholdMs = threshold)) + assertFalse(shouldResetBackoff(hadData = true, sessionUptimeMs = 1_000, thresholdMs = threshold)) + assertFalse(shouldResetBackoff(hadData = true, sessionUptimeMs = 29_999, thresholdMs = threshold)) + } + + @Test + fun `data at exactly threshold resets backoff`() { + assertTrue(shouldResetBackoff(hadData = true, sessionUptimeMs = 30_000, thresholdMs = threshold)) + } + + @Test + fun `data above threshold resets backoff`() { + assertTrue(shouldResetBackoff(hadData = true, sessionUptimeMs = 30_001, thresholdMs = threshold)) + assertTrue(shouldResetBackoff(hadData = true, sessionUptimeMs = 600_000, thresholdMs = threshold)) + } +}