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

Update report on events #224

Merged
merged 3 commits into from
Jun 8, 2022
Merged

Conversation

aeshub
Copy link
Contributor

@aeshub aeshub commented May 26, 2022

Closes #212
Closes #144

@aeshub aeshub added feature New feature or request backend Backend related functionality labels May 26, 2022
@aeshub aeshub self-assigned this May 26, 2022
Cancelled
}

public enum StepType
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not very happy about the "duplication" of StepType and InspectionType here but not certain what else to do.

@aeshub
Copy link
Contributor Author

aeshub commented May 26, 2022

There is one issue which I'm uncertain how should be addressed.

When starting a mission in ISAR from Flotilla the response from ISAR contains the mission details which Flotilla then writes to the database as a Report object which contains IsarTask and IsarStep as lists. However, the state machine in ISAR has started executing the current mission before Flotilla has written to the database. Thus ISAR is sending status updates for mission, task and step which Flotilla will not recognize because it can't find those IDs in the database yet as the write operation is not complete.

Not entirely certain how this should be resolved.

@oysand
Copy link
Contributor

oysand commented May 27, 2022

There is one issue which I'm uncertain how should be addressed.

When starting a mission in ISAR from Flotilla the response from ISAR contains the mission details which Flotilla then writes to the database as a Report object which contains IsarTask and IsarStep as lists. However, the state machine in ISAR has started executing the current mission before Flotilla has written to the database. Thus ISAR is sending status updates for mission, task and step which Flotilla will not recognize because it can't find those IDs in the database yet as the write operation is not complete.

Not entirely certain how this should be resolved.

I think a better way to solve this issue is to move the misssion planner from ISAR to Flotilla so that Flotilla provides the mission to ISAR. That way we can toggle if ISAR should use a local mission planner or be used in combination with Flotilla. We can then write the mission to the database before sending it to ISAR

@aeshub
Copy link
Contributor Author

aeshub commented Jun 7, 2022

There is one issue which I'm uncertain how should be addressed.
When starting a mission in ISAR from Flotilla the response from ISAR contains the mission details which Flotilla then writes to the database as a Report object which contains IsarTask and IsarStep as lists. However, the state machine in ISAR has started executing the current mission before Flotilla has written to the database. Thus ISAR is sending status updates for mission, task and step which Flotilla will not recognize because it can't find those IDs in the database yet as the write operation is not complete.
Not entirely certain how this should be resolved.

I think a better way to solve this issue is to move the misssion planner from ISAR to Flotilla so that Flotilla provides the mission to ISAR. That way we can toggle if ISAR should use a local mission planner or be used in combination with Flotilla. We can then write the mission to the database before sending it to ISAR

I have mentioned this in a new issue #226. It's easier to solve in a different PR and to just include this functionality as it is now. It works somewhat for turtlebot, but there are some unfortunate sideffects.

Copy link
Contributor

@GodVenn GodVenn left a comment

Choose a reason for hiding this comment

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

LGTM, just one renaming suggestion

backend/api/Database/Models/IsarStep.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@GodVenn GodVenn left a comment

Choose a reason for hiding this comment

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

Some naming suggestions for methods

backend/api/Database/Models/IsarStep.cs Outdated Show resolved Hide resolved
backend/api/Database/Models/IsarStep.cs Outdated Show resolved Hide resolved
backend/api/Database/Models/IsarStep.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@oysand oysand left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@GodVenn GodVenn left a comment

Choose a reason for hiding this comment

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

LGTM

@aeshub aeshub merged commit b5c8415 into equinor:main Jun 8, 2022
@aeshub aeshub deleted the update-report-on-events branch June 8, 2022 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Backend related functionality feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add database models for Task and Step to reflect ISAR/Echo Add topic listener process
3 participants