Improvements to train travel mode based on recent field tests

shared/src/commonMain/kotlin/org/scottishtecharmy/soundscape/geoengine/GeoEngine.kt

@@ -18,6 +18,7 @@ import org.scottishtecharmy.soundscape.geoengine.callouts.buildNearbyMarkersCall
 import org.scottishtecharmy.soundscape.geoengine.callouts.buildWhatsAroundMeCallout
 import org.scottishtecharmy.soundscape.geoengine.filters.MapMatchFilter
 import org.scottishtecharmy.soundscape.geoengine.filters.RailMatchArbiter
+import org.scottishtecharmy.soundscape.geoengine.filters.StationaryDetector
 import org.scottishtecharmy.soundscape.geoengine.filters.TrackedCallout
 import org.scottishtecharmy.soundscape.geoengine.mvttranslation.MvtFeature
 import org.scottishtecharmy.soundscape.geoengine.mvttranslation.Way
@@ -119,10 +120,14 @@ class GeoEngine {
     private var railMatchArbiter = RailMatchArbiter()
     // RailMatchArbiter's verdict from the most recent location update.
     private var arbitratedRailway: Way? = null
+    // Whether the user has actually gone anywhere lately, which instantaneous speed cannot say -
+    // see StationaryDetector. Driven below, ahead of the matchers, since the arbiter reads it.
+    private var stationaryDetector = StationaryDetector()
+    private var userStationary = false
 
     // Running total of the time fixes have been arriving too inaccurate to place, and the
     // bookkeeping behind it - see UserGeometry.unobservedMillis and
-    // AutoCallout.discountUnobservedTime.
+    // AutoCallout.discountUninformativeTime.
     private var unobservedMillis = 0L
     private var lastUsableFixMillis: Long? = null
     private var blindSinceLastUsableFix = false
@@ -244,7 +249,12 @@ class GeoEngine {
             currentBeacon = beaconLocation,
             inStreetPreview = streetPreview.running,
             timestampMilliseconds = currentTimeMillis(),
-            unobservedMillis = unobservedMillis
+            unobservedMillis = unobservedMillis,
+            // Also decided when the filters ran, and for the same reason: a UserGeometry is one
+            // location update, and whether the user has gone anywhere is a question about the last
+            // minute of them. See StationaryDetector.
+            stationary = userStationary,
+            stationaryMillis = stationaryDetector.stationaryMillis
         )
     }
 
@@ -466,7 +476,7 @@ class GeoEngine {
                     if (!isAccuracyUsable(location) && haveUsableLocation) {
                         // Fixes are arriving, they just can't be placed. That's the geoengine
                         // going blind rather than idle, and the two mean opposite things to the
-                        // callout sticky windows - see AutoCallout.discountUnobservedTime.
+                        // callout sticky windows - see AutoCallout.discountUninformativeTime.
                         blindSinceLastUsableFix = true
                         return@let
                     }
@@ -500,6 +510,39 @@ class GeoEngine {
                         withContext(gridState.treeContext) {
                             locationProvider.locationFlow.value?.let { unfilteredLocation ->
                                 val unfilteredSpeed = speedFromLocation(unfilteredLocation)
+
+                                // Ahead of both matchers, because RailMatchArbiter reads the
+                                // verdict below. MvtTileTest.testMovingGrid mirrors this ordering
+                                // by hand - if the two drift apart the replays stop reflecting
+                                // what the app does, silently.
+                                //
+                                // The unfiltered flow, deliberately: the same stream the matchers
+                                // are fed, and the one the detector's thresholds were measured on.
+                                // The Kalman-filtered position has had exactly the jitter the
+                                // detector measures smoothed out of it.
+                                //
+                                // The course flag is the GPS course and nothing else - not the
+                                // phone's compass, not the head tracker. Someone standing still
+                                // holding the phone up to read the screen has a rock-steady
+                                // compass heading and has gone nowhere. Stricter than the
+                                // travelHeading gate below, which trusts an ungated bearing: a
+                                // bearing with no accuracy beside it is fine to steer audio with,
+                                // but says nothing about whether the user is moving.
+                                userStationary = stationaryDetector.update(
+                                    LngLatAlt(
+                                        unfilteredLocation.longitude,
+                                        unfilteredLocation.latitude
+                                    ),
+                                    if (unfilteredLocation.hasAccuracy)
+                                        unfilteredLocation.accuracy.toDouble()
+                                    else
+                                        null,
+                                    unfilteredLocation.hasBearing &&
+                                        unfilteredLocation.hasBearingAccuracy &&
+                                        (unfilteredLocation.bearingAccuracyDegrees < 45.0),
+                                    nowMillis
+                                )
+
                                 val mapMatchTime = measureTime {
                                     mapMatchFilter.filter(
                                         LngLatAlt(
@@ -532,7 +575,8 @@ class GeoEngine {
                                 // them: a ride can only end somewhere the train has slowed enough
                                 // to be got off at.
                                 arbitratedRailway = railMatchArbiter.update(
-                                    mapMatchFilter, railMapMatchFilter, unfilteredSpeed
+                                    mapMatchFilter, railMapMatchFilter, unfilteredSpeed,
+                                    userStationary
                                 )
                             }
                         }

shared/src/commonMain/kotlin/org/scottishtecharmy/soundscape/geoengine/GeoEngineHelpers.kt

@@ -255,6 +255,42 @@ internal fun formatDecimal(
     return "$sign$whole$separator$fracOut"
 }
 
+/**
+ * How a road is named to someone travelling: its route number and its local street name together
+ * where it has both, "A81 (Glasgow Road)".
+ *
+ * The number matters to a traveller in a way it doesn't to a pedestrian. It is how the road is
+ * signposted and how it is talked about, and it is the part that stays put while the street name
+ * changes along it - the A81 through Milngavie is Strathblane Road, then Glasgow Road, then Main
+ * Street.
+ *
+ * Used both for the road being travelled along and for a road being crossed - see
+ * AutoCallout.announceableCrossing, where a train passing over the A81 should say so rather than
+ * naming only the street it happens to be called there.
+ *
+ * @param ref the road's route number, or null where it has none or shouldn't be spoken - a railway
+ * line carries no road-style ref, so callers pass null for a train.
+ * @param ownName the name the road actually carries in OSM ([MvtFeature.displayName]), which is
+ * what the ref is worth pairing with.
+ * @param describedName what [Way.getName] made of the road, used when there is no ref to pair
+ * with. That may be a confected description rather than a real name, which is exactly why it is
+ * kept separate from [ownName]: a road with a ref and no name of its own gets a description built
+ * around the ref ("A779 that joins Tailend Moss and Old Deans Road"), and pairing *that* with the
+ * ref produced "A91 (A91 that joins Mathieson Gardens and Middleflat)". Such a road is just its
+ * number.
+ */
+fun roadNameWithRef(
+    ref: String?,
+    ownName: String?,
+    describedName: String?,
+    localized: LocalizedStrings?
+): String? = when {
+    ref == null -> describedName
+    (ownName == null) || (ownName == ref) -> ref
+    else -> localized?.get(StringKey.DirectionsRoadWithRefAndName, ref, ownName)
+        ?: "$ref ($ownName)"
+}
+
 /**
  * Tracks the last railway station passed while travelling by train, so travel-mode reverse
  * geocoding can describe progress along the line as "distance since {station}" rather than just
@@ -269,6 +305,16 @@ class LastStationTracker {
         name = newName
         location = newLocation
     }
+
+    /**
+     * Called when the ride the station was measured on has ended. A distance since a station is a
+     * statement about progress along one journey, and means nothing once that journey is over -
+     * see AutoCallout.buildCalloutForRoadSense, which decides when that has happened.
+     */
+    fun clear() {
+        name = null
+        location = null
+    }
 }
 
 /**
@@ -425,14 +471,8 @@ private fun travellingReverseGeocodeName(
     // the spoken name. Railway lines don't carry a road-style ref and have their own naming path
     // in Way.getName(), so trains are excluded.
     val roadRef = if (!probablyOnTrain) nearestRoad?.ref else null
-    val spokenRoadName = when {
-        roadRef == null -> roadName
-        // getName() already falls back to the ref for a way with no name of its own, so this
-        // covers both "unnamed" and "named after the number anyway" without saying "M8 (M8)".
-        (roadName == null) || (roadName == roadRef) -> roadRef
-        else -> localized?.get(StringKey.DirectionsRoadWithRefAndName, roadRef, roadName)
-            ?: "$roadRef ($roadName)"
-    }
+    val spokenRoadName =
+        roadNameWithRef(roadRef, nearestRoad?.displayName, roadName, localized)
     // Used in the dedup keys below in place of the spoken name, which embeds the street name and
     // so changes along an unchanged road.
     val roadIdentity = roadRef ?: roadName
@@ -580,11 +620,17 @@ private fun travellingReverseGeocodeName(
                     StringKey.DirectionsOnRoadAndSettlementSince,
                     spokenRoadName, nearestSettlementName, distanceText, sinceStationName
                 ) ?: "On $spokenRoadName and close to $nearestSettlementName, $distanceText since $sinceStationName",
-                // Keep the station in the dedup key (unlike the distance, which is never
-                // included) - a genuinely new "since {station}" is worth a fresh announcement,
-                // only the ever-climbing distance number itself shouldn't defeat deduping. This
-                // key is never spoken, so it doesn't need localizing.
-                dedupText = "On $roadIdentity and close to $nearestSettlementName since $sinceStationName"
+                // The station this is measured from is the whole key. The settlement is left out
+                // for the same reason a numbered road's street name is (see roadDedup): at line
+                // speed the nearest one changes almost every location update, so keying on it
+                // re-announced the same stretch of the same journey over and over. The distance is
+                // left out because it climbs on every call and would defeat deduping entirely. And
+                // the line is left out because its name isn't stable enough to key on - the rail
+                // matcher flickers onto depot sidings and adjacent lines mid-journey, and each
+                // flicker would be a fresh announcement. What is left is the one thing that
+                // genuinely marks progress: the last station called at. This key is never spoken,
+                // so it needs no localizing.
+                dedupText = "since $sinceStationName"
             )
         }
 
@@ -593,13 +639,20 @@ private fun travellingReverseGeocodeName(
         // settlement mention.
         if (probablyOnTrain) {
             return ReverseGeocodeText(
-                if (nearestSettlementName != null) {
+                text = if (nearestSettlementName != null) {
                     localized?.get(
                         StringKey.DirectionsOnRoadAndSettlement, spokenRoadName, nearestSettlementName
                     ) ?: "On $spokenRoadName and close to $nearestSettlementName"
                 } else {
                     phrase
-                }
+                },
+                // The settlement is out of the key for the same reason as in the "since station"
+                // case above - passing one isn't news about which line we're on. This is the form
+                // used before any station has been tracked, so there's nothing else to mark
+                // progress by and the line alone is the whole key. Equivalent to roadIdentity (a
+                // train never carries a roadRef - see above), but expressed through spokenRoadName
+                // so the compiler can see it's non-null in this branch.
+                dedupText = "On $spokenRoadName"
             )
         }
 

shared/src/commonMain/kotlin/org/scottishtecharmy/soundscape/geoengine/UserGeometry.kt

@@ -35,7 +35,17 @@ import kotlin.math.abs
  * @param unobservedMillis is how long, in total this session, the geoengine has had fixes arriving
  * that were too inaccurate to place at all (see isAccuracyUsable) - time that passed with nothing
  * observed in it. A running total rather than a per-update figure, since not every location update
- * reaches every consumer; see AutoCallout.discountUnobservedTime, which takes the difference.
+ * reaches every consumer; see AutoCallout.discountUninformativeTime, which takes the difference.
+ * @param stationary is true if the user is standing still rather than travelling. This is a
+ * windowed judgement - it asks whether they have gone anywhere over the last minute, because
+ * instantaneous speed cannot tell standing from walking at all (see StationaryDetector) - and a
+ * UserGeometry is a snapshot of one location update, so it cannot work this out for itself. It is
+ * decided by StationaryDetector and handed in, the same way mapMatchedRailway is decided by
+ * RailMatchArbiter and handed in.
+ * @param stationaryMillis is how long, in total this session, the user has been seen to be standing
+ * still. The counterpart of [unobservedMillis], a running total for the same reason, and consumed
+ * in the same place: time in which nothing can have changed shouldn't count against a window
+ * measuring how long ago something happened.
  *
  * The heading prioritization comes from iOS - see https://github.com/Scottish-Tech-Army/Soundscape-Android/issues/364
  *
@@ -59,6 +69,8 @@ class UserGeometry(
     val ruler: Ruler = CheapRuler(location.latitude),
     val timestampMilliseconds: Long = 0L,
     val unobservedMillis: Long = 0L,
+    val stationary: Boolean = false,
+    val stationaryMillis: Long = 0L,
     private val headingMode: HeadingMode = HeadingMode.CourseAuto,
     private var travelHeading: Double? = null,
     private var headHeading: Double? = null,
@@ -77,7 +89,14 @@ class UserGeometry(
     }
 
     fun inMotion(): Boolean {
-        return speed > 0.2
+        // Instantaneous GPS speed cannot tell standing still from walking. Measured across four
+        // recorded journeys, somebody standing still has a median speed of 1.33 m/s against
+        // 1.39 m/s walking, and this test on its own was true for every stationary window in the
+        // recordings - which is how standing on the concourse at Glasgow Queen Street came to be
+        // read as walking about. Whether the user has actually gone anywhere is a question about
+        // the last half minute, so StationaryDetector answers it and gets the veto; the speed
+        // check stays as the fast path for everything the window hasn't decided yet.
+        return (speed > 0.2) && !stationary
     }
 
     /**
@@ -90,9 +109,21 @@ class UserGeometry(
      * which used to be enough for a driver to be told "On Winchburgh Chord". Deciding this needs
      * the railway match to be weighed against the road match from the same update, which is
      * RailMatchArbiter's job; by the time mapMatchedRailway is set here, that's already happened.
+     *
+     * A train stopped at a station is still a train. Requiring [inVehicle] alone made this false
+     * throughout every dwell - and the recorded dwells run to 340 seconds - so the line the
+     * arbiter was still holding went unused for the whole of each stop, and the callouts that
+     * depend on knowing the user is on a train switched off at exactly the moment a passenger most
+     * wants them.
+     *
+     * Both halves are kept rather than trusting the arbiter's lock alone, because the lock can
+     * outlive the ride by design: walking up a platform stays within the arbiter's
+     * onTheLineDistanceMetres of the line, and a concourse often has no confident road match to
+     * weigh against it. Somebody neither travelling at vehicle speed nor standing where the train
+     * stopped is walking, and walking out of a station is not being on a train.
      */
     fun probablyOnTrain(): Boolean {
-        return inVehicle() && (mapMatchedRailway != null)
+        return (mapMatchedRailway != null) && (inVehicle() || stationary)
     }
 
     private fun transform(distance: Double): Double {