chore: KMP audit — commonize code, centralize utilities, eliminate dead abstractions (#5133)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
James Rich 2026-04-14 21:17:50 -05:00 committed by GitHub
parent 50ade01e55
commit 72b981f73b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
132 changed files with 2186 additions and 916 deletions

View file

@ -18,6 +18,7 @@ package org.meshtastic.feature.firmware
import android.content.Context
import co.touchlab.kermit.Logger
import com.eygraber.uri.toAndroidUri
import io.ktor.client.HttpClient
import io.ktor.client.request.get
import io.ktor.client.request.head
@ -32,7 +33,6 @@ import kotlinx.coroutines.withContext
import org.koin.core.annotation.Single
import org.meshtastic.core.common.util.CommonUri
import org.meshtastic.core.common.util.ioDispatcher
import org.meshtastic.core.common.util.toPlatformUri
import org.meshtastic.core.model.DeviceHardware
import java.io.File
import java.io.FileOutputStream
@ -188,7 +188,7 @@ class AndroidFirmwareFileHandler(private val context: Context, private val clien
if (!tempDir.exists()) tempDir.mkdirs()
try {
val platformUri = uri.toPlatformUri() as android.net.Uri
val platformUri = uri.toAndroidUri()
val inputStream = context.contentResolver.openInputStream(platformUri) ?: return@withContext null
ZipInputStream(inputStream).use { zipInput ->
var entry = zipInput.nextEntry
@ -225,9 +225,9 @@ class AndroidFirmwareFileHandler(private val context: Context, private val clien
override suspend fun getFileSize(file: FirmwareArtifact): Long = withContext(ioDispatcher) {
file.toLocalFileOrNull()?.takeIf { it.exists() }?.length()
?: context.contentResolver
.openAssetFileDescriptor(file.uri.toPlatformUri() as android.net.Uri, "r")
?.use { descriptor -> descriptor.length.takeIf { it >= 0L } }
?: context.contentResolver.openAssetFileDescriptor(file.uri.toAndroidUri(), "r")?.use { descriptor ->
descriptor.length.takeIf { it >= 0L }
}
?: 0L
}
@ -242,16 +242,13 @@ class AndroidFirmwareFileHandler(private val context: Context, private val clien
if (localFile != null && localFile.exists()) {
localFile.readBytes()
} else {
context.contentResolver.openInputStream(artifact.uri.toPlatformUri() as android.net.Uri)?.use {
it.readBytes()
} ?: throw IOException("Cannot open artifact: ${artifact.uri}")
context.contentResolver.openInputStream(artifact.uri.toAndroidUri())?.use { it.readBytes() }
?: throw IOException("Cannot open artifact: ${artifact.uri}")
}
}
override suspend fun importFromUri(uri: CommonUri): FirmwareArtifact? = withContext(ioDispatcher) {
val inputStream =
context.contentResolver.openInputStream(uri.toPlatformUri() as android.net.Uri)
?: return@withContext null
val inputStream = context.contentResolver.openInputStream(uri.toAndroidUri()) ?: return@withContext null
val tempFile = File(context.cacheDir, "firmware_update/ota_firmware.bin")
tempFile.parentFile?.mkdirs()
inputStream.use { input -> tempFile.outputStream().use { output -> input.copyTo(output) } }
@ -282,10 +279,10 @@ class AndroidFirmwareFileHandler(private val context: Context, private val clien
withContext(ioDispatcher) {
val inputStream =
source.toLocalFileOrNull()?.inputStream()
?: context.contentResolver.openInputStream(source.uri.toPlatformUri() as android.net.Uri)
?: context.contentResolver.openInputStream(source.uri.toAndroidUri())
?: throw IOException("Cannot open source URI")
val outputStream =
context.contentResolver.openOutputStream(destinationUri.toPlatformUri() as android.net.Uri)
context.contentResolver.openOutputStream(destinationUri.toAndroidUri())
?: throw IOException("Cannot open content URI for writing")
inputStream.use { input -> outputStream.use { output -> input.copyTo(output) } }

View file

@ -163,9 +163,7 @@ fun FirmwareUpdateScreen(onNavigateUp: () -> Unit, viewModel: FirmwareUpdateView
uri?.let { viewModel.startUpdateFromFile(it) }
}
val saveFileLauncher = rememberSaveFileLauncher { meshtasticUri ->
viewModel.saveDfuFile(CommonUri.parse(meshtasticUri.uriString))
}
val saveFileLauncher = rememberSaveFileLauncher { uri -> viewModel.saveDfuFile(uri) }
val actions =
remember(viewModel, onNavigateUp) {

View file

@ -36,6 +36,7 @@ import kotlinx.coroutines.withTimeoutOrNull
import org.jetbrains.compose.resources.StringResource
import org.koin.core.annotation.KoinViewModel
import org.meshtastic.core.common.util.CommonUri
import org.meshtastic.core.common.util.safeCatching
import org.meshtastic.core.database.entity.FirmwareRelease
import org.meshtastic.core.database.entity.FirmwareReleaseType
import org.meshtastic.core.datastore.BootloaderWarningDataSource
@ -123,9 +124,12 @@ class FirmwareUpdateViewModel(
override fun onCleared() {
super.onCleared()
// viewModelScope is already cancelled when onCleared() runs, so use a standalone scope
// for fire-and-forget cleanup of temporary firmware files.
kotlinx.coroutines.CoroutineScope(NonCancellable).launch {
// viewModelScope is already cancelled when onCleared() runs, so launch cleanup in a
// standalone scope. SupervisorJob prevents the coroutine from propagating failures to a
// shared parent, and NonCancellable on the launch keeps cleanup running even if the scope
// is cancelled concurrently.
@OptIn(kotlinx.coroutines.DelicateCoroutinesApi::class)
kotlinx.coroutines.GlobalScope.launch(NonCancellable) {
tempFirmwareFile = cleanupTemporaryFiles(fileHandler, tempFirmwareFile)
}
}
@ -147,7 +151,7 @@ class FirmwareUpdateViewModel(
updateJob =
viewModelScope.launch {
_state.value = FirmwareUpdateState.Checking
runCatching {
safeCatching {
val ourNode = nodeRepository.myNodeInfo.value
val address = radioPrefs.devAddr.value?.drop(1)
if (address == null || ourNode == null) {
@ -200,7 +204,6 @@ class FirmwareUpdateViewModel(
}
}
.onFailure { e ->
if (e is CancellationException) throw e
Logger.e(e) { "Error checking for updates" }
val unknownError = UiText.Resource(Res.string.firmware_update_unknown_error)
_state.value =
@ -390,7 +393,7 @@ private suspend fun cleanupTemporaryFiles(
fileHandler: FirmwareFileHandler,
tempFirmwareFile: FirmwareArtifact?,
): FirmwareArtifact? {
runCatching {
safeCatching {
tempFirmwareFile?.takeIf { it.isTemporary }?.let { fileHandler.deleteFile(it) }
fileHandler.cleanupAllTemporaryFiles()
}

View file

@ -38,6 +38,7 @@ import org.meshtastic.core.ble.BleWriteType
import org.meshtastic.core.ble.MeshtasticBleConstants.OTA_NOTIFY_CHARACTERISTIC
import org.meshtastic.core.ble.MeshtasticBleConstants.OTA_SERVICE_UUID
import org.meshtastic.core.ble.MeshtasticBleConstants.OTA_WRITE_CHARACTERISTIC
import org.meshtastic.core.common.util.safeCatching
import kotlin.time.Duration
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.seconds
@ -78,7 +79,7 @@ class BleOtaTransport(
}
@Suppress("MagicNumber")
override suspend fun connect(): Result<Unit> = runCatching {
override suspend fun connect(): Result<Unit> = safeCatching {
Logger.i { "BLE OTA: Waiting $REBOOT_DELAY for device to reboot into OTA mode..." }
delay(REBOOT_DELAY)
@ -152,7 +153,7 @@ class BleOtaTransport(
sizeBytes: Long,
sha256Hash: String,
onHandshakeStatus: suspend (OtaHandshakeStatus) -> Unit,
): Result<Unit> = runCatching {
): Result<Unit> = safeCatching {
val command = OtaCommand.StartOta(sizeBytes, sha256Hash)
val packetsSent = sendCommand(command)
@ -189,7 +190,7 @@ class BleOtaTransport(
data: ByteArray,
chunkSize: Int,
onProgress: suspend (Float) -> Unit,
): Result<Unit> = runCatching {
): Result<Unit> = safeCatching {
val totalBytes = data.size
var sentBytes = 0
@ -215,7 +216,7 @@ class BleOtaTransport(
if (nextSentBytes >= totalBytes && isLastPacketOfChunk) {
sentBytes = nextSentBytes
onProgress(1.0f)
return@runCatching Unit
return@safeCatching Unit
}
}
is OtaResponse.Error -> {

View file

@ -45,7 +45,7 @@ internal fun calculateMacPlusOne(macAddress: String): String {
if (parts.size != MAC_PARTS_COUNT) return macAddress
val lastByte = parts[MAC_PARTS_COUNT - 1].toIntOrNull(HEX_RADIX) ?: return macAddress
val incremented = ((lastByte + 1) and BYTE_MASK).toString(HEX_RADIX).uppercase().padStart(2, '0')
return parts.take(MAC_PARTS_COUNT - 1).joinToString(":") + ":" + incremented
return "${parts.take(MAC_PARTS_COUNT - 1).joinToString(":")}:$incremented"
}
/**

View file

@ -32,6 +32,7 @@ import kotlinx.coroutines.delay
import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeout
import org.meshtastic.core.common.util.ioDispatcher
import org.meshtastic.core.common.util.safeCatching
/**
* WiFi/TCP transport implementation for ESP32 Unified OTA protocol.
@ -54,7 +55,7 @@ class WifiOtaTransport(private val deviceIpAddress: String, private val port: In
/** Connect to the device via TCP using Ktor raw sockets. */
override suspend fun connect(): Result<Unit> = withContext(ioDispatcher) {
runCatching {
safeCatching {
Logger.i { "WiFi OTA: Connecting to $deviceIpAddress:$port" }
val selector = SelectorManager(ioDispatcher)
@ -82,7 +83,7 @@ class WifiOtaTransport(private val deviceIpAddress: String, private val port: In
sizeBytes: Long,
sha256Hash: String,
onHandshakeStatus: suspend (OtaHandshakeStatus) -> Unit,
): Result<Unit> = runCatching {
): Result<Unit> = safeCatching {
val command = OtaCommand.StartOta(sizeBytes, sha256Hash)
sendCommand(command)
@ -116,7 +117,7 @@ class WifiOtaTransport(private val deviceIpAddress: String, private val port: In
chunkSize: Int,
onProgress: suspend (Float) -> Unit,
): Result<Unit> = withContext(ioDispatcher) {
runCatching {
safeCatching {
if (!isConnected) {
throw OtaProtocolException.TransferFailed("Not connected")
}

View file

@ -46,6 +46,7 @@ import org.meshtastic.core.ble.BleDevice
import org.meshtastic.core.ble.BleScanner
import org.meshtastic.core.ble.BleWriteType
import org.meshtastic.core.ble.DEFAULT_BLE_WRITE_VALUE_LENGTH
import org.meshtastic.core.common.util.safeCatching
import org.meshtastic.feature.firmware.ota.calculateMacPlusOne
import org.meshtastic.feature.firmware.ota.scanForBleDevice
import kotlin.time.Duration
@ -91,7 +92,7 @@ class SecureDfuTransport(
*
* The caller must have already released the mesh-service BLE connection before calling this.
*/
suspend fun triggerButtonlessDfu(): Result<Unit> = runCatching {
suspend fun triggerButtonlessDfu(): Result<Unit> = safeCatching {
Logger.i { "DFU: Scanning for device $address to trigger buttonless DFU..." }
val device =
@ -152,7 +153,7 @@ class SecureDfuTransport(
* Scans for the device in DFU mode (address or address+1) and establishes the GATT connection, enabling
* notifications on the Control Point.
*/
suspend fun connectToDfuMode(): Result<Unit> = runCatching {
suspend fun connectToDfuMode(): Result<Unit> = safeCatching {
val dfuAddress = calculateMacPlusOne(address)
val targetAddresses = setOf(address, dfuAddress)
Logger.i { "DFU: Scanning for DFU mode device at $targetAddresses..." }
@ -210,7 +211,7 @@ class SecureDfuTransport(
* PRN is explicitly disabled (set to 0) for the init packet per the Nordic DFU library convention the init packet
* is small (<512 bytes, fits in a single object) and does not benefit from flow control.
*/
suspend fun transferInitPacket(initPacket: ByteArray): Result<Unit> = runCatching {
suspend fun transferInitPacket(initPacket: ByteArray): Result<Unit> = safeCatching {
Logger.i { "DFU: Transferring init packet (${initPacket.size} bytes)..." }
setPrn(0)
transferObjectWithRetry(DfuObjectType.COMMAND, initPacket, onProgress = null)
@ -231,12 +232,13 @@ class SecureDfuTransport(
* @param firmware Raw bytes of the `.bin` file.
* @param onProgress Callback receiving progress in [0.0, 1.0].
*/
suspend fun transferFirmware(firmware: ByteArray, onProgress: suspend (Float) -> Unit): Result<Unit> = runCatching {
Logger.i { "DFU: Transferring firmware (${firmware.size} bytes)..." }
setPrn(PRN_INTERVAL)
transferObjectWithRetry(DfuObjectType.DATA, firmware, onProgress)
Logger.i { "DFU: Firmware transferred and executed." }
}
suspend fun transferFirmware(firmware: ByteArray, onProgress: suspend (Float) -> Unit): Result<Unit> =
safeCatching {
Logger.i { "DFU: Transferring firmware (${firmware.size} bytes)..." }
setPrn(PRN_INTERVAL)
transferObjectWithRetry(DfuObjectType.DATA, firmware, onProgress)
Logger.i { "DFU: Firmware transferred and executed." }
}
// ---------------------------------------------------------------------------
// Abort & teardown