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.

Sign up for GitHub

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

Jump to bottom

core: fix "negative" routing requirements + add requirement margin for stop on closed signal (conflict detection) #9195

Merged
merged 3 commits into from
Oct 23, 2024
Merged

core: fix "negative" routing requirements + add requirement margin for stop on closed signal (conflict detection) #9195

merged 3 commits into from
Oct 23, 2024

Conversation

bougue-pe
Copy link
Contributor

@bougue-pe bougue-pe commented Oct 4, 2024 •
edited
Loading

  • Fix routing requirements cases where 'begin' was later than 'end' (stop on closed signal only).
  • Also a 20 s anticipation for the start of resource requirement when stopping on closed signal.

Fix: #8813

@bougue-pe bougue-pe requested a review from a team as a code owner October 4, 2024 09:27
@codecov-commenter
Copy link

codecov-commenter commented Oct 4, 2024 •
edited
Loading

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 39.06%. Comparing base (02ea460) to head (7433ec1).
Report is 10 commits behind head on dev.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@             Coverage Diff              @@
##                dev    #9195      +/-   ##
============================================
- Coverage     39.08%   39.06%   -0.02%     
- Complexity     2267     2270       +3     
============================================
  Files          1308     1308              
  Lines         99203    99251      +48     
  Branches       3312     3315       +3     
============================================
+ Hits          38774    38776       +2     
- Misses        58466    58511      +45     
- Partials       1963     1964       +1
Flag Coverage Δ
core 74.99% <100.00%> (-0.01%) ⬇️
editoast 71.97% <ø> (-0.05%) ⬇️
front 10.32% <ø> (-0.01%) ⬇️
gateway 2.50% <ø> (ø)
osrdyne 3.29% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 86.71% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bougue-pe bougue-pe force-pushed the peb/core/add_closed_signal_reservation_margin branch from 5db36e8 to 7e8ed4e Compare October 4, 2024 09:29
@bougue-pe bougue-pe removed the request for review from Erashin October 10, 2024 08:08
bougue-pe

This comment was marked as off-topic.

Sign in to view

@bougue-pe bougue-pe force-pushed the peb/core/add_closed_signal_reservation_margin branch from 7e8ed4e to d4e146b Compare October 11, 2024 06:57
@bougue-pe
Copy link
Contributor Author

bougue-pe commented Oct 11, 2024 •
edited
Loading

Please review d4e146b: I fixed a little bit the processing of critical distance, as it could lead to use a stop that was before the signalSight in routing requirement (too late if): d4e146b#diff-8078a2f57f53541732c3008fa2e15551402baba5959c846d63fa84e9d9637e4bL395
Also checked that we are not requiring before the signalSight if stop on closed-signal is shorter than 20s and close to signal-sight position.

@bougue-pe bougue-pe force-pushed the peb/core/add_closed_signal_reservation_margin branch from d4e146b to 026bf86 Compare October 17, 2024 16:33
@bougue-pe bougue-pe changed the title core: add requirement margin for stop on closed signal (conflict detection) core: fix "negative" routing requirements + add requirement margin for stop on closed signal (conflict detection) Oct 17, 2024
@bougue-pe
Copy link
Contributor Author

bougue-pe commented Oct 17, 2024 •
edited
Loading

@Khoyo @eckter please re-review as there is an important fix in 8e3010d (and a rename to clarify in previous commit).
This bug actually prevented to trigger the bug fixed in d4e146b.

Tested locally that assertRequirementsPeriodsConsistency does break previous to that commit.

@bougue-pe bougue-pe force-pushed the peb/core/add_closed_signal_reservation_margin branch from 026bf86 to 8e3010d Compare October 17, 2024 20:57
@bougue-pe
core: add requirement margin for stop on closed signal (conflict dete…
3ffbe50 
…ction)

20 s anticipation for the start of resource requirement when stopping on
closed signal.

Signed-off-by: Pierre-Etienne Bougué <bougue.pe@proton.me>
@bougue-pe
core: clarify naming in routing requirements
b3dcb0c 
Signed-off-by: Pierre-Etienne Bougué <bougue.pe@proton.me>
@bougue-pe
core: fix "negative" routing requirements
7433ec1 
Begin of routing requirement was sometimes placed later than the end
requirement of a zone.
This would happen when a stop on closed signal was after the considered
route (and this route was not the first one, as there is an early return).

Too early entrySignalOffset had to be fixed too.

Signed-off-by: Pierre-Etienne Bougué <bougue.pe@proton.me>
@bougue-pe bougue-pe force-pushed the peb/core/add_closed_signal_reservation_margin branch from 8e3010d to 7433ec1 Compare October 23, 2024 17:05
@bougue-pe bougue-pe added this pull request to the merge queue Oct 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 23, 2024
@bougue-pe bougue-pe added this pull request to the merge queue Oct 23, 2024
Merged via the queue into dev with commit 45f92e1 Oct 23, 2024
24 checks passed
@bougue-pe bougue-pe deleted the peb/core/add_closed_signal_reservation_margin branch October 23, 2024 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Erashin Erashin Erashin approved these changes

@Khoyo Khoyo Khoyo approved these changes

@eckter eckter Awaiting requested review from eckter

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Core: start conflict detection on closed signal 20s before train restart
5 participants
@bougue-pe @codecov-commenter @Khoyo @eckter @Erashin