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

Implement localization as part of a mission #1071

Closed
wants to merge 18 commits into from
Closed

Implement localization as part of a mission #1071

wants to merge 18 commits into from

Conversation

aeshub
Copy link
Contributor

@aeshub aeshub commented Oct 18, 2023

No description provided.

@aeshub aeshub added feature New feature or request breaking-change A breaking change which introduces changes to the public APIs labels Oct 18, 2023
@aeshub aeshub self-assigned this Oct 18, 2023
@github-actions
Copy link

🔔 Migrations changes detected 🔔
📣 Remember to comment "/UpdateDatabase" after review approval for migrations to take effect!

@github-actions github-actions bot added the database-change Will require migration label Oct 18, 2023
@aeshub aeshub force-pushed the automatic-localization branch 4 times, most recently from 39c80d0 to eacd5f0 Compare October 24, 2023 13:53
@aeshub aeshub marked this pull request as draft October 24, 2023 13:54
@aeshub aeshub force-pushed the automatic-localization branch 2 times, most recently from 177e68a to 921d2ae Compare October 31, 2023 13:34
@aeshub aeshub mentioned this pull request Nov 9, 2023
Closed
@aeshub aeshub force-pushed the automatic-localization branch 2 times, most recently from 42a00c8 to e1bbd87 Compare November 12, 2023 12:21
@aeshub aeshub force-pushed the automatic-localization branch 4 times, most recently from 235acd8 to 6ede7d6 Compare November 15, 2023 08:47
@github-actions GitHub Actions
Copy link

🔔 Changes in database folder detected 🔔
Do these changes require adding new migrations? 🤔 In that case follow these steps.
If you are uncertain, ask a database admin on the team 😄

@aeshub aeshub force-pushed the automatic-localization branch from 6ede7d6 to bff7038 Compare November 15, 2023 08:49
@aeshub aeshub force-pushed the automatic-localization branch from bff7038 to 6b1ddca Compare November 15, 2023 12:43
@aeshub aeshub force-pushed the automatic-localization branch from 6b1ddca to 7ae264e Compare November 24, 2023 07:42
@Afonso-2403 Afonso-2403 force-pushed the automatic-localization branch 2 times, most recently from ad9ec9d to 398c780 Compare December 19, 2023 07:21
andchiind
andchiind requested changes Dec 19, 2023
View reviewed changes
Copy link
Contributor

@andchiind andchiind left a comment

Choose a reason for hiding this comment

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

Most of these comments are not hard requirements, but I'm just blocking the PR due to the granular access issues.

backend/api/Database/Models/MissionRun.cs Outdated Show resolved Hide resolved
backend/api/EventHandlers/MissionEventHandler.cs
_scheduleLocalizationSemaphore.WaitOne();

string? localizationMissionRunId = null;
try { localizationMissionRunId = await LocalizationService.EnsureRobotIsCorrectlyLocalized(missionRun.Robot, missionRun); }
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, since we don't make use of the exception that is returned anyways

backend/api/EventHandlers/MissionEventHandler.cs Outdated Show resolved Hide resolved
backend/api/Services/AreaService.cs Outdated Show resolved Hide resolved
backend/api/Services/AreaService.cs Outdated Show resolved Hide resolved
backend/api/Services/AreaService.cs Outdated Show resolved Hide resolved
backend/api/Services/LocalizationService.cs Show resolved Hide resolved
backend/api/Services/MissionDefinitionService.cs Outdated Show resolved Hide resolved
@aeshub aeshub force-pushed the automatic-localization branch from 7a99c91 to 355f6e3 Compare December 19, 2023 11:55
@aeshub aeshub marked this pull request as ready for review December 19, 2023 13:05
andchiind
andchiind approved these changes Dec 19, 2023
View reviewed changes
Copy link
Contributor

@andchiind andchiind 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 and others added 18 commits December 19, 2023 16:04
@aeshub
Use robot pose directly from Echo request
6b0debe
@aeshub
Fix formatting
e7e7404
@aeshub
Implement localization as part of a mission
9def051 
The localization procedure is now implemented and will run as part of
regular mission scheduling.

If the queue is empty a localization mission will be started for the
current deck. This assumes that the operator has confirmed that the
robot is on the deck of the mission that has been scheduled.

If there is an existing mission the system will check if a new mission
is in the same deck as that mission and if so schedule it. If not it
will be rejected.

If the last mission finishes a return to home mission will be scheduled
which puts the robot back at the default localization pose. If a
mission is scheduled in between the return to home mission another
localization will not be required.
@aeshub
Add small fixes
b86cabb
@aeshub
Move mutex to event handler
413c9d0
@Afonso-2403 @aeshub
Add TODO comments to unhnadled exceptions
f90f009
@Afonso-2403 @aeshub
Refactor to remove dotnet format warnings
8973db5
@Afonso-2403 @aeshub
Refactor exceptions
d9d76a3
@Afonso-2403 @aeshub
Test localization functionality and fix failing tests
b878847
@Afonso-2403 @aeshub
Cancel missions if localization fails
0a5a0ac
@Afonso-2403 @aeshub
Use Enum for MissionTask Type
0fb47b0
@aeshub
Set default safe positions for areas
12d6a8f
@aeshub
Remove redundant function
6337c4f
@aeshub
Simplify mission type checks
1f394f6
@aeshub
Ensure semaphore is released in case of exception
48b7391
@aeshub
Use GetAreas function in AreaService
5ab2e77
@aeshub
Fix formatting issues
925a0c6
@aeshub
Remove redundant services
7d4e999
@aeshub aeshub force-pushed the automatic-localization branch from bfe08bd to 7d4e999 Compare December 19, 2023 15:04
@aeshub
Copy link
Contributor Author

aeshub commented Dec 19, 2023

/UpdateDatebase

@aeshub
Copy link
Contributor Author

aeshub commented Dec 19, 2023

/UpdateDatabase

@github-actions GitHub Actions
Copy link

⛔ Cannot update database until the Pull Request is approved! ⛔

@aeshub
Copy link
Contributor Author

aeshub commented Dec 19, 2023

Creating new PR to manage migrations

@aeshub aeshub closed this Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oysand oysand oysand left review comments

@Afonso-2403 Afonso-2403 Afonso-2403 left review comments

@andchiind andchiind andchiind approved these changes

Assignees

@aeshub aeshub

Labels
breaking-change A breaking change which introduces changes to the public APIs database-change Will require migration feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aeshub @andchiind @oysand @Afonso-2403