fix(android): output picker CI - passive discovery + LongMethod
android / Build + lint + test (push) Successful in 6m26s
android / Build + lint + test (push) Successful in 6m26s
Two failures on the slice's final dev tip: 1. OutputPickerController referenced MediaRouter.CALLBACK_FLAG_PASSIVE_DISCOVERY which doesn't exist in androidx.mediarouter 1.7.0 - the spec hallucinated it. Passive discovery is the default behavior when addCallback is called with no flag argument. Use the 2-arg overload for the init block and downgradeDiscovery; keep CALLBACK_FLAG_REQUEST_DISCOVERY for upgradeDiscovery. 2. NowPlayingBody grew to 82 lines after the Task 5 output-picker wiring (state collection + permission launcher + LaunchedEffect + conditional Sheet). Extracted the BLUETOOTH_CONNECT permission plumbing into rememberBluetoothPermissionState, the Column layout into NowPlayingContent, and the scrubber+transport pair (which share the smoothed playback position) into PlaybackControlsBlock. NowPlayingBody is back to ~34 lines and the new helpers each sit well under detekt's 60-line LongMethod cap.
This commit is contained in:
+16
-10
@@ -24,11 +24,11 @@ data class RouteSnapshot(
|
||||
|
||||
/**
|
||||
* Hilt-singleton facade over [MediaRouter]. Owns the callback
|
||||
* lifecycle — passive discovery at process start (route list updates
|
||||
* without forcing Bluetooth scans), upgrades to active discovery
|
||||
* while the picker sheet is open so newly-paired devices appear
|
||||
* promptly. The [routesState] StateFlow projects the current route
|
||||
* + sorted available list as a [RouteSnapshot]; the picker
|
||||
* lifecycle — default passive behavior at process start (route list
|
||||
* updates without forcing Bluetooth scans), upgrades to active
|
||||
* discovery while the picker sheet is open so newly-paired devices
|
||||
* appear promptly. The [routesState] StateFlow projects the current
|
||||
* route + sorted available list as a [RouteSnapshot]; the picker
|
||||
* ViewModel collects it.
|
||||
*
|
||||
* Mirrors the OutputPickerController role described in
|
||||
@@ -71,7 +71,12 @@ class OutputPickerController @Inject constructor(
|
||||
}
|
||||
|
||||
init {
|
||||
mediaRouter.addCallback(selector, callback, MediaRouter.CALLBACK_FLAG_PASSIVE_DISCOVERY)
|
||||
// Two-arg addCallback registers with no discovery flag —
|
||||
// androidx.mediarouter 1.7.0's default passive behavior:
|
||||
// route-list updates flow through onRouteAdded/Removed/Changed
|
||||
// without forcing Bluetooth scans. (There is no
|
||||
// CALLBACK_FLAG_PASSIVE_DISCOVERY constant; absent flag = passive.)
|
||||
mediaRouter.addCallback(selector, callback)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -85,12 +90,13 @@ class OutputPickerController @Inject constructor(
|
||||
}
|
||||
|
||||
/**
|
||||
* Downgrade callback registration back to passive — call when the
|
||||
* picker sheet closes so we don't keep Bluetooth scanning on for
|
||||
* battery cost. Same idempotent re-registration semantics.
|
||||
* Downgrade callback registration back to default passive behavior
|
||||
* — call when the picker sheet closes so we don't keep Bluetooth
|
||||
* scanning on for battery cost. Two-arg overload = no flag =
|
||||
* passive. Same idempotent re-registration semantics.
|
||||
*/
|
||||
fun downgradeDiscovery() {
|
||||
mediaRouter.addCallback(selector, callback, MediaRouter.CALLBACK_FLAG_PASSIVE_DISCOVERY)
|
||||
mediaRouter.addCallback(selector, callback)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -284,29 +284,41 @@ private fun NowPlayingBody(
|
||||
val outputViewModel: OutputPickerViewModel = hiltViewModel()
|
||||
val routes by outputViewModel.routes.collectAsStateWithLifecycle()
|
||||
val sheetVisible by outputViewModel.sheetVisible.collectAsStateWithLifecycle()
|
||||
val permissionDenied = rememberBluetoothPermissionState(sheetVisible)
|
||||
NowPlayingContent(
|
||||
inner = inner,
|
||||
state = state,
|
||||
track = track,
|
||||
navController = navController,
|
||||
viewModel = viewModel,
|
||||
trackActionsViewModel = trackActionsViewModel,
|
||||
routes = routes,
|
||||
isLiked = isLiked,
|
||||
onChipTapped = outputViewModel::onChipTapped,
|
||||
)
|
||||
if (sheetVisible) {
|
||||
OutputPickerSheet(
|
||||
snapshot = routes,
|
||||
permissionDenied = permissionDenied,
|
||||
onRouteSelected = outputViewModel::onRouteSelected,
|
||||
onDismiss = outputViewModel::onSheetDismissed,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// One-shot BLUETOOTH_CONNECT permission flow. The launcher is
|
||||
// remembered across recompositions; the LaunchedEffect fires
|
||||
// exactly once when the sheet opens for the first time per
|
||||
// composition. Subsequent opens are no-ops; the system remembers
|
||||
// the user's choice.
|
||||
val context = LocalContext.current
|
||||
var permissionDenied by remember { mutableStateOf(false) }
|
||||
val permissionLauncher = rememberLauncherForActivityResult(
|
||||
contract = ActivityResultContracts.RequestPermission(),
|
||||
) { granted ->
|
||||
permissionDenied = !granted
|
||||
}
|
||||
LaunchedEffect(sheetVisible) {
|
||||
if (sheetVisible &&
|
||||
ContextCompat.checkSelfPermission(
|
||||
context,
|
||||
Manifest.permission.BLUETOOTH_CONNECT,
|
||||
) != PackageManager.PERMISSION_GRANTED
|
||||
) {
|
||||
permissionLauncher.launch(Manifest.permission.BLUETOOTH_CONNECT)
|
||||
}
|
||||
}
|
||||
@Composable
|
||||
@Suppress("LongParameterList") // mirrors NowPlayingBody — pure layout wiring
|
||||
private fun NowPlayingContent(
|
||||
inner: androidx.compose.foundation.layout.PaddingValues,
|
||||
state: com.fabledsword.minstrel.player.PlayerUiState,
|
||||
track: com.fabledsword.minstrel.models.TrackRef,
|
||||
navController: NavHostController,
|
||||
viewModel: PlayerViewModel,
|
||||
trackActionsViewModel: TrackActionsViewModel,
|
||||
routes: RouteSnapshot,
|
||||
isLiked: Boolean,
|
||||
onChipTapped: () -> Unit,
|
||||
) {
|
||||
Column(
|
||||
modifier = Modifier
|
||||
.fillMaxSize()
|
||||
@@ -341,37 +353,78 @@ private fun NowPlayingBody(
|
||||
Spacer(Modifier.height(8.dp))
|
||||
DeviceChip(
|
||||
route = routes.current,
|
||||
onClick = outputViewModel::onChipTapped,
|
||||
onClick = onChipTapped,
|
||||
modifier = Modifier.align(Alignment.CenterHorizontally),
|
||||
)
|
||||
}
|
||||
Spacer(Modifier.height(4.dp))
|
||||
val smoothPositionMs by rememberSmoothPositionMs(
|
||||
positionMs = state.positionMs,
|
||||
durationMs = state.durationMs,
|
||||
isPlaying = state.isPlaying,
|
||||
)
|
||||
ScrubberRow(
|
||||
positionMs = smoothPositionMs,
|
||||
durationMs = state.durationMs,
|
||||
onSeek = viewModel::seekTo,
|
||||
)
|
||||
Spacer(Modifier.height(4.dp))
|
||||
TransportRow(
|
||||
isPlaying = state.isPlaying,
|
||||
onPrev = viewModel::skipToPrevious,
|
||||
onPlayPause = { if (state.isPlaying) viewModel.pause() else viewModel.play() },
|
||||
onNext = viewModel::skipToNext,
|
||||
)
|
||||
PlaybackControlsBlock(state = state, viewModel = viewModel)
|
||||
}
|
||||
if (sheetVisible) {
|
||||
OutputPickerSheet(
|
||||
snapshot = routes,
|
||||
permissionDenied = permissionDenied,
|
||||
onRouteSelected = outputViewModel::onRouteSelected,
|
||||
onDismiss = outputViewModel::onSheetDismissed,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Scrubber + transport pair, sharing the smoothed playback position.
|
||||
* Extracted from [NowPlayingContent] to keep that body under detekt's
|
||||
* LongMethod ceiling — the two rows belong together (the scrubber's
|
||||
* smoothed position would otherwise need to be hoisted into the
|
||||
* caller just to thread it into the row below).
|
||||
*/
|
||||
@Composable
|
||||
private fun PlaybackControlsBlock(
|
||||
state: com.fabledsword.minstrel.player.PlayerUiState,
|
||||
viewModel: PlayerViewModel,
|
||||
) {
|
||||
val smoothPositionMs by rememberSmoothPositionMs(
|
||||
positionMs = state.positionMs,
|
||||
durationMs = state.durationMs,
|
||||
isPlaying = state.isPlaying,
|
||||
)
|
||||
ScrubberRow(
|
||||
positionMs = smoothPositionMs,
|
||||
durationMs = state.durationMs,
|
||||
onSeek = viewModel::seekTo,
|
||||
)
|
||||
Spacer(Modifier.height(4.dp))
|
||||
TransportRow(
|
||||
isPlaying = state.isPlaying,
|
||||
onPrev = viewModel::skipToPrevious,
|
||||
onPlayPause = { if (state.isPlaying) viewModel.pause() else viewModel.play() },
|
||||
onNext = viewModel::skipToNext,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* One-shot BLUETOOTH_CONNECT permission flow tied to picker-sheet
|
||||
* visibility. The launcher is remembered across recompositions; the
|
||||
* LaunchedEffect fires when the sheet opens and the permission is not
|
||||
* already granted. Subsequent opens are no-ops once the user has
|
||||
* answered — the system remembers their choice. Returns whether the
|
||||
* user explicitly denied, so the sheet can surface the rationale row.
|
||||
*
|
||||
* Extracted from [NowPlayingBody] to keep that body under detekt's
|
||||
* LongMethod ceiling — the permission plumbing is incidental to the
|
||||
* player layout.
|
||||
*/
|
||||
@Composable
|
||||
private fun rememberBluetoothPermissionState(sheetVisible: Boolean): Boolean {
|
||||
val context = LocalContext.current
|
||||
var permissionDenied by remember { mutableStateOf(false) }
|
||||
val permissionLauncher = rememberLauncherForActivityResult(
|
||||
contract = ActivityResultContracts.RequestPermission(),
|
||||
) { granted ->
|
||||
permissionDenied = !granted
|
||||
}
|
||||
LaunchedEffect(sheetVisible) {
|
||||
if (sheetVisible &&
|
||||
ContextCompat.checkSelfPermission(
|
||||
context,
|
||||
Manifest.permission.BLUETOOTH_CONNECT,
|
||||
) != PackageManager.PERMISSION_GRANTED
|
||||
) {
|
||||
permissionLauncher.launch(Manifest.permission.BLUETOOTH_CONNECT)
|
||||
}
|
||||
}
|
||||
return permissionDenied
|
||||
}
|
||||
|
||||
private fun shouldShowChip(snapshot: RouteSnapshot): Boolean {
|
||||
|
||||
Reference in New Issue
Block a user