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

Make MissionEventHandler events pass the relevant robot state in the event arguments #1891

Open
andchiind opened this issue Dec 17, 2024 · 0 comments · May be fixed by #1920
Open

Make MissionEventHandler events pass the relevant robot state in the event arguments #1891

andchiind opened this issue Dec 17, 2024 · 0 comments · May be fixed by #1920
Labels
backend Backend related functionality improvement Improvement to existing functionality Northern Lights

Comments

@andchiind
Copy link
Contributor

Describe the improvement you would like to see
Currently these events trigger mechanics based on user interactions or ISAR updates which updated Flotilla state, but they still read in the current robot state again when the event is handled. This leads to nondeterministic behaviour where event handlers can no longer handle the event as the state they are reading in is inconsistent with what was read in when triggering the event. I suggest that the events in MissionEventHandler should pass along the relevant state when the event was triggered, so we can be sure that the event is handled as intended when we call the event. For instance, when triggering OnRobotAvailable, we want a mission to be started, but if the state is read in again after the event is triggered, we can end up with inconcistent state which soft locks us from starting a mission. If the robot becomes available we want to guarantee that at least one event handler tries to run the mission. Trying to many times is not an issue if we handle the ISAR return values correctly.

Once this change is made, we can also make OnIsarStatus in MqttEventHandler.cs use the OnRobotAvailable event instead of calling MissionScheduling.StartNextMissionRunIfSystemIsAvailable.

How will this change existing functionality?
It will make the system truly event-based as we can rely on event handlers to handle the event instead of the perceived current state of the system which may be inconcistent at the time the event is handled.

How will this improvement affect the current Threat Model?
N/A

@andchiind andchiind added backend Backend related functionality improvement Improvement to existing functionality Northern Lights labels Dec 17, 2024
@andchiind andchiind linked a pull request Jan 2, 2025 that will close this issue
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Backend related functionality improvement Improvement to existing functionality Northern Lights
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant