feat(diagnostics): 'playback' kind, newest-first sort, fix active-route subtitle
Relabel (#1204): route + player_state events fire for every output route, not just UPnP — split them into a new 'playback' kind; 'upnp_sync' now means genuinely UPnP/Sonos signal (drops, resync). Migration 0037 adds 'playback' to the kind CHECK; server whitelist, Android reporter labels, and the web kind filter updated. Web sort: the diagnostics list gains a Newest/Oldest-first sort (default newest at top); export follows the displayed order. Fix (#1205): OutputRoute.isConnected was derived from RouteInfo.connectionState, which stays DISCONNECTED for local SYSTEM routes even when active — so a connected Bluetooth device showed "Available" and reported connected:false. The picker subtitle now uses isSelected (route == selected route); the dead isConnected field is removed and the misleading `connected` field dropped from the diagnostics route event (it only ever logs the active route). Refs Scribe M9 (#119), tasks #1204 #1205. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K55iTxn95BtshocgdE1shW
This commit is contained in:
+6
-3
@@ -147,11 +147,13 @@ class DiagnosticsReporter @Inject constructor(
|
||||
}
|
||||
|
||||
private suspend fun collectPlayerState() {
|
||||
// 'playback', not 'upnp_sync' — player state applies to every output
|
||||
// route (phone speaker, Bluetooth, UPnP), not just casting.
|
||||
playerController.uiState
|
||||
.map { Triple(it.currentSource, it.isUpnpLoading, it.playbackError) }
|
||||
.distinctUntilChanged()
|
||||
.collect { (source, upnpLoading, err) ->
|
||||
record("upnp_sync", buildJsonObject {
|
||||
record("playback", buildJsonObject {
|
||||
put("event", "player_state")
|
||||
put("source", source ?: "")
|
||||
put("upnp_loading", upnpLoading)
|
||||
@@ -161,14 +163,15 @@ class DiagnosticsReporter @Inject constructor(
|
||||
}
|
||||
|
||||
private suspend fun collectRoutes() {
|
||||
// 'playback' — route changes happen for all outputs. This only ever
|
||||
// logs the ACTIVE route (routesState.current), so no "connected" flag.
|
||||
outputPicker.routesState.map { it.current }.distinctUntilChanged().collect { r ->
|
||||
record("upnp_sync", buildJsonObject {
|
||||
record("playback", buildJsonObject {
|
||||
put("event", "route")
|
||||
put("id", r.id)
|
||||
put("name", r.name)
|
||||
put("kind", r.kind.name)
|
||||
put("protocol", r.protocol.name)
|
||||
put("connected", r.isConnected)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+6
-3
@@ -133,7 +133,7 @@ private fun RouteRow(
|
||||
maxLines = 2,
|
||||
overflow = TextOverflow.Ellipsis,
|
||||
)
|
||||
val subtitle = route.description ?: defaultSubtitle(route)
|
||||
val subtitle = route.description ?: defaultSubtitle(route, isSelected)
|
||||
Text(
|
||||
text = subtitle,
|
||||
style = MaterialTheme.typography.bodySmall,
|
||||
@@ -198,10 +198,13 @@ private fun MulticastHintRow() {
|
||||
}
|
||||
}
|
||||
|
||||
private fun defaultSubtitle(route: OutputRoute): String = when (route.kind) {
|
||||
// isSelected (route == the active/selected route) drives the Bluetooth
|
||||
// "Connected" subtitle. RouteInfo.connectionState can't — it stays
|
||||
// DISCONNECTED for local SYSTEM routes even when they're in use.
|
||||
private fun defaultSubtitle(route: OutputRoute, isSelected: Boolean): String = when (route.kind) {
|
||||
OutputRoute.Kind.BuiltIn -> "Phone speaker"
|
||||
OutputRoute.Kind.Wired -> "Wired"
|
||||
OutputRoute.Kind.Bluetooth -> if (route.isConnected) "Connected" else "Available"
|
||||
OutputRoute.Kind.Bluetooth -> if (isSelected) "Connected" else "Available"
|
||||
OutputRoute.Kind.Cast -> "Cast"
|
||||
OutputRoute.Kind.Other -> "Available"
|
||||
}
|
||||
|
||||
@@ -21,7 +21,6 @@ data class OutputRoute(
|
||||
val description: String?,
|
||||
val kind: Kind,
|
||||
val protocol: Protocol,
|
||||
val isConnected: Boolean,
|
||||
) {
|
||||
enum class Kind { BuiltIn, Wired, Bluetooth, Cast, Other }
|
||||
|
||||
@@ -43,9 +42,11 @@ data class OutputRoute(
|
||||
/**
|
||||
* Lift a MediaRouter [route] into the domain model. Kind is
|
||||
* inferred from [MediaRouter.RouteInfo.getDeviceType]; unknown
|
||||
* device types fall through to [Kind.Other]. The
|
||||
* `connectionState` proxy is good enough for the chip's
|
||||
* "Connected"/"Available" subtitle.
|
||||
* device types fall through to [Kind.Other]. "Active" is NOT an
|
||||
* intrinsic of the route — the caller derives it by comparing to
|
||||
* the selected route (RouteSnapshot.current), because
|
||||
* RouteInfo.connectionState only reflects remote-route handshakes
|
||||
* and stays DISCONNECTED for local SYSTEM routes even when in use.
|
||||
*/
|
||||
fun fromRouteInfo(route: MediaRouter.RouteInfo): OutputRoute {
|
||||
val kind = when (route.deviceType) {
|
||||
@@ -58,15 +59,12 @@ data class OutputRoute(
|
||||
MediaRouter.RouteInfo.DEVICE_TYPE_SPEAKER -> Kind.Other
|
||||
else -> Kind.Other
|
||||
}
|
||||
val connected =
|
||||
route.connectionState == MediaRouter.RouteInfo.CONNECTION_STATE_CONNECTED
|
||||
return OutputRoute(
|
||||
id = route.id,
|
||||
name = route.name,
|
||||
description = route.description,
|
||||
kind = kind,
|
||||
protocol = Protocol.SYSTEM,
|
||||
isConnected = connected,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -76,10 +74,10 @@ data class OutputRoute(
|
||||
* network speakers into the same `OutputPickerController`
|
||||
* routes stream the system routes come through.
|
||||
*
|
||||
* `isConnected = false` because UPnP devices have no
|
||||
* MediaRouter connection-state concept — they're always
|
||||
* "available" on the LAN, and the picker's selected-route
|
||||
* rendering handles the "currently playing" indicator.
|
||||
* UPnP devices have no MediaRouter connection-state concept —
|
||||
* they're always "available" on the LAN, and the picker's
|
||||
* selected-route rendering handles the "currently playing"
|
||||
* indicator.
|
||||
*
|
||||
* Subtitle is `manufacturer modelName` joined by a single
|
||||
* space, falling back to "Network speaker" when both fields
|
||||
@@ -96,7 +94,6 @@ data class OutputRoute(
|
||||
description = description,
|
||||
kind = Kind.Other,
|
||||
protocol = Protocol.UPNP,
|
||||
isConnected = false,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -21,7 +21,8 @@ import (
|
||||
// variants do NOT require a new category here.
|
||||
var validDiagnosticKinds = map[string]struct{}{
|
||||
"connectivity": {},
|
||||
"upnp_sync": {},
|
||||
"upnp_sync": {}, // genuinely UPnP/Sonos-specific (drops, resync)
|
||||
"playback": {}, // route + player-state, any output (phone/BT/UPnP)
|
||||
"power": {},
|
||||
"lifecycle": {},
|
||||
"heartbeat": {},
|
||||
|
||||
@@ -0,0 +1,9 @@
|
||||
-- Drop rows using the new value first so they don't violate the restored
|
||||
-- (narrower) constraint, then revert the whitelist.
|
||||
DELETE FROM diagnostic_events WHERE kind = 'playback';
|
||||
ALTER TABLE diagnostic_events DROP CONSTRAINT diagnostic_events_kind_check;
|
||||
ALTER TABLE diagnostic_events ADD CONSTRAINT diagnostic_events_kind_check
|
||||
CHECK (kind IN (
|
||||
'connectivity', 'upnp_sync', 'power', 'lifecycle',
|
||||
'heartbeat', 'http'
|
||||
));
|
||||
@@ -0,0 +1,19 @@
|
||||
-- Add 'playback' to the diagnostic_events kind whitelist (M9 follow-up).
|
||||
--
|
||||
-- `route` and `player_state` events fire for EVERY output route (phone
|
||||
-- speaker, Bluetooth, UPnP/Sonos), so bucketing them under 'upnp_sync'
|
||||
-- was misleading — an operator on Bluetooth earbuds saw "upnp_sync"
|
||||
-- rows. 'upnp_sync' now means genuinely UPnP/Sonos-specific signal
|
||||
-- (drops, and future resync/desync); general playback + route telemetry
|
||||
-- moves to the new 'playback' kind.
|
||||
--
|
||||
-- Per the enum-CHECK-whitelist rule, the new value lands by dropping and
|
||||
-- re-adding the constraint in the same change. Existing rows tagged
|
||||
-- 'upnp_sync' stay as-is (debug telemetry on a 30-day retention).
|
||||
|
||||
ALTER TABLE diagnostic_events DROP CONSTRAINT diagnostic_events_kind_check;
|
||||
ALTER TABLE diagnostic_events ADD CONSTRAINT diagnostic_events_kind_check
|
||||
CHECK (kind IN (
|
||||
'connectivity', 'upnp_sync', 'power', 'lifecycle',
|
||||
'heartbeat', 'http', 'playback'
|
||||
));
|
||||
@@ -22,11 +22,20 @@
|
||||
|
||||
const client = useQueryClient();
|
||||
|
||||
const KINDS = ['connectivity', 'upnp_sync', 'power', 'lifecycle', 'heartbeat', 'http'] as const;
|
||||
const KINDS = [
|
||||
'connectivity',
|
||||
'playback',
|
||||
'upnp_sync',
|
||||
'power',
|
||||
'lifecycle',
|
||||
'heartbeat',
|
||||
'http'
|
||||
] as const;
|
||||
|
||||
function kindLabel(k: string): string {
|
||||
switch (k) {
|
||||
case 'connectivity': return 'Connectivity';
|
||||
case 'playback': return 'Playback';
|
||||
case 'upnp_sync': return 'UPnP sync';
|
||||
case 'power': return 'Power';
|
||||
case 'lifecycle': return 'Lifecycle';
|
||||
@@ -82,10 +91,17 @@
|
||||
const devicesQuery = $derived($devicesStore);
|
||||
const devices = $derived(devicesQuery.data ?? []);
|
||||
|
||||
// Display sort. The API returns newest-first; default the view to that
|
||||
// (most recent at the top), with an Oldest-first option for reading a
|
||||
// timeline top-to-bottom. Export follows whatever's displayed.
|
||||
let sortOrder = $state<'newest' | 'oldest'>('newest');
|
||||
|
||||
const diagStore = $derived(createAdminDiagnosticsQuery(filter));
|
||||
const diagQuery = $derived($diagStore);
|
||||
// API returns newest-first; reverse to chronological for a readable timeline.
|
||||
const rows = $derived([...((diagQuery.data ?? []) as AdminDiagnostic[])].reverse());
|
||||
const rows = $derived.by(() => {
|
||||
const data = [...((diagQuery.data ?? []) as AdminDiagnostic[])]; // newest-first
|
||||
return sortOrder === 'oldest' ? data.reverse() : data;
|
||||
});
|
||||
|
||||
function fmtTime(iso: string): string {
|
||||
const d = new Date(iso);
|
||||
@@ -232,6 +248,16 @@
|
||||
{/each}
|
||||
</select>
|
||||
</label>
|
||||
<label class="block text-xs text-text-secondary">
|
||||
Sort
|
||||
<select
|
||||
bind:value={sortOrder}
|
||||
class="mt-1 block w-36 rounded border border-border bg-surface px-2 py-1.5 text-sm text-text-primary"
|
||||
>
|
||||
<option value="newest">Newest first</option>
|
||||
<option value="oldest">Oldest first</option>
|
||||
</select>
|
||||
</label>
|
||||
|
||||
<div class="ml-auto flex gap-2">
|
||||
<button
|
||||
@@ -311,9 +337,9 @@
|
||||
<div class="flex items-center justify-between">
|
||||
<span class="text-xs text-text-muted">
|
||||
{#if hasWindow}
|
||||
{rows.length} events in window (oldest first)
|
||||
{rows.length} events in window ({sortOrder === 'newest' ? 'newest' : 'oldest'} first)
|
||||
{:else}
|
||||
Most recent {rows.length} events (oldest first) · set a time window under Advanced
|
||||
Most recent {rows.length} events ({sortOrder === 'newest' ? 'newest' : 'oldest'} first) · set a time window under Advanced
|
||||
{/if}
|
||||
</span>
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user