Skip to content

Handle miscellaneous service as BUS instead of crashing build - #3755

Merged
optionsome merged 5 commits into
opentripplanner:dev-2.xfrom
HSLdevcom:handle-miscellaneous-service
Dec 1, 2021
Merged

Handle miscellaneous service as BUS instead of crashing build#3755
optionsome merged 5 commits into
opentripplanner:dev-2.xfrom
HSLdevcom:handle-miscellaneous-service

Conversation

@optionsome

Copy link
Copy Markdown
Member

Summary

Naive support for miscellaneous service route type (17xx) from GTFS. It is now handled as BUS instead of crashing build.

Issue

closes #3754

Unit tests

Tested manually

Code style

Have you followed the suggested code style?
Yes

Documentation

RoutingModes.md is the only relevant file I can think of but it might be updated at some point when support for submodes is added.

Changelog

From title

@optionsome
optionsome requested a review from a team as a code owner November 23, 2021 12:11
@optionsome

optionsome commented Nov 23, 2021

Copy link
Copy Markdown
Member Author

Should there be some log message from this conversion like for 15xx route types or issue store update? Nothing is being logged for 16xx messages currently where the same naive conversion to BUS is made.

t2gran
t2gran previously approved these changes Nov 23, 2021
@t2gran t2gran added this to the 2.1 milestone Nov 23, 2021
@t2gran t2gran added +GTFS Related to import of GTFS data !Improvement A functional improvement or micro feature labels Nov 23, 2021
@leonardehrenfried

Copy link
Copy Markdown
Member

https://developers.google.com/transit/gtfs/reference/extended-route-types lists only "horse-drawn carriage" under miscellaneous. Is this what it is? :)

@t2gran

t2gran commented Nov 23, 2021

Copy link
Copy Markdown
Member

Maybe Rickshaw as well? ;-)

@optionsome
optionsome dismissed stale reviews from leonardehrenfried and t2gran via 78ddd52 November 26, 2021 10:58
@optionsome

Copy link
Copy Markdown
Member Author

We discussed this in yesterday's meeting and I further discussed this today with @hannesj . We decided that the log warning could be replaced with issue store entry and also we don't need to set default mode for stops.

* @param route The route for which the mode is for
* @param issueStore Issue store
*/
public static TransitMode mapModeForRoute(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thinks it would make sense to move this to RouteMapper

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if there was a better place for this. However, would it be weird to use the gtfs RouteMapper from the netex RouteMapper. To me it feels a bit weird that the netex uses methods from the gtfs package but it already does use this file that is under the gtfs package. In the current state, this TransitModeMapper is closely linked to GTFS and therefore it feels reasonable that it's in the gtfs package.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this today and in the future, netex will no longer use this gtfs transit mode mapper. I refactored the code to RouteMapper as suggested.

@optionsome
optionsome merged commit 7933df3 into opentripplanner:dev-2.x Dec 1, 2021
@optionsome
optionsome deleted the handle-miscellaneous-service branch December 1, 2021 18:43
t2gran pushed a commit that referenced this pull request Dec 1, 2021
@miklcct

miklcct commented Sep 25, 2024

Copy link
Copy Markdown
Contributor

Should we add HORSE as a transport mode in our models for GTFS route type 1702?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

+GTFS Related to import of GTFS data !Improvement A functional improvement or micro feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Miscellaneous service gtfs route types should be handled gracefully

5 participants