feat(routing): use graphhopper to provide GTFS based public transport routing #441

kschrab · 2024-11-29T16:21:40Z

Description

A new routing interface for finding public transport routes.

Open issues:

test with larger GTFS
approve graphhopper-reader-gtfs
approve io.mobilitydata.transit:gtfs-realtime-bindings
approve org.mapdb:mapdb
~~approve javacsv - it's LGPL and I cannot find its source code :(~~ approve opencsv

Issue(s) related to this PR

Partly resolves internal issue 955

Affected parts of the online documentation

Changes in the documentation required?

Definition of Done

Prerequisites

You have read CONTRIBUTING.md carefully.
You have signed the Contributor License Agreement.
Your GitHub user id is linked with your Eclipse Account.

Required

The title of this merge request follows the scheme type(scope): description (in the style of Conventional Commits)
You have assigned a suitable label to this pull request (e.g., enhancement, or bugfix)
origin/main has been merged into your Fork.
Coding guidelines have been followed (see CONTRIBUTING.md).
All checks on GitHub pass.
All tests on Jenkins pass.

Requested (can be enforced by maintainers)

New functionality is covered by unit tests or integration tests. Code coverage must not decrease.
If a bug has been fixed, a new unit test has been written (beforehand) to prove misbehavior
There are no new SpotBugs warnings.

Special notes to reviewer

schwepmo

Nice new feature! However, I think there is some refactoring necessary before merging this PR.

schwepmo · 2024-12-03T10:18:46Z

...ipse/mosaic/fed/application/ambassador/simulation/navigation/CentralNavigationComponent.java

@@ -83,11 +87,22 @@ public class CentralNavigationComponent {
     */
    private Routing routing;

+
+    /**
+     * Public Transport routing


Checkstyle would probably like a dot here :D

schwepmo · 2024-12-03T15:49:55Z

...lication/src/main/java/org/eclipse/mosaic/fed/application/config/CApplicationAmbassador.java

+     * Configuration options for route calculation via public transport.
+     * Requires paths to OSM and GTFS files.
+     */
+    public CPublicTransportRouting publicTransportConfiguration = new CPublicTransportRouting();


It's a little bit confusing that we define default configs for the perception and publicTransportRouting but not for the navigationConfiguration

They both now have default configs.

schwepmo · 2024-12-03T16:04:25Z

lib/mosaic-routing/src/main/java/org/eclipse/mosaic/lib/routing/config/CVehicleRouting.java

@@ -20,7 +20,7 @@
 /**
 * Base Class for the navigation configuration.
 */
-public class CRouting implements Serializable {
+public class CVehicleRouting implements Serializable {

    /**
     * The source for the route calculation, e.g. the path to the database containing the road network.


Suggested change

* The source for the route calculation, e.g. the path to the database containing the road network.

* The source for the route calculation, e.g., the path to the database containing the road network.

schwepmo · 2024-12-03T16:13:13Z

lib/mosaic-routing/src/main/java/org/eclipse/mosaic/lib/routing/pt/MultiModalLeg.java

+    private WalkLeg walkLeg = null;
+
+    //For legs where a vehicle already exists
+    private String carID = null;


Suggested change

private String carID = null;

private String carId = null;

schwepmo · 2024-12-03T16:15:12Z

lib/mosaic-routing/src/main/java/org/eclipse/mosaic/lib/routing/pt/MultiModalLeg.java

+    //For legs where a vehicle needs to be spawned
+    private VehicleDeparture vehicleLeg = null;
+
+    //For PTlegs


This comment isn't really explaining much and can be formatted nicer

Suggested change

//For PTlegs

// For {@link PtLeg PtLegs}

Improved javadoc a lot here

schwepmo · 2024-12-04T09:51:15Z

lib/mosaic-routing/src/main/java/org/eclipse/mosaic/lib/routing/pt/PtRouting.java

+        return scheduleDateTime.plusNanos(simTime).atZone(timeZone).toInstant();
+    }
+
+    private Long fromScheduleTime(Date date) {


Suggested change

private Long fromScheduleTime(Date date) {

private Long fromScheduleTime(@Nonnull Date date) {

Erm, you mean @Nullable? The method is able to handle the null case.

schwepmo · 2024-12-04T09:52:02Z

lib/mosaic-routing/src/main/java/org/eclipse/mosaic/lib/routing/pt/PtRouting.java

+        ).createWithoutRealtimeFeed();
+    }
+
+


Remove some blank lines here

schwepmo · 2024-12-04T09:52:26Z

lib/mosaic-routing/src/main/java/org/eclipse/mosaic/lib/routing/pt/PtRouting.java

+        return scheduleDateTime.until(LocalDateTime.ofInstant(instant, timeZone), ChronoUnit.NANOS);
+    }
+
+    public void close() {


This seems to never be used?

Now it does

lib/mosaic-routing/src/test/java/org/eclipse/mosaic/lib/routing/pt/PtRoutingTest.java

lib/mosaic-routing/src/main/java/com/csvreader/CsvReader.java

kschrab · 2024-12-11T13:11:13Z

@schwepmo I also added a small UNITS class for conversion of kmh to meters per second

… routing

…m GTFS

rti/mosaic-rti-api/src/main/java/org/eclipse/mosaic/rti/UNITS.java

This reverts commit 20dbb20.

schwepmo requested changes Dec 4, 2024

View reviewed changes

schwepmo reviewed Dec 4, 2024

View reviewed changes

lib/mosaic-routing/src/main/java/com/csvreader/CsvReader.java Show resolved Hide resolved

kschrab requested a review from schwepmo December 11, 2024 13:11

kschrab added the enhancement New feature or request label Dec 11, 2024

kschrab added 9 commits December 11, 2024 14:51

feat(routing): use graphhopper to provide GTFS based public transport…

2ed219a

… routing

docs(routing): documentation for pt routing config

25541ae

refactor(routing): use opencsv instead of javacsv for CSV parsing fro…

85ece70

…m GTFS

clean(routing): exclude unused transient libraries

7abcd47

clean(routing): resolve several checkstyle problems

789f5bc

clean(routing): define default navigationConfiguration

1ab6f22

fix(routing): fix handling of empty lines

af26c87

feat(routing): new UNITS class for km/h conversion

20dbb20

docs(routing): javadoc

3e0229a

kschrab force-pushed the 955-public-transport-routing branch from a8aaca0 to 3e0229a Compare December 11, 2024 13:51

schwepmo reviewed Dec 11, 2024

View reviewed changes

kschrab added 2 commits December 12, 2024 10:08

Revert "feat(routing): new UNITS class for km/h conversion"

875b11b

This reverts commit 20dbb20.

clean(routing): use SpeedUtils for kmh conversion

f7e0aa9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(routing): use graphhopper to provide GTFS based public transport routing #441

feat(routing): use graphhopper to provide GTFS based public transport routing #441

kschrab commented Nov 29, 2024 •

edited

Loading

schwepmo left a comment

schwepmo Dec 3, 2024

schwepmo Dec 3, 2024

kschrab Dec 11, 2024

schwepmo Dec 3, 2024

schwepmo Dec 3, 2024

schwepmo Dec 3, 2024

kschrab Dec 11, 2024

schwepmo Dec 4, 2024

kschrab Dec 5, 2024

schwepmo Dec 4, 2024

schwepmo Dec 4, 2024

kschrab Dec 11, 2024

kschrab commented Dec 11, 2024

	* The source for the route calculation, e.g. the path to the database containing the road network.
	* The source for the route calculation, e.g., the path to the database containing the road network.

	private Long fromScheduleTime(Date date) {
	private Long fromScheduleTime(@Nonnull Date date) {

feat(routing): use graphhopper to provide GTFS based public transport routing #441

Are you sure you want to change the base?

feat(routing): use graphhopper to provide GTFS based public transport routing #441

Conversation

kschrab commented Nov 29, 2024 • edited Loading

Description

Issue(s) related to this PR

Affected parts of the online documentation

Definition of Done

Special notes to reviewer

schwepmo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kschrab commented Dec 11, 2024

kschrab commented Nov 29, 2024 •

edited

Loading