Basic support for for taxi traverse mode #2373

Open
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
@mlundblad

Add support for "communal taxi" mode. Based on patch in #2280

@novalis

Does this require corresponding front-end changes?

@mlundblad

This comment has been minimized.

Show comment
Hide comment
@mlundblad

mlundblad Dec 18, 2016

Actually, I've only looked at it at the API level, since that's what I'm using :)
Will have to check if the web frontend needs changes as well…

Actually, I've only looked at it at the API level, since that's what I'm using :)
Will have to check if the web frontend needs changes as well…

@andreyz

This comment has been minimized.

Show comment
Hide comment
@andreyz

andreyz Dec 18, 2016

Contributor

@novalis the answer from my experience is a no. Communal taxi is part of public transit mode umbrella and web UI doesn't offer very granular selection of modes at that level of specificity, i.e. it isn't possible to choose just Gondola as a mode.

Contributor

andreyz commented Dec 18, 2016

@novalis the answer from my experience is a no. Communal taxi is part of public transit mode umbrella and web UI doesn't offer very granular selection of modes at that level of specificity, i.e. it isn't possible to choose just Gondola as a mode.

@novalis

This comment has been minimized.

Show comment
Hide comment
@novalis

novalis Dec 19, 2016

Collaborator

https://github.com/opentripplanner/OpenTripPlanner/tree/master/src/client/images/mode is one place where modes appear. src/client/js/otp/locale/en.json and src/client/js/otp/util/Itin.js are two more.

Collaborator

novalis commented Dec 19, 2016

https://github.com/opentripplanner/OpenTripPlanner/tree/master/src/client/images/mode is one place where modes appear. src/client/js/otp/locale/en.json and src/client/js/otp/util/Itin.js are two more.

@abyrd

This comment has been minimized.

Show comment
Hide comment
@abyrd

abyrd Mar 14, 2017

Member

I am concerned that full support for an additional mode may be a little more complicated than this. But I see that these taxis are in GTFS as public transit (they are in fact more like tiny buses). Can we get some confirmation from other users/developers that this patch works properly with no conflicts? I'd say as long as the new mode is not selectable in the UI (available by API only) we wouldn't really need to worry about full support in the UI.

Member

abyrd commented Mar 14, 2017

I am concerned that full support for an additional mode may be a little more complicated than this. But I see that these taxis are in GTFS as public transit (they are in fact more like tiny buses). Can we get some confirmation from other users/developers that this patch works properly with no conflicts? I'd say as long as the new mode is not selectable in the UI (available by API only) we wouldn't really need to worry about full support in the UI.

@abyrd abyrd assigned abyrd and unassigned mlundblad Mar 14, 2017

@mlundblad

This comment has been minimized.

Show comment
Hide comment
@mlundblad

mlundblad Mar 14, 2017

Yes, they appear in GTFS data and basically works as tiny buses (but they often have restrictions, such as you need to book a trip in advance, but they run according to a schedule, still).

Yes, they appear in GTFS data and basically works as tiny buses (but they often have restrictions, such as you need to book a trip in advance, but they run according to a schedule, still).

@johannilsson

This comment has been minimized.

Show comment
Hide comment
@johannilsson

johannilsson Mar 14, 2017

Contributor

@abyrd We're using this patch in production too, the feed we see this mode in requires additional
data similar to what @mlundblad described before we can present it to the user though.

Contributor

johannilsson commented Mar 14, 2017

@abyrd We're using this patch in production too, the feed we see this mode in requires additional
data similar to what @mlundblad described before we can present it to the user though.

@abyrd

This comment has been minimized.

Show comment
Hide comment
@abyrd

abyrd Mar 14, 2017

Member

@johannilsson if you set the pickup_type to "2: Must phone agency to arrange pickup" does it work as expected in OTP?

Member

abyrd commented Mar 14, 2017

@johannilsson if you set the pickup_type to "2: Must phone agency to arrange pickup" does it work as expected in OTP?

@johannilsson

This comment has been minimized.

Show comment
Hide comment
@johannilsson

johannilsson Mar 14, 2017

Contributor

@abyrd Unfortunatly I can't tell, the feed we see this in does not provide correct pickup type for this so we haven't looked into it. But I think OTP would handle it as long as the type is not 1 (No pickup available). OTP does not expose pickup type in any responses though which I guess would be good together with this patch to make it a bit easier for clients to make decisions.

Contributor

johannilsson commented Mar 14, 2017

@abyrd Unfortunatly I can't tell, the feed we see this in does not provide correct pickup type for this so we haven't looked into it. But I think OTP would handle it as long as the type is not 1 (No pickup available). OTP does not expose pickup type in any responses though which I guess would be good together with this patch to make it a bit easier for clients to make decisions.

@johannilsson

This comment has been minimized.

Show comment
Hide comment
@johannilsson

johannilsson Mar 14, 2017

Contributor

Sorry I was misstaken, OTP do expose pickup types in the plan response in the fields boardRule and alightRule.

Contributor

johannilsson commented Mar 14, 2017

Sorry I was misstaken, OTP do expose pickup types in the plan response in the fields boardRule and alightRule.

@novalis

This comment has been minimized.

Show comment
Hide comment
@novalis

novalis Mar 26, 2017

Collaborator

Other than the missing front-end changes, this looks good to me. Can you add in the changes that I mentioned above, and we'll merge this?

Collaborator

novalis commented Mar 26, 2017

Other than the missing front-end changes, this looks good to me. Can you add in the changes that I mentioned above, and we'll merge this?

@mlundblad

This comment has been minimized.

Show comment
Hide comment
@mlundblad

mlundblad May 12, 2017

novalis: I'm not exactly sure about the front-end changes you referred to above?

novalis: I'm not exactly sure about the front-end changes you referred to above?

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