Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ final class StopVehicleAnnotation: VehicleAnnotation {
// clobbered exactly as the initializer's comment describes.
//
// Restore `from` before `VehicleCoordinateUpdate.apply` so a city-block
// hop interpolates instead of that didSet teleporting the pin. A
// kilometre-scale jump still snaps; see VehicleCoordinateUpdate.
// hop interpolates instead of that didSet teleporting the pin.
// A kilometre-scale jump still snaps; see VehicleCoordinateUpdate (#1341).
let from = self.coordinate
self.tripStatus = tripStatus
self.coordinate = from
Expand Down
7 changes: 5 additions & 2 deletions OBAKit/Mapping/VehicleCoordinateUpdate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,13 @@ import UIKit
///
/// Arrival polls land every 15–30s. Assigning `coordinate` each time teleports
/// the pin; interpolating every hop looks like motion. Hops longer than
/// `snapBeyondMeters` are a new fix (or a trip change), not something to ease
/// across the map.
/// `snapBeyondMeters` (**500 m** — a bit over one 30s poll at ~50 km/h) snap
/// instead. That is a deliberate city-traffic tuning: freeway/express buses
/// near or above ~60 km/h usually snap and degrade to the pre-interpolation
/// behavior rather than easing across hundreds of metres.
///
/// See: https://github.com/OneBusAway/onebusaway-ios/issues/1109
/// Follow-up: https://github.com/OneBusAway/onebusaway-ios/issues/1341
enum VehicleCoordinateUpdate {
enum Decision: Equatable {
case unchanged
Expand Down
21 changes: 21 additions & 0 deletions OBAKit/Trip/TripViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -272,14 +272,35 @@ class TripViewController: UIViewController,
}

if let vehicleAnnotation = vehicleAnnotation {
// `tripStatus`'s didSet writes lastKnownLocation onto coordinate
// immediately. Restore `from` so `VehicleCoordinateUpdate` can
// interpolate instead of teleporting (#1341) — same pattern as
// `TripFocusMapLayer.drawVehicle`.
let from = vehicleAnnotation.coordinate
vehicleAnnotation.tripStatus = currentTripStatus
vehicleAnnotation.coordinate = from
// No lastKnownLocation → drop the pin, matching `TripFocusMapLayer`
// (`removeVehicle()` when the feed omits a coordinate). Keeping a
// stale real coordinate used to drag `showAnnotations` zoom and
// skip the null-island filter that the old `(0,0)` fallback hit.
guard let to = currentTripStatus.lastKnownLocation?.coordinate else {
removeVehicleAnnotation()
updateTitleView()
return
}
VehicleCoordinateUpdate.apply(from: from, to: to, on: vehicleAnnotation)
// Update the annotation view's heading and real-time state since
// the annotation property didSet on the view won't re-fire.
if let vehicleAnnotationView = vehicleAnnotationView as? PulsingVehicleAnnotationView {
vehicleAnnotationView.applyTripStatus(currentTripStatus)
}
}
else {
// Don't mint a pin on null island when the feed has no location yet.
guard currentTripStatus.lastKnownLocation != nil else {
updateTitleView()
return
}
vehicleAnnotation = VehicleAnnotation(tripStatus: currentTripStatus)
self.mapView.addAnnotation(vehicleAnnotation!)
}
Expand Down
44 changes: 42 additions & 2 deletions OBAKitTests/Mapping/VehicleCoordinateUpdateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//

import CoreLocation
import MapKit
import Testing
import OBAKitCore
@testable import OBAKit
Expand Down Expand Up @@ -35,8 +36,9 @@ struct VehicleCoordinateUpdateTests {
#expect(decision == .animate(duration: VehicleCoordinateUpdate.animationDuration))
}

/// Farther than `snapBeyondMeters` is a new fix, not motion.
@Test func `A kilometre jump snaps`() {
/// Farther than `snapBeyondMeters` (500 m) is a new fix, not motion.
/// A ~1 km hop is well past that cliff — snaps rather than animating.
@Test func `A jump beyond snapBeyondMeters snaps`() {
let far = CLLocationCoordinate2D(latitude: 47.62, longitude: -122.33)
#expect(VehicleCoordinateUpdate.decision(from: seattle, to: far) == .snap)
}
Expand All @@ -49,4 +51,42 @@ struct VehicleCoordinateUpdateTests {
@Test func `An invalid coordinate snaps`() {
#expect(VehicleCoordinateUpdate.decision(from: kCLLocationCoordinate2DInvalid, to: seattle) == .snap)
}

// MARK: - apply(from:to:on:)

@Test @MainActor func `Apply snap writes the destination coordinate`() {
let annotation = MKPointAnnotation()
annotation.coordinate = seattle
let far = CLLocationCoordinate2D(latitude: 47.62, longitude: -122.33)

VehicleCoordinateUpdate.apply(from: seattle, to: far, on: annotation)

#expect(annotation.coordinate.latitude == far.latitude)
#expect(annotation.coordinate.longitude == far.longitude)
}

@Test @MainActor func `Apply unchanged leaves the coordinate alone`() {
let annotation = MKPointAnnotation()
annotation.coordinate = seattle
let almost = CLLocationCoordinate2D(latitude: 47.6062, longitude: -122.33211)

VehicleCoordinateUpdate.apply(from: seattle, to: almost, on: annotation)

#expect(annotation.coordinate.latitude == seattle.latitude)
#expect(annotation.coordinate.longitude == seattle.longitude)
}

@Test @MainActor func `Apply animate eventually reaches the destination`() async {
let annotation = MKPointAnnotation()
annotation.coordinate = seattle

VehicleCoordinateUpdate.apply(from: seattle, to: nearby, on: annotation)

// `UIView.animate`'s animations closure assigns `coordinate` synchronously;
// `MKPointAnnotation` has no presentation-layer interpolation, so the
// destination is already written. This still proves the `.animate` branch
// runs `apply` rather than ignoring the hop.
#expect(abs(annotation.coordinate.latitude - nearby.latitude) < 0.00001)
#expect(abs(annotation.coordinate.longitude - nearby.longitude) < 0.00001)
}
}
28 changes: 28 additions & 0 deletions docs/vehicle-coordinate-interpolation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Vehicle coordinate interpolation (#1109 / #1323 / #1341)

Between arrival polls the trip-page (and stop-route-focus) vehicle marker
interpolates when the hop is short, and snaps when it is not.

## Thresholds

`VehicleCoordinateUpdate` has three branches:

- `ignoreBelowMeters` — hops smaller than this are left alone (noise).
- interpolate — city-block hops animate for `animationDuration`.
- `snapBeyondMeters` (**500 m**) — longer hops teleport. A bit over one 30 s
poll at ~50 km/h (city-traffic tuning). At *exactly* 500 m the comparison is
`>`, so that hop still animates; freeway / express buses usually snap.

Docs and tests describe the constants, not a motorway-speed framing.

## Apply path

Callers that assign `VehicleAnnotation.tripStatus` must restore the previous
`coordinate` before `VehicleCoordinateUpdate.apply`, because `tripStatus`'s
`didSet` writes `lastKnownLocation` immediately and would otherwise skip the
animation. Used by:

- `TripFocusMapLayer.drawVehicle` (removes the pin when the feed omits a coordinate)
- `StopVehicleAnnotation.update`
- `TripViewController.currentTripStatus` (legacy trip screen; same remove-when-nil
policy as the focus layer — fixed in #1341)
Loading