feat: stream on every screen, heal phantom satellite pads, accurate latency readout - #138
Conversation
Framework-path controllers (Bluetooth, USB Standard) only streamed while Main, Connections, ConfigureBindings, or an input overlay was focused; on Settings, Diagnostics, the controller inspector, Help, Donate, Licenses, and the setup flow their events fell through to UI navigation instead of the wire, and keep-screen-on plus the low-power dim timer disarmed. BaseGamepadHostActivity now owns the GamepadActivityHost plumbing (dispatch forwarding, focus release, dim cancel on stop) and every screen except MainActivity (GameActivity superclass, wires the host itself) and NativeUnavailableActivity extends it. All activity layouts include the low-power overlay and countdown chip, including the sw600dp variants. ConnectionsActivity and ConfigureBindingsActivity drop their duplicated copies of the same plumbing. StreamingScreenCoverageTest guards the invariant: every manifest activity must host gamepad streaming and every activity layout must carry the overlay includes.
A device that departed while the binding observer was stopped (app backgrounded during a transport switch) was in neither `present` nor `lastBound`, so reconcileSlots never unbound it. Its slot stayed in the binding store and in SatelliteConnection's declarative slot map, which re-registered a phantom pad on the satellite on every reconnect, with no UI able to remove it. reconcileSlots now also treats any numeric binding whose device id the registry no longer knows as departed, emitting the same ordered Unbind/Forget/ReleaseHubBinding ops. Non-numeric logical slots (the on-screen controller) are exempt, matching the slotExists rule. The registry's 5s disconnect grace and the claim path's add-before-bind ordering keep the sweep from racing live transport switches.
The network-latency figure was a percentile over every heartbeat since the profiling toggle was armed, so idle-radio wakeup samples (tens of ms) blended with in-game samples (2-7 ms) and the number drifted for minutes instead of reporting current conditions. The RTT ring is now a 64-sample sliding window; stage-1 and URB-gap keep their accumulate-then-dump bench semantics. While the diagnostics latency panel is open, a probe mode drops the heartbeat interval to 250 ms (steady state stays the contractual 2000 ms; the death window stays ~10 s because the miss threshold scales with the interval) and the window is cleared on entry, so the readout converges within seconds and only reflects probe-era samples. markPingSent no longer overwrites an in-flight ping's clock, which previously paired a late ack with a newer ping and read low. The panel shows the sample count next to the figure and refreshes at 1 Hz.
|
Added two fixes on this branch beyond the original scope: 0776d5c fix(slots): 30383d5 fix(diagnostics): the network-latency figure was a percentile over every heartbeat since arming, blending idle-radio wakeups (tens of ms) with in-game samples (2-7 ms). The RTT ring is now a 64-sample sliding window, the panel enables a 250 ms ping probe while open (steady-state cadence stays the contractual 2000 ms; death window still ~10 s via a scaled miss threshold, and the server acks unconditionally so this is protocol-safe), the window clears on probe entry, an in-flight ping's clock is no longer overwritten by the next ping, and the readout shows the sample count. |
emir-hasanbegovic
changed the title
Slot topology had three writable representations kept consistent by convention: the registry, the binding/type stores, and each SatelliteConnection's slot map written imperatively from five call sites. Any invariant slip became a persistent phantom pad because the session PUT faithfully re-broadcasts the slot map. SlotTopologyComposer now derives the desired topology (connectionId -> slotId -> type) purely from SlotBindingStore + ControllerTypeStore, and SlotTopologyController converges every live connection to it through SatelliteConnection.applyDesired, the only production write path into the slot map (the old mutators are internal plumbing under it). A same-type remove+add pair in one emission is resolved as a rename so a USB claim/release keeps the server-side pad alive with its controller index; SlotBindingStore.migrate re-keys in a single emission to make that diff unambiguous. ConnectionCoordinator shrinks to intent recording: store writes only, type written before the binding so the composer can never observe a binding without its type. A binding recorded before the session object exists now converges as soon as the connection appears, closing the old satellite.get() == null hole. AbstractController's contract now states the actuator rule the phantom bug violated: reconcile fully from current inputs on start, never trust memory recorded before a stop.
The ping clock was a process global, so with two live sessions one session's ping could pair with the other's ack. The clock now lives on the Session; hotpath keeps the policy (in-flight guard, loss reclaim, validity cap) and the shared sliding window.
Every STARTED attachment collected every post, so both sides of an activity transition rendered the same banner and persistent banners could land on the outgoing screen. The bus now keeps a resume-ordered attachment stack and routes posts only to the top; deferred posts drain when the next attachment resumes, and a destroyed top attachment falls back to the previous one. Dismissals still broadcast.
The coordinator root and the two low-power overlay includes were duplicated across 15 activity layouts. screen_scaffold.xml now holds them once; BaseGamepadHostActivity.setScaffoldContent inflates each standard screen's content layout into it and wires the gamepad host, system bars, and transitions in one place. The 12 standard screens' layouts become content-only (LinearLayout roots, sw600dp variants included); bespoke screens (main, connections, configure-bindings, input overlays) keep their own coordinator roots. The coverage test now requires the overlays on any coordinator-rooted activity layout and on the scaffold itself.
|
Four architecture follow-ups from the review, each fixing the pattern behind the earlier bugs rather than another instance of them:
Full CI mirror ran locally: clang-format, ktlintCheck, detekt, :app:lint, the complete :app:testDebugUnitTest suite, :app:assembleDebug, and the native build. |
…atch base DishNavigator gains the missing destinations (diagnostics, licenses, input inspector) and the setup-to-dashboard handoff, so every in-app hop goes through it; raw Intents remain only for external targets. The four per-screen copies of openExternalUrl collapse into one base-activity helper, and the Licenses copy stops silently swallowing failures. BaseInputOverlayActivity extends BaseGamepadHostActivity, dropping its duplicate dispatch overrides and injected fields; the fully-qualified inline types in ConnectionsActivity's injections become imports.
DiagnosticsActivity built its cards and rows with programmatic View construction and ConfigureBindingsActivity assembled dialog containers in code, against the layouts-in-XML rule. diagnostics_card/body_row/empty_row and dialog_card_list now carry the structure; the Kotlin only inflates and fills.
ConnectionsActivity read four flows' .value inside render(); the row derivation now lives in pure mappers behind ConnectionsViewModel's single UiState flow, and the post-command render() nudges go away because the state is reactive. DiagnosticsActivity owned two polling loops and JSON parsing; DiagnosticsViewModel owns them now, with the latency probe tied to collection (WhileSubscribed) instead of a try/finally in the activity, and parseLatencyPanel as a pure, tested function.
|
Consistency pass from the audit, four commits:
Full CI mirror green locally: ktlintCheck, detekt, :app:lint, complete :app:testDebugUnitTest, :app:assembleDebug. |
Crashlytics never received native crash reports: the NDK artifact was absent and no symbols were uploaded, so a crash in the hot path would have surfaced only in Play Console vitals. firebase-crashlytics-ndk is now on the classpath, release builds enable nativeSymbolUploadEnabled (gated on google-services.json like the plugins), and the release workflow runs uploadCrashlyticsSymbolFileRelease alongside the build. nativeTestConfigure picks Ninja on Windows instead of the unavailable Unix Makefiles generator, so :app:nativeTest (159 tests) runs locally as well as in CI. StickAxesTest pins the virtual pad's touch-to-axis mapping (clamp, sign flip, saturation), the one piece of that path that only had indirect regression coverage.
|
One more commit,
Also investigated the dead dependency-check: the OWASP Dependency-Check job hung on NVD sync (weekly scheduled Security runs cancelled at the 30-minute timeout from Jun 22 to Jul 6) and was already removed in July in favor of OSV-Scanner + dependency-review + Grype + Dependabot. Next Monday's scheduled run should be the first clean one. |
Dependency-Check left in July but its ghosts stayed: the removal note in security.yml, a suppression-file cross-reference in the allowlist, three CONTRIBUTING sections documenting a gradle task that no longer exists, and the README gate list. All gone; the documented coverage now matches reality (OSV-Scanner + dependency-review on PRs, Grype on release artifacts, Dependabot). release.yml only runs uploadCrashlyticsSymbolFileRelease when google-services.json was decoded: without the secret the crashlytics plugin is never applied, so the unconditional task name would have failed the whole release build.
1 participant
Three streaming-continuity fixes: every screen now hosts gamepad streaming and the wake/dim contract, stale slot bindings self-heal instead of re-creating phantom satellite pads forever, and the diagnostics latency readout reports current conditions instead of a since-arming blend.
1. Stream gamepad input and hold wake state on every screen (
5abe9f6)Only five screens installed
GamepadActivityHost. Everywhere else:SatelliteNative. Most visible on Settings, Diagnostics, and the controller inspector.FLAG_KEEP_SCREEN_ONand the low-power dim countdown disarmed while streaming, so the screen could time out mid-session.USB Direct was unaffected (native reader threads are screen-independent), which is why this only showed in Standard mode.
Changes:
BaseGamepadHostActivity(ui/common) owns the host plumbing: dispatch forwarding, focus-loss release, unbuffered joystick dispatch, keep-screen-on, dim countdown, and dim cancel on stop. Subclasses callinstallGamepadHost(binding.root)aftersetContentView.GameActivitysuperclass). NativeUnavailableActivity stays plain: no native library to stream through.sw600dpvariants of settings/donate/help) includesoverlay_low_powerandoverlay_low_power_chip, which the host binds.StreamingScreenCoverageTestenforces the invariant: every manifest activity extends a gamepad-hosting base (explicit exemptions: MainActivity, NativeUnavailableActivity), MainActivity still declares its own dispatch overrides, and everyactivity_*.xmlin every layout folder carries both overlay includes.2. Sweep stale bindings so phantom satellite pads self-heal (
0776d5c)A device that departed while the binding observer was stopped (app backgrounded during a transport switch) was in neither
presentnorlastBound, soreconcileSlotsnever unbound it. Its slot stayed in the binding store and inSatelliteConnection's declarative slot map, which re-registered a phantom pad on the satellite on every reconnect (the session PUT re-declares the full topology and the server applies it authoritatively), with no UI able to remove it since controller cards render from registry devices.reconcileSlotsnow also treats any numeric binding whose device id the registry no longer knows as departed, emitting the same ordered Unbind/Forget/ReleaseHubBinding ops. Non-numeric logical slots (the on-screen controller) are exempt, matching the existingslotExistsrule. The registry's 5 s disconnect grace and the claim path's add-before-bind ordering keep the sweep from racing live transport switches. Six new tests cover the sweep, the synthetic-id Forget skip, the virtual-slot exemption, dedup with the normal departed path, and departed-before-bind ordering.Behavior note: a controller that departs now loses its binding for real once the grace passes, even if the app was backgrounded at the time, same as an in-foreground departure always did.
3. Windowed RTT with a probe mode so latency reads "now" (
30383d5)The network-latency figure was a percentile over every heartbeat since the profiling toggle was armed (and the toggle persists across launches), so idle-radio wakeup samples (tens of ms, one 2 s ping is worst-case Wi-Fi power-save traffic) blended with in-game samples (2-7 ms) and the number drifted for minutes.
handleHeartbeathas no rate policing), so this is protocol-safe.markPingSentno longer overwrites an in-flight ping's clock, which paired a late ack with a newer ping and read low.~3.41 ms · last 64 pings); the new string is translated in all five locales.Verification
StreamingScreenCoverageTest,PhysicalSlotBindingObserverTest,BindOpDedupeTestgreen locally; full suite on CI.:app:hiltJavaCompileDebug,:app:compileDebugUnitTestKotlin,:app:externalNativeBuildDebug(new JNI symbol confirmed inlibsatellite.sovia llvm-nm),:app:ktlintCheck,:app:detekt, clang-format 22.1.4 all green.Notes for review
DishNotificationsbus on every screen, so connection banners/snackbars now appear app-wide. That matches the bus design but widens exposure to the known banner-overlap quirk during activity transitions.HEARTBEAT_INTERVAL_PROBE_MSto 500 ms is a one-line change.4. Architecture follow-ups (root fixes behind the bugs above)
0be67c1):SlotTopologyComposerprojects the desired satellite topology purely fromSlotBindingStore+ControllerTypeStore;SlotTopologyControllerconverges every live connection viaSatelliteConnection.applyDesired, now the only production write path into a connection's slot map.ConnectionCoordinatorrecords intent only. A USB claim/release resolves as an index-preserving rename from an atomic single-emission binding re-key, so mode switches stay wire-silent. This makes the phantom-pad class unrepresentable (the sweep from fix 2 remains as defense in depth).AbstractControllernow states the actuator rule: reconcile from current inputs on start, never from pre-stop memory.0909b48): the heartbeat ping clock lives on theSession, so concurrent satellites cannot cross-pair pings and acks.b4c896b): posts route only to the most recently resumed attachment (resume-ordered stack with destroy fallback; deferred posts drain on next resume). Fixes the transition-overlap banner leak at the bus, superseding the per-screen mitigation concern flagged in the review notes.1a1a264):screen_scaffold.xmlowns the coordinator root and low-power overlays once; the 12 standard screens are content-only layouts inflated viaBaseGamepadHostActivity.setScaffoldContent. Bespoke screens (main, connections, configure-bindings, input overlays) keep their own chrome. Coverage test updated accordingly.e400d6e):docs/architecture.mdcatches up (per-screen input entry, topology flow, actuator rule, scaffold, notification routing).Full CI mirror ran locally for the follow-ups: clang-format, ktlintCheck, detekt, :app:lint, complete :app:testDebugUnitTest, :app:assembleDebug, native build.