mirror of
https://github.com/meshtastic/Meshtastic-Android.git
synced 2026-06-26 06:25:24 -04:00
133 lines
9.4 KiB
Markdown
133 lines
9.4 KiB
Markdown
# Craft Workpad: Live BT/Wi-Fi adapter-state detection
|
||
|
||
> Generated by /craft · Started: 2026-06-18 · Branch: claude/gallant-thompson-d1b2ea (PR #5851)
|
||
|
||
---
|
||
|
||
## Task
|
||
|
||
Make the Bluetooth/Wi-Fi adapter-disabled detection **live** instead of `ON_RESUME`-polled — add a `BroadcastReceiver` for Bluetooth adapter state and a `ConnectivityManager.NetworkCallback` for network availability, so the Connections recovery banners update in real time (e.g. toggling BT from the quick-settings shade) rather than only when the activity resumes.
|
||
|
||
Scope: `isBluetoothDisabled()` and `isWifiUnavailable()` in `core/ui/src/androidMain/.../util/PlatformUtils.kt`. Follow-up to the adapter-state feature added in commit 2c06a8019 on PR #5851.
|
||
|
||
---
|
||
|
||
## Exploration Report
|
||
|
||
### Key Facts
|
||
|
||
- Both target functions live in `core/ui/src/androidMain/.../util/PlatformUtils.kt`: `isBluetoothDisabled()` (reads `BluetoothManager.adapter.isEnabled`) and `isWifiUnavailable()` (reads `ConnectivityManager.activeNetwork` transports). Both currently wrap their read in the private `rememberOnResumeState { ... }` helper — recomputed only on `Lifecycle.Event.ON_RESUME`.
|
||
- `rememberOnResumeState(check)` is the only consumer-shared refresh primitive; also used by `isGpsDisabled()`. Changing the two BT/Wi-Fi functions must NOT change `isGpsDisabled()` behavior (out of scope).
|
||
- **NetworkCallback pattern already exists** in `core/network/.../ConnectivityManager.kt`: `observeNetworks()` uses `callbackFlow { … registerNetworkCallback(req, cb); awaitClose { unregisterNetworkCallback(cb) } }` with `onAvailable`/`onLost`/`onCapabilitiesChanged`. Mirror it with a `DisposableEffect` in the composable (or `registerDefaultNetworkCallback` for the single active network).
|
||
- **BroadcastReceiver pattern already exists** in `core/ble/.../AndroidBluetoothRepository.kt`: registers via `ContextCompat.registerReceiver(context, receiver, filter, ContextCompat.RECEIVER_NOT_EXPORTED)` and `context.unregisterReceiver(...)`. For adapter state, the action is `BluetoothAdapter.ACTION_STATE_CHANGED`.
|
||
- The two functions are consumed only by `ConnectionsScreen.kt` (hoisted as `bluetoothDisabled`/`wifiUnavailable` booleans driving inline `RecoveryCard` banners + the BLE toggle routing). No other callers — the public contract (`@Composable expect fun … : Boolean`) is unchanged; only the androidMain implementation changes.
|
||
- `androidApp/src/main/AndroidManifest.xml` already declares Bluetooth + network permissions; `ACTION_STATE_CHANGED` and a `NetworkCallback` need no extra permission beyond what's declared (`ACCESS_NETWORK_STATE` is present for connectivity callbacks — verify).
|
||
|
||
### Key Files
|
||
|
||
- `core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt` — the two functions + `rememberOnResumeState`; the only file that changes.
|
||
- `core/network/.../repository/ConnectivityManager.kt` — reference NetworkCallback/callbackFlow pattern.
|
||
- `core/ble/.../AndroidBluetoothRepository.kt` — reference registerReceiver/RECEIVER_NOT_EXPORTED pattern.
|
||
- `feature/connections/.../ui/ConnectionsScreen.kt` — sole consumer (no change needed; reads the same booleans, now updated live).
|
||
- jvm/ios actuals already return constant `false` — unaffected.
|
||
|
||
---
|
||
|
||
## Requirements
|
||
|
||
R1: `isBluetoothDisabled()` updates reactively — when the Bluetooth adapter is turned on/off (incl. from the quick-settings shade while the app is foregrounded), the returned value changes without waiting for `ON_RESUME`.
|
||
Source: human confirmed
|
||
Verification: review confirms a `BroadcastReceiver` on `BluetoothAdapter.ACTION_STATE_CHANGED` drives the state; manual toggle from shade flips the banner.
|
||
|
||
R2: `isWifiUnavailable()` updates reactively — connecting/disconnecting Wi-Fi (or losing the active local network) changes the returned value live.
|
||
Source: human confirmed
|
||
Verification: review confirms a `ConnectivityManager` `NetworkCallback` (default-network) drives the state.
|
||
|
||
R3: `ON_RESUME` polling is removed for these two functions; the receiver/callback is registered and unregistered with the composable's lifetime via `DisposableEffect` (no leaks). `isGpsDisabled()` continues to use `rememberOnResumeState` unchanged.
|
||
Source: human confirmed
|
||
Verification: `grep` shows no `rememberOnResumeState` in the two functions; `awaitClose`/`onDispose` unregisters; `isGpsDisabled` untouched.
|
||
|
||
R4: Registration follows existing repo conventions — `ContextCompat.registerReceiver(..., RECEIVER_NOT_EXPORTED)` for the BT receiver; `registerDefaultNetworkCallback`/`unregisterNetworkCallback` for the network callback. No new permissions (ACCESS_NETWORK_STATE already declared).
|
||
Source: craft-clarify recommendation
|
||
Verification: review confirms the registration calls + flag matches `RECEIVER_NOT_EXPORTED`.
|
||
|
||
R5: The public `expect` contract and all consumers are unchanged — `ConnectionsScreen` reads the same `Boolean`s, now live. jvm/ios actuals stay constant `false`.
|
||
Source: craft-clarify recommendation
|
||
Verification: no change to commonMain expect or jvm/iosMain; ConnectionsScreen diff empty; both flavors assemble.
|
||
|
||
---
|
||
|
||
## Architectural Decision
|
||
|
||
- **Chosen approach:** Extract a private `rememberObservedFlag(read, subscribe)` primitive (DisposableEffect + mutableStateOf, re-seed on registration); express `isBluetoothDisabled()` via a `BroadcastReceiver` on `ACTION_STATE_CHANGED` (RECEIVER_NOT_EXPORTED, main-thread delivery) and `isWifiUnavailable()` via `registerDefaultNetworkCallback(callback, mainHandler)`.
|
||
- **Approved:** 2026-06-18
|
||
- **Adversarial:** P1-ish threading risk (NetworkCallback on background thread) mitigated by main-thread Handler; leaks prevented by onDispose unregister; cold-start staleness prevented by read() re-seed. No blocker.
|
||
- **Files:** `core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt` only.
|
||
- **Test:** build both flavors + detekt/spotless; manual shade-toggle verification (no headless test feasible — `core/ui` has no instrumentation).
|
||
|
||
---
|
||
|
||
## Acceptance Criteria
|
||
|
||
AC1: [R1] `isBluetoothDisabled()` is driven by a `BroadcastReceiver` on `BluetoothAdapter.ACTION_STATE_CHANGED`. Auto-verify: grep ACTION_STATE_CHANGED + BroadcastReceiver in the function.
|
||
AC2: [R2] `isWifiUnavailable()` is driven by `registerDefaultNetworkCallback`. Auto-verify: grep registerDefaultNetworkCallback.
|
||
AC3: [R3] neither function uses `rememberOnResumeState`; observer torn down in `onDispose`; `isGpsDisabled()` still uses `rememberOnResumeState`. Auto-verify: grep.
|
||
AC4: [R4] BT receiver uses `RECEIVER_NOT_EXPORTED`; network callback uses a main-thread `Handler`. Auto-verify: grep.
|
||
AC5: [R5] commonMain expect + jvm/ios actuals + ConnectionsScreen unchanged; both flavors assemble; detekt/spotless clean. Auto-verify: git diff scope + build.
|
||
|
||
---
|
||
|
||
## Implementation Plan
|
||
|
||
**Branch:** claude/gallant-thompson-d1b2ea **Base commit:** 2c06a8019
|
||
|
||
### Task list
|
||
|
||
### Pre-existing failures (do not fix — out of scope)
|
||
|
||
---
|
||
|
||
## Completion Bar
|
||
|
||
1. [x] All planned files created/modified
|
||
2. [x] Linter clean
|
||
3. [x] Tests pass (no new tests feasible; regressions green)
|
||
4. [x] Every AC has a completion note
|
||
5. [x] No open markers remain
|
||
6. [x] Scope discipline honored
|
||
|
||
---
|
||
|
||
## Review
|
||
|
||
Reviewer: concurrency (right-sized — single-file ~40-line change on established repo patterns; build+detekt covered the rest).
|
||
|
||
Verdict: **SOUND.** Threading main-thread-confined (BT receiver no-Handler → main; NetworkCallback with main-Looper Handler); register/unregister balanced 1:1 via `DisposableEffect(Unit)`/`onDispose`; no leak or double-unregister; `read()` re-seed correct.
|
||
|
||
Findings (both P3, no action):
|
||
- F-1 [P3] `rememberUpdatedState(subscribe)` freshness is never exercised (subscribe runs once). NOT removed: accessing `subscribe` directly inside `DisposableEffect` re-triggers the `LambdaParameterInRestartableEffect` detekt rule, so the wrapper is required for lint. `LocalContext` is stable, so no stale-context defect. Kept as-is.
|
||
- F-2 [P3] `registerDefaultNetworkCallback` `TooManyRequests` — structurally bounded (1:1 with a single live banner); no retry loop. No guard needed.
|
||
|
||
### Acceptance criteria status
|
||
- AC1 ✓ BroadcastReceiver on ACTION_STATE_CHANGED drives isBluetoothDisabled.
|
||
- AC2 ✓ registerDefaultNetworkCallback drives isWifiUnavailable.
|
||
- AC3 ✓ neither uses rememberOnResumeState; onDispose unregisters; isGpsDisabled untouched.
|
||
- AC4 ✓ RECEIVER_NOT_EXPORTED + main-thread Handler.
|
||
- AC5 ✓ expect/jvm/ios/ConnectionsScreen unchanged; both flavors assemble; detekt/spotless clean.
|
||
|
||
---
|
||
|
||
## Deferred Items
|
||
|
||
---
|
||
|
||
## Phase Log
|
||
|
||
- explore: done — 2026-06-18 — lean direct explore (deep prior context). Both reactive patterns already in repo (NetworkCallback callbackFlow in core/network; registerReceiver RECEIVER_NOT_EXPORTED in core/ble). Only androidMain PlatformUtils changes.
|
||
- clarify: done — 2026-06-18 — Q1 confirmed (replace ON_RESUME entirely). R1–R5 recorded.
|
||
- architect: done — 2026-06-18 — single approach (rememberObservedFlag primitive) + self-adversarial pass (threading via main Handler). Approved.
|
||
- implement: done — 2026-06-18 — one file (core/ui androidMain PlatformUtils). All 5 ACs met; build+detekt+spotless+both flavors green.
|
||
- review: done — 2026-06-18 — concurrency reviewer: SOUND. 2×P3 non-actionable (lint-required wrapper; bounded TooManyRequests).
|
||
- refine: done — 2026-06-18 — fast path, no actionable findings. P3s recorded as considered/declined.
|
||
- pr: done — 2026-06-18 — pushed to existing PR #5851 (174db32ae); no new PR (same branch/feature).
|