Conversation
- [x] vehicle location - [ ] alerts - [ ] trip update
This comment was marked as outdated.
This comment was marked as outdated.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the GTFS Real-Time provider logic to improve filtering and fallback mechanisms for trip updates, service alerts, and vehicle positions. Key improvements include the introduction of the GTFSRealTimeProviderFilter interface, centralized UUID resolution, and a new fallback mechanism in GTFSRealTimeTripUpdatesProvider to generate schedules directly from trip updates. The review feedback correctly identifies critical issues in the new fallback logic, specifically regarding the handling of last-stop timestamps which typically lack departure times, and points out a performance bottleneck in nested stop-searching loops.
…SRealTimeTripUpdatesProvider.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…SRealTimeTripUpdatesProvider.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances the GTFS Real-Time provider and logging utilities. It introduces new logging extension functions in MtLogExt.kt, centralizes time formatting in MTLog.java, and implements a fallback mechanism in GTFSRealTimeTripUpdatesProvider.kt to generate schedules from trip updates when static data is missing. The PR also refactors target UUID retrieval across various providers by introducing the GTFSRealTimeProviderFilter interface and associated extension functions. Review feedback suggests using a more idiomatic LOG_TAG for consistency in logging calls within GTFSRealTimeTripUpdatesProvider.kt.
|
There was a problem hiding this comment.
Pull request overview
Adds GTFS-RT “trip ID no match” fallbacks across vehicle positions, service alerts, and trip updates so real-time data can still be surfaced when static trip IDs don’t align with the feed.
Changes:
- Introduce a shared
GTFSRealTimeProviderFilter+ extensions to derive target UUIDs / primary UUIDs consistently. - Add fallback behavior for GTFS-RT vehicle positions & service alerts when trip IDs are missing/empty.
- Add a trip-updates fallback path that can synthesize schedules from StopTimeUpdates when timestamps are provided (vs delays), plus some logging / schedule-string refactors.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/mtransit/android/MtLogExt.kt | Adds duration/date-time log helpers (but currently formats Duration incorrectly). |
| src/main/java/org/mtransit/android/commons/provider/vehiclelocations/VehicleLocationProviderContract.kt | Makes vehicle-location filter implement GTFSRealTimeProviderFilter for shared UUID derivation. |
| src/main/java/org/mtransit/android/commons/provider/vehiclelocations/VehicleLocationProvider.kt | Adds single-UUID overload for cached vehicle-location retrieval. |
| src/main/java/org/mtransit/android/commons/provider/vehiclelocations/GTFSRealTimeVehiclePositionsProvider.kt | Implements fallback caching logic when trip IDs aren’t available, using a primary target UUID. |
| src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProviderExt.kt | Makes stop-sequence matching optional when finding closest trip timestamp. |
| src/main/java/org/mtransit/android/commons/provider/status/GTFSRealTimeTripUpdatesProvider.kt | Adds schedule-synthesis fallback from TripUpdates and refactors real-time schedule caching. |
| src/main/java/org/mtransit/android/commons/provider/serviceupdate/ServiceUpdateProviderContract.java | Makes service-alert filter implement GTFSRealTimeProviderFilter (Java interop getters). |
| src/main/java/org/mtransit/android/commons/provider/serviceupdate/GTFSRealTimeServiceAlertsProvider.kt | Adds fallback to unfiltered alerts when trip IDs are missing/empty. |
| src/main/java/org/mtransit/android/commons/provider/GTFSRealTimeProvider.java | Exposes getAGENCY_TIME_ZONE(...) publicly for shared extensions. |
| src/main/java/org/mtransit/android/commons/provider/gtfs/GtfsStatusProviderExt.kt | Parameterizes RDS schedule query window (look-behind, max data requests). |
| src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSStatusProvider.java | Minor signature/visibility adjustments + clarifying comments. |
| src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSRealTimeProviderFilter.kt | New interface enabling shared GTFS-RT filter behavior across providers. |
| src/main/java/org/mtransit/android/commons/provider/gtfs/GTFSRealTimeProviderExt.kt | Adds shared ignoreDirection/timeZone and UUID derivation helpers for filters. |
| src/main/java/org/mtransit/android/commons/provider/gtfs/GtfsRealtimeExt.kt | Improves stop-time event logging by formatting event times as date-times. |
| src/main/java/org/mtransit/android/commons/MTLog.java | Adds makeTime(...) helpers and reuses them in log prefix formatting. |
| src/main/java/org/mtransit/android/commons/data/ScheduleExt.kt | Refactors schedule creation + introduces richer Schedule.toString() output (but currently logs delays as date-times). |
| src/main/java/org/mtransit/android/commons/data/Schedule.java | Delegates toString() to Kotlin extension and normalizes arrivalDiffMs zero handling. |
There was a problem hiding this comment.
Code Review
This pull request refactors the GTFS Real-Time provider implementation, introducing a unified GTFSRealTimeProviderFilter interface and enhancing logging capabilities through new Kotlin extension functions. Key changes include the addition of fallback logic for trip updates and vehicle positions when trip IDs are unavailable, as well as improvements to schedule formatting and data validity checks. Feedback was provided to ensure consistent use of LOG_TAG for logging within the GTFSRealTimeTripUpdatesProvider object.
Uh oh!
There was an error while loading. Please reload this page.