Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,19 @@ open class FirmwareReleaseRepositoryImpl(
/** Single-flight guard so concurrent collectors share one network refresh. */
private val refreshMutex = Mutex()

/** Guards [seedChecked] so concurrent collectors decode the bundled snapshot at most once per process. */
/**
* Guards [bundledSnapshot] decode so concurrent collectors decode the bundled JSON at most once per process. The
* apply/skip decision itself is re-evaluated every time against the CURRENT active DB — the active Room database
* switches per selected device, so a one-shot seed gate would miss a freshly activated DB whose `firmware_release`
* rows are empty.
*/
private val seedMutex = Mutex()

@Volatile private var seedChecked = false
/** Decoded bundled snapshot cached for the process lifetime; the asset file never changes between launches. */
@Volatile private var bundledSnapshot: NetworkFirmwareReleases? = null

/** Set when the bundled asset is missing or un-decodable, so we don't retry on every collection. */
@Volatile private var bundleDecodeFailed = false

override val stableRelease: Flow<FirmwareRelease?> = getLatestFirmware(FirmwareReleaseType.STABLE)

Expand Down Expand Up @@ -87,35 +96,50 @@ open class FirmwareReleaseRepositoryImpl(
/**
* Applies the bundled snapshot per release type whenever it is newer than what is cached for that type — not just
* when the cache is empty. The bundle is refreshed weekly in CI, so an app update carries fresh data even for users
* whose network path to api.meshtastic.org chronically fails. Checked once per process; a cache that is already
* newer (from a successful network refresh) is never regressed, and a type the bundle doesn't ship is left
* untouched.
* whose network path to api.meshtastic.org chronically fails. The decoded snapshot is cached for the process; the
* apply/skip decision is re-evaluated every call against the CURRENT active DB, because that DB switches per
* selected device and a one-shot seed gate would skip a freshly activated DB whose `firmware_release` rows are
* empty. A cache that is already newer (from a successful network refresh) is never regressed, and a type the
* bundle doesn't ship is left untouched.
*/
private suspend fun ensureSeeded() {
if (seedChecked) return
seedMutex.withLock {
if (seedChecked) return
safeCatching {
val bundled = assetReader.decode<NetworkFirmwareReleases>("firmware_releases.json", json)?.releases
if (bundled == null) {
Logger.w { "FirmwareReleaseRepository: no bundled releases available to seed from" }
} else {
val toApply =
listOf(
FirmwareReleaseType.STABLE to bundled.stable,
FirmwareReleaseType.ALPHA to bundled.alpha,
)
.filter { (type, releases) -> isBundleNewerFor(type, releases) }
.toMap()
if (toApply.isNotEmpty()) {
Logger.i { "FirmwareReleaseRepository: applying bundled snapshot for ${toApply.keys}" }
localDataSource.replaceFirmwareReleases(toApply)
if (bundleDecodeFailed) return // don't retry the bundled asset on every collection once it has failed

// seedMutex guards only the decode + snapshot cache; the DB apply runs outside it (so concurrent
// collectors don't block on a Room write or on refreshMutex) and under refreshMutex (so it can't
// race singleFlightRefresh and overwrite fresher data that just arrived from the API).
val bundled =
seedMutex.withLock {
// Decode the bundled JSON once per process; the asset never changes between launches.
if (bundledSnapshot == null && !bundleDecodeFailed) {
safeCatching { assetReader.decode<NetworkFirmwareReleases>("firmware_releases.json", json) }
.onSuccess { snapshot -> bundledSnapshot = snapshot }
.onFailure { e ->
Logger.w(e) { "FirmwareReleaseRepository: failed to decode bundled JSON" }
bundleDecodeFailed = true
}
// Decode returning null (asset missing) also stops further retries.
if (bundledSnapshot == null && !bundleDecodeFailed) {
Logger.w { "FirmwareReleaseRepository: no bundled releases available to seed from" }
bundleDecodeFailed = true
}
}
bundledSnapshot?.releases
} ?: return

safeCatching {
refreshMutex.withLock {
val toApply =
listOf(FirmwareReleaseType.STABLE to bundled.stable, FirmwareReleaseType.ALPHA to bundled.alpha)
.filter { (type, releases) -> isBundleNewerFor(type, releases) }
.toMap()
if (toApply.isNotEmpty()) {
Logger.i { "FirmwareReleaseRepository: applying bundled snapshot for ${toApply.keys}" }
localDataSource.replaceFirmwareReleases(toApply)
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
.onFailure { e -> Logger.w(e) { "FirmwareReleaseRepository: failed to seed cache from bundled JSON" } }
seedChecked = true
}
.onFailure { e -> Logger.w(e) { "FirmwareReleaseRepository: failed to apply bundled snapshot" } }
}

/** True when [bundled] contains a release newer than anything cached for [type]. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ internal fun <T : Any> staleWhileRevalidateFlow(
networkTimeoutMs: Long? = DEFAULT_NETWORK_TIMEOUT_MS,
tag: String = "StaleWhileRevalidate",
): Flow<T?> = flow {
val cached = loadFromCache()
val cached =
safeCatching { loadFromCache() }
.onFailure { e -> Logger.w(e) { "$tag: cache read failed" } }
.getOrNull()
emit(cached)

if (!shouldFetch(cached)) return@flow
Expand All @@ -78,7 +81,10 @@ internal fun <T : Any> staleWhileRevalidateFlow(
Logger.w { "$tag: network fetch timed out after ${networkTimeoutMs}ms" }
}

val fresh = loadFromCache()
val fresh =
safeCatching { loadFromCache() }
.onFailure { e -> Logger.w(e) { "$tag: cache reload failed after fetch" } }
.getOrElse { cached }
if (fresh != cached) {
emit(fresh)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,13 @@ class FirmwareReleaseRepositoryImplTest {

/** Serves `firmware_releases.json` from [bundled] via the real decode path, or nothing when null. */
private class FakeBundledAssetReader(var bundled: Releases? = null) : BundledAssetReader {
var failuresBeforeSuccess = 0

override fun open(name: String): Source? {
if (failuresBeforeSuccess > 0) {
failuresBeforeSuccess -= 1
error("Bundled asset read failed")
}
val releases = bundled ?: return null
if (name != "firmware_releases.json") return null
val bytes = Json.encodeToString(NetworkFirmwareReleases(releases = releases)).encodeToByteArray()
Expand Down Expand Up @@ -154,6 +160,43 @@ class FirmwareReleaseRepositoryImplTest {
assertEquals(listOf("v2.7.25.104df5f"), dao.getReleasesByType(FirmwareReleaseType.ALPHA).map { it.id })
}

@Test
fun failedBundledDecodeIsNotRetried() = runBlocking {
// A broken bundled asset is a permanent failure — retrying on every collection just burns I/O.
seed.failuresBeforeSuccess = 1
seed.bundled = Releases(stable = listOf(release("v2.7.15.567b8ea")))
api.response = NetworkFirmwareReleases()

val initialEmissions = repository.stableRelease.toList()
// Subsequent collection must NOT retry the decode (the second call would succeed if it did).
val emissions = repository.stableRelease.toList()

assertEquals(null, initialEmissions.first())
assertEquals(null, emissions.first(), "failed bundled decode is permanent; no retry on subsequent collections")
}

@Test
fun reSeedsWhenActiveDatabaseSwitches() = runBlocking {
// Reproduces the original bug: the active Room DB switches per selected device. A process-wide
// seed gate set during the first collection (against the default DB) skipped seeding the newly
// activated device DB, leaving the firmware picker empty.
seed.bundled = Releases(stable = listOf(release("v2.7.15.567b8ea")))
api.response = NetworkFirmwareReleases()

val firstEmissions = repository.stableRelease.toList()
assertEquals("v2.7.15.567b8ea", firstEmissions.first()?.id, "first collection seeds DB-A")

// The selected device's DB becomes active — it has no firmware_release rows.
dbProvider.switchToNewDatabase()

val secondEmissions = repository.stableRelease.toList()
assertEquals(
"v2.7.15.567b8ea",
secondEmissions.first()?.id,
"DB switch re-evaluates the bundled snapshot against the now-empty active DB and re-seeds it",
)
}

@Test
fun newerBundledSnapshotReplacesOlderCache() = runBlocking {
// App update ships a fresher bundle than what a network-starved user has cached.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,27 @@ import kotlinx.coroutines.flow.StateFlow
import org.meshtastic.core.database.DatabaseProvider
import org.meshtastic.core.database.MeshtasticDatabase
import org.meshtastic.core.database.getInMemoryDatabaseBuilder
import kotlin.concurrent.Volatile

/** A real [DatabaseProvider] that uses an in-memory database for testing. */
class FakeDatabaseProvider : DatabaseProvider {
private val db: MeshtasticDatabase = getInMemoryDatabaseBuilder().build()
@Volatile private var db: MeshtasticDatabase = getInMemoryDatabaseBuilder().build()
private val _currentDb = MutableStateFlow(db)
override val currentDb: StateFlow<MeshtasticDatabase> = _currentDb

override suspend fun <T> withDb(block: suspend (MeshtasticDatabase) -> T): T? = block(db)

/**
* Simulates the active-DB switch that happens when the app selects a different device — the new DB has no rows, so
* any cache that lived only in the previous DB is gone. Closes the prior DB to free its resources.
*/
fun switchToNewDatabase() {
val previous = db
db = getInMemoryDatabaseBuilder().build()
_currentDb.value = db
previous.close()
}

fun close() {
db.close()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,57 +172,56 @@ class FirmwareUpdateViewModel(
enterRecoveryModeOrError()
return@launch
}
getDeviceHardware(ourNode)?.let { deviceHardware ->
_deviceHardware.value = deviceHardware
_currentFirmwareVersion.value = ourNode.firmwareVersion

val releaseFlow =
if (_selectedReleaseType.value == FirmwareReleaseType.LOCAL) {
flowOf(null)
} else {
firmwareReleaseRepository.getReleaseFlow(_selectedReleaseType.value)
}

releaseFlow.collectLatest { release ->
_selectedRelease.value = release
val dismissed = bootloaderWarningDataSource.isDismissed(address)
val firmwareUpdateMethod =
when {
radioPrefs.isSerial() -> {
// Serial OTA is not yet supported for ESP32 — only nRF52/RP2040 UF2.
if (deviceHardware.isEsp32Arc) {
FirmwareUpdateMethod.Unknown
} else {
FirmwareUpdateMethod.Usb
}
val deviceHardware = getDeviceHardware(ourNode) ?: return@launch
_deviceHardware.value = deviceHardware
_currentFirmwareVersion.value = ourNode.firmwareVersion

val releaseFlow =
if (_selectedReleaseType.value == FirmwareReleaseType.LOCAL) {
flowOf(null)
} else {
firmwareReleaseRepository.getReleaseFlow(_selectedReleaseType.value)
}
releaseFlow.collectLatest { release ->

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description says release collection now starts before hardware resolution, but releaseFlow is a cold flow still collected here — after getDeviceHardware() returns at line 180, in the same coroutine. During a slow api.meshtastic.org hardware lookup the release state is still not populated any earlier. The real fix is the null-guard restructure below; the "collect before resolution" framing (and the construction reorder) is inert — worth correcting the PR body so it doesn't imply a latency win that isn't there.

_selectedRelease.value = release

val dismissed = bootloaderWarningDataSource.isDismissed(address)
val firmwareUpdateMethod =
when {
radioPrefs.isSerial() -> {
// Serial OTA is not yet supported for ESP32 — only nRF52/RP2040 UF2.
if (deviceHardware.isEsp32Arc) {
FirmwareUpdateMethod.Unknown
} else {
FirmwareUpdateMethod.Usb
}
}

radioPrefs.isBle() -> FirmwareUpdateMethod.Ble
radioPrefs.isBle() -> FirmwareUpdateMethod.Ble

radioPrefs.isTcp() -> {
// WiFi OTA is ESP32-only; nRF52/RP2040 have no TCP update path.
if (deviceHardware.isEsp32Arc) {
FirmwareUpdateMethod.Wifi
} else {
FirmwareUpdateMethod.Unknown
}
radioPrefs.isTcp() -> {
// WiFi OTA is ESP32-only; nRF52/RP2040 have no TCP update path.
if (deviceHardware.isEsp32Arc) {
FirmwareUpdateMethod.Wifi
} else {
FirmwareUpdateMethod.Unknown
}

else -> FirmwareUpdateMethod.Unknown
}
_state.value =
FirmwareUpdateState.Ready(
release = release,
deviceHardware = deviceHardware,
address = address,
showBootloaderWarning =
deviceHardware.requiresBootloaderUpgradeForOta == true &&
!dismissed &&
radioPrefs.isBle(),
updateMethod = firmwareUpdateMethod,
currentFirmwareVersion = ourNode.firmwareVersion,
)
}

else -> FirmwareUpdateMethod.Unknown
}
_state.value =
FirmwareUpdateState.Ready(
release = release,
deviceHardware = deviceHardware,
address = address,
showBootloaderWarning =
deviceHardware.requiresBootloaderUpgradeForOta == true &&
!dismissed &&
radioPrefs.isBle(),
updateMethod = firmwareUpdateMethod,
currentFirmwareVersion = ourNode.firmwareVersion,
)
}
}
.onFailure { e ->
Expand All @@ -244,12 +243,14 @@ class FirmwareUpdateViewModel(
private suspend fun enterRecoveryModeOrError() {
val recovery = firmwareRecoveryDataSource.pending.first()
if (recovery == null) {
clearDeviceMetadata()
_state.value = FirmwareUpdateState.Error(UiText.Resource(Res.string.firmware_update_no_device))
return
}
pendingRecovery = recovery
val hardware =
deviceHardwareRepository.getDeviceHardwareByModel(recovery.hwModel, recovery.pioEnv).getOrElse {
clearDeviceMetadata()
_state.value =
FirmwareUpdateState.Error(
UiText.Resource(Res.string.firmware_update_unknown_hardware, recovery.hwModel),
Expand Down Expand Up @@ -571,15 +572,23 @@ class FirmwareUpdateViewModel(

return if (hwModelInt != null) {
deviceHardwareRepository.getDeviceHardwareByModel(hwModelInt, target).getOrElse {
clearDeviceMetadata()
Comment thread
jamesarich marked this conversation as resolved.
_state.value =
FirmwareUpdateState.Error(UiText.Resource(Res.string.firmware_update_unknown_hardware, hwModelInt))
null
}
} else {
clearDeviceMetadata()
_state.value = FirmwareUpdateState.Error(UiText.Resource(Res.string.firmware_update_node_info_missing))
null
}
}

private fun clearDeviceMetadata() {
_selectedRelease.value = null
_deviceHardware.value = null
_currentFirmwareVersion.value = null
}
}

private suspend fun cleanupTemporaryFiles(
Expand Down
Loading