Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OJPTripChangeRequest - TripParams added #384

Merged
merged 11 commits into from
Aug 4, 2023

Conversation

trurlurl
Copy link
Contributor

This solves #383. The params of the initial TripRequest can now be passed.

I removed two example files since they concerned TripRefineRequest.

@herlitze Can you please check that my understanding of ChangeLegRef is correct? I slightly adapted the annotation to clarify the meaning and changed the example to match that understanding.

@trurlurl trurlurl added bug Something isn't working documentation labels Jul 19, 2023
@trurlurl trurlurl added this to the v1.1 milestone Jul 19, 2023
herlitze
herlitze previously approved these changes Jul 20, 2023
<EarlierArrivalLaterDeparture>laterDeparture</EarlierArrivalLaterDeparture>
<TripParams>
<NoStairs>true</NoStairs>
<NumberOfResults>3</NumberOfResults>
Copy link
Contributor

Choose a reason for hiding this comment

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

NumberOfResults=3 does not make sense in my opinion, you are just changing one specified trip, so I would expect 1 trip as result.

@@ -1768,7 +1768,7 @@
<xs:sequence>
<xs:element name="ChangeLegRef" type="LegObjectIdType" maxOccurs="unbounded">
<xs:annotation>
<xs:documentation>Refers to the legs to be changed by the server. </xs:documentation>
<xs:documentation>Refers to the transfer legs to be changed by the server. </xs:documentation>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would you want to restrict it to transfer legs? There might be future use cases which do not work for transfer legs.
For requesting more transfer time I think both legs could be understood, the transfer leg or the timedLeg before/after. So the change of the examples is fine for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, then things are still not completely clear, at least to me.

@herlitze My interpretation was that the ChangeLegRef would be the place to specify where exactly in the trip the passenger wants to have more time for an interchange. The ChangeLegRef would tell the transfer leg corresponding to this interchange. Another possible interpretation didn't seem to make much sense to me: specifying all the legs that might be changed, because I guess that every leg either before (earlier arrival) or after (later departure) the transfer leg is potentially subject to change.

Copy link
Contributor

Choose a reason for hiding this comment

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

@trurlurl Yes, that is my interpretation as well. I am just saying this does not have to be a transfer leg. The TripChange-Request is not used for requesting longer transfers only. When you want to request the times for a specific sharing operator, you would pass the sharing operator you are interested in and the current trip with the generic sharing leg. This sharing leg is not a transfer leg. In the future there might be other use cases as well.

My second argument (For requesting more transfer time I think both legs could be understood, the transfer leg or the timedLeg before/after. So the change of the examples is fine for me.) was for requesting a longer transfer.

  1. TimedLeg1
  2. TransferLeg
  3. TimedLeg2

You have two options you either arrive earlier at the stop of the transfer, or depart later from the stop of the transfer.
Let's say we want to keep TimedLeg1 and want to get a TimedLeg with later departure instead of TimedLeg2.

To do this you would expect to send ChangeLegRef 2 with laterDeparture, my idea was to send ChangeLegRef3 because it is the leg that actually departs later. In my opinion to send ChangeLegRef 2 the wording of the enum should be changed as well to "Extend to the back/front" instead of later/earlierDeparture.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@herlitze You are right that the wording of the enum should be changed. I did it and adapted the annotations to reflect what you explained.

Perhaps last question: In the scenario of getting the exact times of two sharing legs, it only seems possible to specify the same operator for both legs?

@ue71603 ue71603 requested a review from skinkie July 24, 2023 14:41
@ue71603 ue71603 modified the milestones: v1.1, v2.0 Jul 24, 2023
@skinkie skinkie merged commit 2627629 into changes_for_v1.1 Aug 4, 2023
@skinkie skinkie deleted the trip_change_trip_params branch August 4, 2023 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working doc updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants