Skip to content

feat: add more details to trainrun events#853

Open
Synar wants to merge 5 commits intomainfrom
ali/improve-events
Open

feat: add more details to trainrun events#853
Synar wants to merge 5 commits intomainfrom
ali/improve-events

Conversation

@Synar
Copy link
Copy Markdown
Contributor

@Synar Synar commented Feb 23, 2026

Close #792

I don't completely understand numberOfStops, so it's possible I have not used the tag properly.

I have not tested the pr in osrd yet. In particular I'm quite unsure about the way to detect forward/backward direction.

The last commit add a few events that seemed missing. There's a couple that are not useful for osrd (as we deactivated the features), but should probably be emitted for other potential users that may want to use the event system too. There are a few that correspond to niche features enabled in osrd, and for which the osrd behavior is currently bugged. Finally I have not added any to visibleTrainrunsSetLabel, as the behavior already seems propagated somehow?

Signed-off-by: Alice K. <alice.khoudli@gmail.com>
Signed-off-by: Alice K. <alice.khoudli@gmail.com>
Signed-off-by: Alice K. <alice.khoudli@gmail.com>
Signed-off-by: Alice K. <alice.khoudli@gmail.com>
Signed-off-by: Alice K. <alice.khoudli@gmail.com>
@Synar Synar requested a review from aiAdrian as a code owner February 23, 2026 14:46
@aiAdrian
Copy link
Copy Markdown
Contributor

I don't completely understand numberOfStops, so it's possible I have not used the tag properly.

The idea behind numberOfStops is that it serves as a placeholder for the number of stations (nodes) that are not explicitly defined. This means we don’t have to draw or specify every station where a train will stop. Instead, we can simply provide a number indicating that the train stops at n stations, without defining where they are or how long the train stays there. The stopping times and travel times are combined for that edge, or the stopping times are already integrated. This helps long‑term planners work more quickly without dealing with details that are not important at this stage of planning.

@aiAdrian
Copy link
Copy Markdown
Contributor

aiAdrian commented Feb 26, 2026

The last commit add a few events that seemed missing. There's a couple that are not useful for osrd (as we deactivated the features), but should probably be emitted for other potential users that may want to use the event system too. There are a few that correspond to niche features enabled in osrd, and for which the osrd behavior is currently bugged. Finally I have not added any to visibleTrainrunsSetLabel, as the behavior already seems propagated somehow?

Could you share an overview of what OSRD uses? It might be helpful to have a short document with links in the technical documentation folder. I think it would be important to provide an overview of the components OSRD relies on, and which application uses which layer or interface (API).

API documentation would be really helpful -> https://github.com/OpenRailAssociation/netzgrafik-editor-frontend/tree/main/documentation/technical (if the documentation is somewhere else -> please share / link it)

@louisgreiner
Copy link
Copy Markdown
Contributor

As OSRD is directly listening to every "operation" events, you can consider that OSRD reacts to every operation.emit you can find in the code of NGE. See our listener.

Such documentation would be difficult to provide, and more important, difficult to maintain. For instance, I tried to note everything here, a long time ago, but I fear that this is not up to date anymore.

@Synar
Copy link
Copy Markdown
Contributor Author

Synar commented Feb 26, 2026

To add some information to Louis's answer, we have only disabled a couple of actions in the fork. For example, I think the action of turning a node into a non node stop or vice versa is disabled in osrd currently (as osrd lack a model for non node stop). We have also disabled the file import (as it's done in osrd interface instead), and we have disabled the save filter actions as we don't store them. I may forget a thing or two, but the differences are intentionally kept minimal. Everything else we use!

@Synar
Copy link
Copy Markdown
Contributor Author

Synar commented Feb 26, 2026

I don't completely understand numberOfStops, so it's possible I have not used the tag properly.

The idea behind numberOfStops is that it serves as a placeholder for the number of stations (nodes) that are not explicitly defined. This means we don’t have to draw or specify every station where a train will stop. Instead, we can simply provide a number indicating that the train stops at n stations, without defining where they are or how long the train stays there. The stopping times and travel times are combined for that edge, or the stopping times are already integrated. This helps long‑term planners work more quickly without dealing with details that are not important at this stage of planning.

Thanks for the explanation!

@louisgreiner
Copy link
Copy Markdown
Contributor

Yep, if you are curious, here are all the things we do differently when we use NGE for OSRD: main...osrd-project:netzgrafik-editor-frontend:standalone

}

updateDirection(trainrun: Trainrun, direction: Direction) {
updateDirection(trainrun: Trainrun, direction: Direction, isTrainInverted?: boolean) {
Copy link
Copy Markdown
Member

@emersion emersion Mar 17, 2026

Choose a reason for hiding this comment

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

Could we pass "forward" | "backward" directly as a function argument to avoid the boolean?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure!

new TrainrunOperation(OperationType.update, trainrunSections.trainrunSection1.getTrainrun()),
new TrainrunUpdateOperation(trainrunSections.trainrunSection1.getTrainrun(), [
"times",
"numberOfStops",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think numberOfStops changes when toggling non-stop?

Copy link
Copy Markdown
Contributor Author

@Synar Synar Mar 19, 2026

Choose a reason for hiding this comment

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

I will trust you on this.

I am relatively confident about the other tags, but for numberOfStops I only have a general idea of its meaning. So I added it to anything that seemed to touch stops (better to update for nothing than miss an update). But yeah, if you do know which functions actually touch it, that's better =p

this.trainrunsUpdated();
this.operation.emit(
new TrainrunUpdateOperation(trainrun1, ["nodes", "times", "numberOfStops"]),
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Aren't we missing a TrainrunDeleteOperation here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm I don't think so, there is a call to this.deleteTrainrun earlier which I believe already emits the event

if (this.getTrainrunSections().length) {
this.operation.emit(
new TrainrunOperation(OperationType.update, trainrunSection.getTrainrun()),
new TrainrunUpdateOperation(trainrunSection.getTrainrun(), [
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm, do you recall why we fire a TrainrunUpdateOperation here?

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 honestly don't remember :/

Copy link
Copy Markdown
Contributor Author

@Synar Synar Mar 19, 2026

Choose a reason for hiding this comment

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

My understanding is as follow:

  • either we deleted the last section of the trainrun, in which case the deletion of the section will trigger the deletion of the trainrun which will fire a delete event
  • or we still have some sections left in the trainrun. In that case we need to update it as we have changed its path, and I don't believe any update event is fired prior.

Copy link
Copy Markdown
Contributor

@aiAdrian aiAdrian left a comment

Choose a reason for hiding this comment

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

For me fine - but i can't test it ! @louisgreiner please give final OK !

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.

[Feature request]: Add more details to NGE trainrun events

4 participants