From 9468bc6ebe7d423387a4f61d81986199443b43b3 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Sat, 11 Apr 2026 19:50:52 -0500 Subject: [PATCH] refactor(service): unify dual connectionState flows into single source of truth (#5077) --- .../data/manager/MeshConnectionManagerImpl.kt | 10 ++++++ .../meshtastic/core/model/RadioController.kt | 11 +++++- .../core/repository/RadioInterfaceService.kt | 34 +++++++++++++++++-- .../core/repository/ServiceRepository.kt | 29 ++++++++++++++-- .../service/AndroidRadioControllerImpl.kt | 1 + .../core/service/DirectRadioControllerImpl.kt | 1 + .../core/service/ServiceRepositoryImpl.kt | 2 +- .../service/SharedRadioInterfaceService.kt | 9 +++++ .../core/testing/FakeRadioController.kt | 1 + .../core/testing/FakeRadioInterfaceService.kt | 10 +++++- .../core/testing/FakeServiceRepository.kt | 1 + .../core/ui/viewmodel/UIViewModel.kt | 2 +- 12 files changed, 103 insertions(+), 8 deletions(-) diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImpl.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImpl.kt index d64753bbf..918f25719 100644 --- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImpl.kt +++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImpl.kt @@ -98,6 +98,9 @@ class MeshConnectionManagerImpl( private var connectionRestored = false init { + // Bridge transport-level state into the canonical app-level state. + // This is the ONLY consumer of RadioInterfaceService.connectionState — it applies + // light-sleep policy and handshake awareness before writing to ServiceRepository. radioInterfaceService.connectionState.onEach(::onRadioConnectionState).launchIn(scope) // Ensure notification title and content stay in sync with state changes @@ -131,6 +134,13 @@ class MeshConnectionManagerImpl( .launchIn(scope) } + /** + * Bridges a transport-level [ConnectionState] into the canonical app-level state. + * + * Applies light-sleep policy (power-saving / router role) to decide whether a [ConnectionState.DeviceSleep] event + * should be surfaced as sleep or as a full disconnect, then delegates to [onConnectionChanged] for the actual state + * transition. + */ private suspend fun onRadioConnectionState(newState: ConnectionState) { val localConfig = radioConfigRepository.localConfigFlow.first() val isRouter = localConfig.device?.role == Config.DeviceConfig.Role.ROUTER diff --git a/core/model/src/commonMain/kotlin/org/meshtastic/core/model/RadioController.kt b/core/model/src/commonMain/kotlin/org/meshtastic/core/model/RadioController.kt index 54797eb75..84994e628 100644 --- a/core/model/src/commonMain/kotlin/org/meshtastic/core/model/RadioController.kt +++ b/core/model/src/commonMain/kotlin/org/meshtastic/core/model/RadioController.kt @@ -28,7 +28,16 @@ import org.meshtastic.proto.ClientNotification */ @Suppress("TooManyFunctions") interface RadioController { - /** Reactive connection state of the radio. */ + /** + * Canonical app-level connection state, delegated from [ServiceRepository][connectionState]. + * + * This exposes the same single source of truth as `ServiceRepository.connectionState`, surfaced through the + * controller interface for convenience in feature modules and ViewModels that depend on [RadioController] rather + * than [ServiceRepository] directly. + * + * This is **not** the transport-level state — it reflects the fully reconciled app-level state including handshake + * progress and device sleep policy. + */ val connectionState: StateFlow /** diff --git a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt index 2788a7f07..bb9cea52d 100644 --- a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt +++ b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt @@ -24,12 +24,42 @@ import org.meshtastic.core.model.DeviceType import org.meshtastic.core.model.InterfaceId import org.meshtastic.core.model.MeshActivity -/** Interface for the low-level radio interface that handles raw byte communication. */ +/** + * Interface for the low-level radio interface that handles raw byte communication. + * + * This is the **transport layer** — it manages the raw hardware connection (BLE, TCP, Serial, USB) to a Meshtastic + * radio. Its [connectionState] reflects whether the physical link is up or down, **before** any handshake or + * config-loading logic is applied. + * + * **Important:** UI and feature modules should **never** observe [connectionState] directly. Instead, they should use + * [ServiceRepository.connectionState], which is the canonical app-level connection state that accounts for handshake + * progress, light-sleep policy, and other higher-level concerns. The only legitimate consumer of this transport-level + * flow is [MeshConnectionManager], which bridges transport state changes into the app-level + * [ServiceRepository.connectionState]. + * + * @see ServiceRepository.connectionState + */ interface RadioInterfaceService { /** The device types supported by this platform's radio interface. */ val supportedDeviceTypes: List - /** Reactive connection state of the radio. */ + /** + * Transport-level connection state of the radio hardware. + * + * This flow reflects the raw state of the physical link (BLE, TCP, Serial, USB): + * - [ConnectionState.Connected] — the transport link is established + * - [ConnectionState.Disconnected] — the transport link is down (permanent) + * - [ConnectionState.DeviceSleep] — the transport link is down (transient, device sleeping) + * + * **This is NOT the canonical app-level connection state.** The transport may report [ConnectionState.Connected] + * while the app is still performing the mesh handshake (config + node-info exchange), during which the app-level + * state remains [ConnectionState.Connecting]. + * + * Only [MeshConnectionManager] should observe this flow. All other consumers (ViewModels, feature modules, UI) must + * use [ServiceRepository.connectionState]. + * + * @see ServiceRepository.connectionState + */ val connectionState: StateFlow /** Flow of the current device address. */ diff --git a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/ServiceRepository.kt b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/ServiceRepository.kt index 4a8af1143..57b1d71ec 100644 --- a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/ServiceRepository.kt +++ b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/ServiceRepository.kt @@ -31,14 +31,39 @@ import org.meshtastic.proto.MeshPacket * * This repository acts as the primary data bridge between the long-running mesh service and the UI/Feature layers. It * maintains reactive flows for connection status, error messages, and incoming mesh traffic. + * + * **Connection state contract:** [connectionState] is the **canonical, app-level** connection state that all UI, + * feature modules, and ViewModels should observe. It incorporates handshake progress, light-sleep policy, and transport + * reconciliation — unlike [RadioInterfaceService.connectionState], which only reflects the raw hardware link status. + * The [MeshConnectionManager] is the sole writer of this state; it bridges [RadioInterfaceService.connectionState] + * changes into app-level transitions via [setConnectionState]. + * + * @see RadioInterfaceService.connectionState */ @Suppress("TooManyFunctions") interface ServiceRepository { - /** Reactive flow of the current connection state. */ + /** + * Canonical app-level connection state. + * + * This is the **single source of truth** for connection status across the entire application. All UI components, + * feature modules, and ViewModels should observe this flow — never [RadioInterfaceService.connectionState]. + * + * State transitions are managed exclusively by [MeshConnectionManager], which reconciles transport-level events + * with handshake progress and device sleep policy: + * - [ConnectionState.Disconnected] — no active connection to a radio + * - [ConnectionState.Connecting] — transport is up, mesh handshake (config + node-info) in progress + * - [ConnectionState.Connected] — handshake complete, radio fully operational + * - [ConnectionState.DeviceSleep] — radio entered light-sleep (transient disconnect) + * + * @see RadioInterfaceService.connectionState + */ val connectionState: StateFlow /** - * Updates the current connection state. + * Updates the canonical app-level connection state. + * + * **This should only be called by [MeshConnectionManager].** Direct mutation from other components would bypass the + * transport-to-app reconciliation logic and create state inconsistencies. * * @param connectionState The new [ConnectionState]. */ diff --git a/core/service/src/androidMain/kotlin/org/meshtastic/core/service/AndroidRadioControllerImpl.kt b/core/service/src/androidMain/kotlin/org/meshtastic/core/service/AndroidRadioControllerImpl.kt index c7ef0ed10..a96b3ffc1 100644 --- a/core/service/src/androidMain/kotlin/org/meshtastic/core/service/AndroidRadioControllerImpl.kt +++ b/core/service/src/androidMain/kotlin/org/meshtastic/core/service/AndroidRadioControllerImpl.kt @@ -41,6 +41,7 @@ class AndroidRadioControllerImpl( private val nodeRepository: NodeRepository, ) : RadioController { + /** Delegates to [ServiceRepository.connectionState] — the canonical app-level source of truth. */ override val connectionState: StateFlow get() = serviceRepository.connectionState diff --git a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/DirectRadioControllerImpl.kt b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/DirectRadioControllerImpl.kt index fce0438dd..a753d2d08 100644 --- a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/DirectRadioControllerImpl.kt +++ b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/DirectRadioControllerImpl.kt @@ -63,6 +63,7 @@ class DirectRadioControllerImpl( private val myNodeNum: Int get() = nodeManager.myNodeNum.value ?: 0 + /** Delegates to [ServiceRepository.connectionState] — the canonical app-level source of truth. */ override val connectionState: StateFlow get() = serviceRepository.connectionState diff --git a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/ServiceRepositoryImpl.kt b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/ServiceRepositoryImpl.kt index ad5b92bd5..8671188ef 100644 --- a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/ServiceRepositoryImpl.kt +++ b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/ServiceRepositoryImpl.kt @@ -42,7 +42,7 @@ import org.meshtastic.proto.MeshPacket @Suppress("TooManyFunctions") open class ServiceRepositoryImpl : ServiceRepository { - // Connection state to our radio device + // Canonical app-level connection state — written exclusively by MeshConnectionManager. private val _connectionState: MutableStateFlow = MutableStateFlow(ConnectionState.Disconnected) override val connectionState: StateFlow get() = _connectionState diff --git a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt index 0785624f5..1865dd4c6 100644 --- a/core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt +++ b/core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt @@ -77,6 +77,15 @@ class SharedRadioInterfaceService( override val supportedDeviceTypes: List get() = transportFactory.supportedDeviceTypes + /** + * Transport-level connection state reflecting the raw hardware link status. + * + * Updated directly by [onConnect] and [onDisconnect] when the physical transport (BLE, TCP, Serial) connects or + * disconnects. This is consumed exclusively by + * [MeshConnectionManager][org.meshtastic.core.repository.MeshConnectionManager], which reconciles it into the + * canonical app-level + * [ServiceRepository.connectionState][org.meshtastic.core.repository.ServiceRepository.connectionState]. + */ private val _connectionState = MutableStateFlow(ConnectionState.Disconnected) override val connectionState: StateFlow = _connectionState.asStateFlow() diff --git a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioController.kt b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioController.kt index bf83be372..fac69e28c 100644 --- a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioController.kt +++ b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioController.kt @@ -30,6 +30,7 @@ class FakeRadioController : BaseFake(), RadioController { + /** Canonical app-level connection state, mirroring [ServiceRepository][connectionState] semantics. */ private val _connectionState = mutableStateFlow(ConnectionState.Connected) override val connectionState: StateFlow = _connectionState diff --git a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt index e1a26c6c3..3b8c83fe9 100644 --- a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt +++ b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt @@ -28,12 +28,20 @@ import org.meshtastic.core.model.InterfaceId import org.meshtastic.core.model.MeshActivity import org.meshtastic.core.repository.RadioInterfaceService -/** A test double for [RadioInterfaceService] that provides an in-memory implementation. */ +/** + * A test double for [RadioInterfaceService] that provides an in-memory implementation. + * + * The [connectionState] here mirrors the transport-level semantics of the real implementation. In production, only + * [MeshConnectionManager][org.meshtastic.core.repository.MeshConnectionManager] observes this flow; tests should verify + * that bridging behavior rather than consuming it directly from UI/feature test code (use + * [FakeServiceRepository.connectionState] instead). + */ @Suppress("TooManyFunctions") class FakeRadioInterfaceService(override val serviceScope: CoroutineScope = MainScope()) : RadioInterfaceService { override val supportedDeviceTypes: List = emptyList() + /** Transport-level connection state (raw hardware link status). */ private val _connectionState = MutableStateFlow(ConnectionState.Disconnected) override val connectionState: StateFlow = _connectionState diff --git a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeServiceRepository.kt b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeServiceRepository.kt index 266a0d958..ae06843b6 100644 --- a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeServiceRepository.kt +++ b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeServiceRepository.kt @@ -31,6 +31,7 @@ import org.meshtastic.proto.MeshPacket @Suppress("TooManyFunctions") class FakeServiceRepository : ServiceRepository { + /** Canonical app-level connection state — the single source of truth for UI/feature tests. */ private val _connectionState = MutableStateFlow(ConnectionState.Disconnected) override val connectionState: StateFlow = _connectionState diff --git a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/UIViewModel.kt b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/UIViewModel.kt index 95bf4365c..1e2021304 100644 --- a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/UIViewModel.kt +++ b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/UIViewModel.kt @@ -241,7 +241,7 @@ class UIViewModel( _sharedContactRequested.value = null } - // Connection state to our radio device + /** Canonical app-level connection state, sourced from [ServiceRepository.connectionState]. */ val connectionState get() = serviceRepository.connectionState