Skip to content

Commit d26aeb4

Browse files
jeremiah-kjeremiah-k
committed
fix(firmware): Harden ESP32 OTA post-confirm connects
1 parent 867d8e5 commit d26aeb4

8 files changed

Lines changed: 539 additions & 85 deletions

File tree

‎.skills/compose-ui/strings-index.txt‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎core/resources/src/commonMain/composeResources/values/strings.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,7 @@
649649
<string name="firmware_update_retrieval_failed">Could not retrieve firmware file.</string>
650650
<string name="firmware_update_retry">Retry</string>
651651
<string name="firmware_update_save_dfu_file">Please save the .uf2 file to your device's DFU drive.</string>
652+
<string name="firmware_update_searching_device">Searching for OTA device on the network...</string>
652653
<string name="firmware_update_select_file">Select Local File</string>
653654
<string name="firmware_update_slow_bootloader_hint">Updates are slow on this device's bootloader. Flashing the OTAFIX bootloader enables much faster BLE updates.</string>
654655
<string name="firmware_update_source_local">Source: Local File</string>

‎core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/UIViewModel.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,12 @@ class UIViewModel(
256256
serviceRepository.clientNotification
257257
.filterNotNull()
258258
.onEach { notification ->
259+
// OTA status notifications (e.g. "Rebooting to WiFi OTA") are consumed by the firmware update
260+
// preflight gate — don't show a popup dialog for them.
261+
val msg = notification.message.trim()
262+
if (msg.contains("OTA") && (msg.startsWith("Rebooting to") || msg.startsWith("Cannot start OTA"))) {
263+
return@onEach
264+
}
259265
val isCompromised = notification.low_entropy_key != null || notification.duplicated_public_key != null
260266
showAlert(
261267
titleRes = Res.string.client_notification,

‎feature/firmware/src/commonMain/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransport.kt‎

Lines changed: 104 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.meshtastic.feature.firmware.ota
1818

1919
import co.touchlab.kermit.Logger
20+
import kotlinx.coroutines.CancellationException
2021
import kotlinx.coroutines.CompletableDeferred
2122
import kotlinx.coroutines.CoroutineDispatcher
2223
import kotlinx.coroutines.CoroutineScope
@@ -66,79 +67,110 @@ class BleOtaTransport(
6667
private suspend fun scanForOtaDevice(): BleDevice? {
6768
val otaAddress = calculateMacPlusOne(address)
6869
val targetAddresses = setOf(address, otaAddress)
69-
Logger.i { "BLE OTA: Will match addresses: $targetAddresses" }
70+
Logger.i { "BLE OTA: Will match addresses: ${targetAddresses.joinToString(", ") { maskMac(it) }}" }
7071

7172
return scanForBleDevice(scanner = scanner, tag = "BLE OTA", serviceUuid = OTA_SERVICE_UUID) {
7273
it.address in targetAddresses
7374
}
7475
}
7576

7677
@Suppress("MagicNumber", "ThrowsCount") // distinct exception types for scan-miss, connect-fail, and timeout
77-
override suspend fun connect(): Result<Unit> = safeCatching {
78-
Logger.i { "BLE OTA: Waiting $REBOOT_DELAY for device to reboot into OTA mode..." }
79-
delay(REBOOT_DELAY)
80-
81-
Logger.i { "BLE OTA: Connecting to $address using Kable..." }
82-
83-
val device =
84-
scanForOtaDevice()
85-
?: throw OtaProtocolException.ConnectionFailed(
86-
"Device not found at address $address. " +
87-
"Ensure the device has rebooted into OTA mode and is advertising.",
88-
)
89-
90-
bleConnection.connectionState
91-
.onEach { state ->
92-
Logger.d { "BLE OTA: Connection state changed to $state" }
93-
isConnected = state is BleConnectionState.Connected
78+
override suspend fun connect(): Result<Unit> {
79+
val result = safeCatching {
80+
Logger.i { "BLE OTA: Waiting $REBOOT_DELAY for device to reboot into OTA mode..." }
81+
delay(REBOOT_DELAY)
82+
83+
Logger.i { "BLE OTA: Connecting to ${maskMac(address)} using Kable..." }
84+
85+
val device =
86+
scanForOtaDevice()
87+
?: throw OtaProtocolException.ConnectionFailed(
88+
"Device not found at address $address. " +
89+
"Ensure the device has rebooted into OTA mode and is advertising.",
90+
)
91+
92+
bleConnection.connectionState
93+
.onEach { state ->
94+
Logger.d { "BLE OTA: Connection state changed to $state" }
95+
isConnected = state is BleConnectionState.Connected
96+
}
97+
.launchIn(transportScope)
98+
99+
try {
100+
val finalState = bleConnection.connectAndAwait(device, CONNECTION_TIMEOUT)
101+
if (finalState is BleConnectionState.Disconnected) {
102+
Logger.w { "BLE OTA: Failed to connect to ${maskMac(device.address)} (state=$finalState)" }
103+
throw OtaProtocolException.ConnectionFailed(
104+
"Failed to connect to device at address ${device.address}",
105+
)
106+
}
107+
} catch (@Suppress("SwallowedException") e: kotlinx.coroutines.TimeoutCancellationException) {
108+
Logger.w { "BLE OTA: Timed out waiting to connect to ${maskMac(device.address)}. Error: ${e.message}" }
109+
throw OtaProtocolException.Timeout("Timed out connecting to device at address ${device.address}")
94110
}
95-
.launchIn(transportScope)
96111

97-
try {
98-
val finalState = bleConnection.connectAndAwait(device, CONNECTION_TIMEOUT)
99-
if (finalState is BleConnectionState.Disconnected) {
100-
Logger.w { "BLE OTA: Failed to connect to ${device.address} (state=$finalState)" }
101-
throw OtaProtocolException.ConnectionFailed("Failed to connect to device at address ${device.address}")
102-
}
103-
} catch (@Suppress("SwallowedException") e: kotlinx.coroutines.TimeoutCancellationException) {
104-
Logger.w { "BLE OTA: Timed out waiting to connect to ${device.address}. Error: ${e.message}" }
105-
throw OtaProtocolException.Timeout("Timed out connecting to device at address ${device.address}")
112+
Logger.i { "BLE OTA: Connected to ${maskMac(device.address)}, discovering services..." }
113+
prepareOtaProfile()
106114
}
115+
if (result.isFailure) {
116+
closeAfterFailedConnect()
117+
}
118+
return result
119+
}
107120

108-
Logger.i { "BLE OTA: Connected to ${device.address}, discovering services..." }
109-
110-
bleConnection.profile(OTA_SERVICE_UUID) { service ->
111-
// Log negotiated MTU for diagnostics
112-
val maxLen = bleConnection.maximumWriteValueLength(BleWriteType.WITHOUT_RESPONSE)
113-
Logger.i { "BLE OTA: Service ready. Max write value length: $maxLen bytes" }
121+
private suspend fun closeAfterFailedConnect() {
122+
try {
123+
close()
124+
} catch (e: CancellationException) {
125+
throw e
126+
} catch (@Suppress("TooGenericExceptionCaught") e: Exception) {
127+
Logger.w(e) { "BLE OTA: Failed to close after connection failure" }
128+
}
129+
}
114130

115-
// Collect responses. onSubscription fires when the CCCD write completes — a precise readiness
116-
// signal; the settle below is a conservative cushion.
117-
val subscribed = CompletableDeferred<Unit>()
118-
service
119-
.observe(txChar) {
120-
Logger.d { "BLE OTA: TX characteristic subscribed" }
121-
subscribed.complete(Unit)
122-
}
123-
.onEach { notifyBytes ->
124-
try {
125-
val response = notifyBytes.decodeToString()
126-
Logger.d { "BLE OTA: Received response: $response" }
127-
responseChannel.trySend(response)
128-
} catch (@Suppress("TooGenericExceptionCaught") e: Exception) {
129-
Logger.e(e) { "BLE OTA: Failed to decode response bytes" }
131+
private suspend fun prepareOtaProfile() {
132+
try {
133+
bleConnection.profile(OTA_SERVICE_UUID) { service ->
134+
// Log negotiated MTU for diagnostics
135+
val maxLen = bleConnection.maximumWriteValueLength(BleWriteType.WITHOUT_RESPONSE)
136+
Logger.i { "BLE OTA: Service ready. Max write value length: $maxLen bytes" }
137+
138+
// Collect responses. onSubscription fires when the CCCD write completes — a precise readiness
139+
// signal; the settle below is a conservative cushion.
140+
val subscribed = CompletableDeferred<Unit>()
141+
service
142+
.observe(txChar) {
143+
Logger.d { "BLE OTA: TX characteristic subscribed" }
144+
subscribed.complete(Unit)
130145
}
131-
}
132-
.catch { e ->
133-
if (!subscribed.isCompleted) subscribed.completeExceptionally(e)
134-
Logger.e(e) { "BLE OTA: Error in TX characteristic subscription" }
135-
}
136-
.launchIn(this)
146+
.onEach { notifyBytes ->
147+
try {
148+
val response = notifyBytes.decodeToString()
149+
Logger.d { "BLE OTA: Received response: $response" }
150+
responseChannel.trySend(response)
151+
} catch (@Suppress("TooGenericExceptionCaught") e: Exception) {
152+
Logger.e(e) { "BLE OTA: Failed to decode response bytes" }
153+
}
154+
}
155+
.catch { e ->
156+
if (!subscribed.isCompleted) subscribed.completeExceptionally(e)
157+
Logger.e(e) { "BLE OTA: Error in TX characteristic subscription" }
158+
}
159+
.launchIn(this)
137160

138-
subscribed.await()
139-
// Conservative settle after CCCD confirmation before issuing commands.
140-
delay(SUBSCRIPTION_SETTLE)
141-
Logger.i { "BLE OTA: Service discovered and ready" }
161+
subscribed.await()
162+
// Conservative settle after CCCD confirmation before issuing commands.
163+
delay(SUBSCRIPTION_SETTLE)
164+
Logger.i { "BLE OTA: Service discovered and ready" }
165+
}
166+
} catch (e: CancellationException) {
167+
throw e
168+
} catch (@Suppress("TooGenericExceptionCaught") e: Exception) {
169+
Logger.w(e) { "BLE OTA: Connected over GATT but OTA service was not ready" }
170+
throw OtaProtocolException.ConnectionFailed(
171+
"Connected over BLE, but the OTA service was not available yet",
172+
e,
173+
)
142174
}
143175
}
144176

@@ -263,10 +295,20 @@ class BleOtaTransport(
263295
OtaProtocolException.TransferFailed("Transfer failed: ${error.message}")
264296
}
265297

298+
/**
299+
* Masks a BLE MAC address for logging: preserves the OUI (first 3 octets) and last octet for diagnostics while
300+
* hiding the unique middle portion. Privacy rule: never log raw PII.
301+
*/
302+
private fun maskMac(address: String): String =
303+
address.takeIf { it.length >= 17 }?.let { "${it.substring(0, 8)}:**:**:**:${it.substring(15)}" } ?: address
304+
266305
override suspend fun close() {
267-
bleConnection.disconnect()
268-
isConnected = false
269-
transportScope.cancel()
306+
try {
307+
bleConnection.disconnect()
308+
} finally {
309+
isConnected = false
310+
transportScope.cancel()
311+
}
270312
}
271313

272314
private suspend fun sendCommand(command: OtaCommand): Int {

0 commit comments

Comments
 (0)