Skip to content

Commit 0bb532f

Browse files
committed
Address CodeRabbit's second-pass review comments
- urimap.json: add the /api/api alias for the no_active_trip fixture, matching its sibling HART trip-details entries (the Tampa region's base URL produces the doubled path). - TripExtrapolationController: every failed-frame exit now hides the vehicle marker and estimate overlays — the state-missing/shape-missing early returns and the degenerate-schedule catch previously left a prior frame's marker on screen, unlike the other failure paths. The thrice-duplicated hide pair is now one hideFrameOverlays(), and the non-finite-median and null-interpolation branches collapse into a single failed-frame branch. - TripState.withStatus KDoc: the "0 to use local clock" fallback never existed — entries without a server time are skipped (and tested as such); the doc now says so. - SingleFlight rethrows CancellationException instead of masking a cancelled scope as a logged null result, matching the convention everywhere else on this branch.
1 parent ec8820b commit 0bb532f

4 files changed

Lines changed: 30 additions & 22 deletions

File tree

onebusaway-android/src/androidTest/res/raw/urimap.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
"/api/where/trip-details/Hillsborough%20Area%20Regional%20Transit_1389962.json": "trip_details_hart_1389962",
7575
"/api/api/where/trip-details/Hillsborough%20Area%20Regional%20Transit_1389962.json": "trip_details_hart_1389962",
7676
"/api/where/trip-details/Hillsborough%20Area%20Regional%20Transit_1389962_no_active_trip.json": "trip_details_hart_1389962_no_active_trip",
77+
"/api/api/where/trip-details/Hillsborough%20Area%20Regional%20Transit_1389962_no_active_trip.json": "trip_details_hart_1389962_no_active_trip",
7778
"/api/where/trip-details/1_18196913.json?includeSchedule=false": "trip_details_1_18196913_no_schedule",
7879
"/api/where/trip-details/1_18196913.json?includeStatus=false": "trip_details_1_18196913_no_status",
7980
"/api/where/trip-details/1_18196913.json?includeTrip=false": "trip_details_1_18196913_no_trip",

onebusaway-android/src/google/java/org/onebusaway/android/map/googlemapsv2/tripmap/TripExtrapolationController.kt

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,14 @@ internal constructor(
4141
fun stop() = frameLoop.stop()
4242

4343
private fun doFrame(now: Long) {
44-
val state = lookupTripState(tripId) ?: return
45-
val shapeData = state.polyline ?: return
44+
val state = lookupTripState(tripId)
45+
val shapeData = state?.polyline
46+
if (state == null || shapeData == null) {
47+
// Nothing to render this frame (trip evicted or shape not hydrated) — don't
48+
// leave a previous frame's marker on screen
49+
hideFrameOverlays()
50+
return
51+
}
4652
val result =
4753
try {
4854
state.extrapolate(now)
@@ -51,37 +57,34 @@ internal constructor(
5157
// surfaces, then skip this frame; the next frame retries. Anything else
5258
// propagates rather than degrading silently at 20fps.
5359
Log.w(TAG, "Extrapolation failed for $tripId", e)
60+
hideFrameOverlays()
5461
return
5562
}
5663

5764
when (result) {
5865
is ExtrapolationResult.Success -> {
5966
val distribution = result.distribution
6067
val medianDist = distribution.median()
61-
if (medianDist.isFinite()) {
62-
val loc = shapeData.interpolate(medianDist)
63-
if (loc != null) {
64-
vehicleOverlay.updateVehiclePosition(loc, state.anchor, now)
65-
vehicleOverlay.updateEstimateOverlays(distribution)
66-
} else {
67-
// Interpolation failed (degenerate polyline) — treat the frame as
68-
// failed rather than leaving a stale marker on screen
69-
vehicleOverlay.hideVehicleMarker()
70-
vehicleOverlay.hideEstimateOverlays()
71-
}
68+
val loc = if (medianDist.isFinite()) shapeData.interpolate(medianDist) else null
69+
if (loc != null) {
70+
vehicleOverlay.updateVehiclePosition(loc, state.anchor, now)
71+
vehicleOverlay.updateEstimateOverlays(distribution)
7272
} else {
73-
// Bisect could not converge on a quantile — treat as a failed
74-
// extrapolation frame rather than propagating NaN into rendering.
75-
vehicleOverlay.hideVehicleMarker()
76-
vehicleOverlay.hideEstimateOverlays()
73+
// Bisect could not converge on a quantile, or interpolation failed on a
74+
// degenerate polyline — treat the frame as failed rather than leaving a
75+
// stale marker or propagating NaN into rendering.
76+
hideFrameOverlays()
7777
}
7878
}
79-
else -> {
80-
vehicleOverlay.hideVehicleMarker()
81-
vehicleOverlay.hideEstimateOverlays()
82-
}
79+
else -> hideFrameOverlays()
8380
}
8481

8582
state.anchor?.let { vehicleOverlay.showOrUpdateDataReceivedMarker(it, now) }
8683
}
84+
85+
/** Every failed frame renders the same way: no vehicle marker, no estimate overlays. */
86+
private fun hideFrameOverlays() {
87+
vehicleOverlay.hideVehicleMarker()
88+
vehicleOverlay.hideEstimateOverlays()
89+
}
8790
}

onebusaway-android/src/main/java/org/onebusaway/android/extrapolation/data/TripState.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ data class TripState(
107107
* keeps the snapshot — and with it the lazily fitted extrapolator and the anchor
108108
* instance — intact.
109109
*
110-
* @param serverTimeMs the server's currentTime from the API response, or 0 to use local clock
110+
* @param serverTimeMs the server's currentTime from the API response; entries without one
111+
* (zero or negative) are skipped, since the anchor clocks couldn't be derived
111112
* @param localTimeMs local device clock time when the response was received
112113
*/
113114
fun withStatus(status: ObaTripStatus, serverTimeMs: Long, localTimeMs: Long): TripState {

onebusaway-android/src/main/java/org/onebusaway/android/util/SingleFlight.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.onebusaway.android.util
1717

1818
import android.util.Log
19+
import kotlinx.coroutines.CancellationException
1920
import kotlinx.coroutines.CoroutineScope
2021
import kotlinx.coroutines.Deferred
2122
import kotlinx.coroutines.async
@@ -44,6 +45,8 @@ class SingleFlight<K : Any, V : Any>(private val scope: CoroutineScope) {
4445
scope.async {
4546
try {
4647
block()
48+
} catch (e: CancellationException) {
49+
throw e // scope cancelled — propagate, don't mask as a null result
4750
} catch (e: Exception) {
4851
Log.e(TAG, "Single-flight block failed for $key", e)
4952
null

0 commit comments

Comments
 (0)