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

[WIP] Enable trainrun asymmetry #409

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

louisgreiner
Copy link
Collaborator

@louisgreiner louisgreiner commented Mar 7, 2025

In pair programming with @Yohh

Description

PR linked with issue on OSRD OpenRailAssociation repository : OpenRailAssociation/osrd#10576

PR work in progress, for exploration and review purposes

How to review properly

B) TrainrunSection arrival/departure time at nodes

Test all the different combinaisons of +/- 1 for the 5 different time values, and also for all locks combinaisons:

  • leftDeparture / rightArrival / rightDeparture / leftArrival / travelTime
  • leftLock / travelTimeLock / rightLock

left = source and right = target

C) Enable one-way trip

Warning

The return times (targetDeparture and sourceArrival) are only css-hidden, accordingly to the way NGE currently hides elements

Warning

Bug: when a trainrun is non-stopping at a node, the times for the trainrun dialog window are all hidden (because the target is not direct left/right)

D) Enable different travel times for round trips

Question: should we rename travelTime and returnTravelTime into forwardTravelTime and backwardTravelTime? I'm not a huge fan of backward, since we don't really go backward, rather forward also, but in another direction.

Remaining to do

Bugs:

  • for C) fix display times for non-stopping nodes (see above)

Spread the features for the rest of the application:

  • Pearls view (Perlenkette) doesn't take in account the above features, need to spread the changes to it:
    • B) TrainrunSection arrival/departure time at nodes
    • C) Enable one-way trip
    • D) Enable different travel times for round trips
  • Space-time diagram (Streckengrafik) doesn't take in account the above features, need to spread the changes to it:
    • B) TrainrunSection arrival/departure time at nodes
    • C) Enable one-way trip
    • D) Enable different travel times for round trips
  • For D), take in account non-stops for extractTravelTime()

List of features / fixes that would be nice to do if we have the time:

  • Re-calculate symmetric values when symmetric toggle is hit
  • Fix the conditional behaviour of TrainrunsectionValidator where we have set if (true) with a // TODO
  • Documentation on code additions
  • for B), when asymmetry is on
    • current behaviour: +/- 1 leftDepature and +/- 1 leftArrival change the same travelTime
    • target behaviour: +/- 1 leftDepature and +/- 1 leftArrival change a different travelTime
  • fix behaviour when travelTime is reduced to 0
    • current behaviour: rounds to 0.1, then propagating to the other time values with decimal
    • target behaviour: goes to 60, not modifying the other time values
  • For B) and C), use a toggle from SBB design system instead of checkbox
  • Set custom input for node symmetry (something else than 60')
  • Move roundTrip and symmetry checkbox to bottom of the dialog
  • Set isSymmetry for whole trainrun at once

Issues

Checklist

  • This PR contains a description of the changes I'm making
  • I've read the Contribution Guidelines
  • I've added tests for changes or features I've introduced
  • I documented any high-level concepts I'm introducing in documentation/
  • CI is currently green and this is ready for review

@louisgreiner louisgreiner requested a review from aiAdrian as a code owner March 7, 2025 09:44
@louisgreiner louisgreiner force-pushed the lgr-yoh/enable-trainrun-asymmetry branch from b3c156a to f265e62 Compare March 7, 2025 10:10
@louisgreiner louisgreiner changed the title WIP - Enable trairun asymmetry WIP - Enable trainrun asymmetry Mar 11, 2025
@louisgreiner louisgreiner force-pushed the lgr-yoh/enable-trainrun-asymmetry branch 3 times, most recently from cda0e43 to f2baf88 Compare March 12, 2025 16:04
@louisgreiner louisgreiner changed the title WIP - Enable trainrun asymmetry [WIP] Enable trainrun asymmetry Mar 13, 2025
louisgreiner and others added 4 commits March 14, 2025 10:51
* add isSymmetric to Trainrun

Signed-off-by: Louis Greiner <[email protected]>

* enable trainrunSection departure/arrival assymetry at nodes

Signed-off-by: Louis Greiner <[email protected]>

* trainrun: isolate symmetry check (#410)

add checkAndAdjustSymmetry() in trainrunsection.helper.ts

* fix onInputTravel time for asymmetry

Signed-off-by: Louis Greiner <[email protected]>

* fix bug on adjust time on symmetry

Signed-off-by: Louis Greiner <[email protected]>

* to-be-removed: fix left/right inversion for travelTime calculation

Signed-off-by: Louis Greiner <[email protected]>

* fix rebase

Signed-off-by: Louis Greiner <[email protected]>

---------

Signed-off-by: Louis Greiner <[email protected]>
Co-authored-by: Yoh <[email protected]>
* add isSymmetric to Trainrun

Signed-off-by: Louis Greiner <[email protected]>

* enable trainrunSection departure/arrival assymetry at nodes

Signed-off-by: Louis Greiner <[email protected]>

* round-trip: add round trip parameter to trainrun

Signed-off-by: Louis Greiner <[email protected]>

* asymmetry: hide elements for return trip if one-way

Signed-off-by: Louis Greiner <[email protected]>

* refactor: getHiddenTagForTime for TrainrunSectionText

Signed-off-by: Louis Greiner <[email protected]>

* asymmetry: hide trainrunSectionsView times for one-way trips

Signed-off-by: Louis Greiner <[email protected]>

* to-be-removed: fix fix rebase

Signed-off-by: Louis Greiner <[email protected]>

* clean code from comments

Signed-off-by: Louis Greiner <[email protected]>

---------

Signed-off-by: Louis Greiner <[email protected]>
@louisgreiner louisgreiner force-pushed the lgr-yoh/enable-trainrun-asymmetry branch from d931cc6 to 3c2649f Compare March 14, 2025 09:51
- move isSymmetric from TrainrunDto to trainrunServiceDto
- adapt related services and helpers
- in trainrunSection html template, display isSymmetric checkbox only if
  isRoundTrip is true

Signed-off-by: Louis Greiner <[email protected]>
@louisgreiner louisgreiner force-pushed the lgr-yoh/enable-trainrun-asymmetry branch from 3c2649f to d7c4f06 Compare March 14, 2025 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants