Mission Planning improvements #1469

ArturoManzoli · 2024-11-26T20:01:11Z

New Features List:

Added a context menu for navigating through mission planning creation and editing processes
Surveys can now be relocated by dragging them on the map
Surveys can be deleted
Users can select the angle at which the survey will be scanned
The scanning angle can also be edited after the survey is completed
Surveys can now be edited back to the polygon level, allowing users to adjust the distance between scan lines (scan density) and the scanning angle
It is now possible to concatenate multiple surveys or simple paths to form a complex mission around a complex map scene

Screenshare.-.2024-11-26.4_37_36.PM.mp4

Screenshare.-.2024-11-26.5_44_58.PM.mp4

closes #1355
closes #1350

rafaellehmkuhl · 2024-11-26T20:11:19Z

@ArturoManzoli since the two first commits are unrelated to the mission planning part, could you open dedicated PRs for them? This way we can test them separately and merge sooner. This will also make the review and rebase processes easier.

rafaellehmkuhl

Nice improvements!

I will add some points from my usage tests here.

Could we make the survey path follow the angle change live, as it was before? It helps specially with surveys with concavities where the user needs to know if the change in the survey angle will make it go outside the survey polygon.

Kapture.2024-12-05.at.06.12.11.mp4

The survey angle change step should be 0.1 degrees. It's currently not limited.

I cannot select the regular mission path waypoints to see them, and so I cannot see information around them.

Kapture.2024-12-05.at.06.15.19.mp4

If I have more than one survey, I cannot edit them. Is this expected? Once I removed one of them, I could edit the other again.

Kapture.2024-12-05.at.06.17.43.mp4

"Scan density" should be renamed to something like "Scan spacing (m)", as the density is actually decreasing as the number increases.
I couldn't find the button to delete all waypoints. Where is it?
About Allow user to specify the angle of attack for a survey mission #1350, is this PR changing something on the logic related to the survey angle?

Will take a better look at the code and put notes at a separate review.

julled · 2024-12-08T15:41:37Z

Very nice new features! looking forward to get them integrated!

ArturoManzoli · 2024-12-09T11:18:06Z

Could we make the survey path follow the angle change live, as it was before? It helps specially with surveys with concavities where the user needs to know if the change in the survey angle will make it go outside the survey polygon.

Didn't get what you mean here. Are you referring to the dashed lines that preview the survey scan path? Because they do update live.

Done
This is almost done. Its coming on the next mission-planning related PR. Check it out:
https://github.com/user-attachments/assets/e1aff670-3eee-43ea-b50a-6e335720925c
Yes it is a known limitation that the current Undo process has. It will be addressed on a near future improvement task.
Done

I couldn't find the button to delete all waypoints. Where is it?

Oops, its added now:

About Allow user to specify the angle of attack for a survey mission #1350, is this PR changing something on the logic related to the survey angle?

Yes, it did implement the feature of selecting and changing the 'angle of attack' for a survey. Although, we did decided later on to not call it 'angle of attack' because of the existing aeronautical term.

rafaellehmkuhl · 2024-12-09T12:07:35Z

Could we make the survey path follow the angle change live, as it was before? It helps specially with surveys with concavities where the user needs to know if the change in the survey angle will make it go outside the survey polygon.

Didn't get what you mean here. Are you referring to the dashed lines that preview the survey scan path? Because they do update live.

As discussed on a meeting, the idea here is to the path angle to change live while the user is dragging the arrow, instead of needing to release and press it repeatedly to archive the desired path.

rafaellehmkuhl

Some thoughts around the code:

This PR has around 8 big changes. It is important to separate those into several commits during development. It is really hard to review one big commit with 2 thousand lines, and it's also difficult to make sure nothing is broken during the review process, as all changes are made to the same commit.
When moving a file that is also being modified in the same commit, we lose the diff completely. This is what is happening with the MissionPlanningView.vue file. There's no way for those reviewing the code to understand what is being changed. This is specially a problem here where the file had 700 lines added. My suggestion is that this file remains where it was, as it is the base view file, and move the ContextMenu.vue and ScanDirectionDial.vue to a subfolder in the src/components folder instead.
The provide/inject functionality was added in Vue to make it easier to deal with with prop drilling. This is not the case here, since the ContextMenu is a direct child of MissionPlanningView. We should use props and events instead.

src/types/mission.ts

rafaellehmkuhl · 2024-12-10T16:36:54Z

src/types/mission.ts

+  polygonPositions: L.LatLng[]
+  /**
+   * The markers that represent the polygon vertices.
+   */
+  polygonMarkers: L.Marker[]
+  /**
+   * The markers that represent the polygon edges.
+   */
+  edgeMarkers: L.Marker[]


We shouldn't use Leaflet types here. We should use those from an agnostic library like geojson (already in the project) or, in case there's nothing there for us, create our own. This ensures the project does not get too tied to the choice of library for the map backend (which is already in our plans to change soon).

I agree with you and the lifetime for this lib is very short on Cockpit.
But since geojson has no compatible types for L.LatLang and the L.Marker type is too complex to mock with a custom type, my suggestion is that we leave the interface as it is until the time to replace the lib comes.

For the LatLng, can't we make our own? Seem like a simple type.

For the markers, I think my question turns to another: do we need to have the markers in the Survey object? Can't we generate the markers at runtime based on the positions? Seems like we are duplicating information.

@ArturoManzoli pinging on this one

For the LatLng, can't we make our own? Seem like a simple type.

For the markers, I think my question turns to another: do we need to have the markers in the Survey object? Can't we generate the markers at runtime based on the positions? Seems like we are duplicating information.

Yes, I think we can. I'll check that 👍

For the LatLng, can't we make our own? Seem like a simple type.

For the markers, I think my question turns to another: do we need to have the markers in the Survey object? Can't we generate the markers at runtime based on the positions? Seems like we are duplicating information.

Yes, it was actually possible to do that with a few adjustments on the store/restore methods 😀

Nice! The code looks much better now. Will take a good look during the afternoon so we can merge it on monday.

src/views/MissionPlanning/ContextMenu.vue

src/components/mission-planning/ContextMenu.vue

rafaellehmkuhl · 2024-12-11T13:56:50Z

src/stores/mission.ts

@@ -30,6 +31,28 @@ export const useMissionStore = defineStore('mission', () => {

  const currentPlanningWaypoints = reactive<Waypoint[]>([])

+  const surveys = reactive<Survey[]>([])
+
+  const paths = reactive<Path[]>([])


Isn't a path just an array of lat-longs? If so, can we use a GeoJSON type and not import Leaflet here?

src/types/mission.ts

src/components/mission-planning/ContextMenu.vue

Signed-off-by: Arturo Manzoli <[email protected]>

ArturoManzoli requested a review from rafaellehmkuhl November 26, 2024 20:01

ArturoManzoli force-pushed the 1350-allow-change-angle-of-attack-on-survey branch 2 times, most recently from c63d7c4 to 59d7919 Compare November 26, 2024 20:07

ArturoManzoli force-pushed the 1350-allow-change-angle-of-attack-on-survey branch 2 times, most recently from 700af16 to 1616067 Compare November 28, 2024 19:57

rafaellehmkuhl requested changes Dec 4, 2024

View reviewed changes

ArturoManzoli force-pushed the 1350-allow-change-angle-of-attack-on-survey branch from 1616067 to 9f76bcd Compare December 9, 2024 11:18

ArturoManzoli requested a review from rafaellehmkuhl December 9, 2024 11:20

ArturoManzoli force-pushed the 1350-allow-change-angle-of-attack-on-survey branch from 9f76bcd to 603180e Compare December 9, 2024 12:57

ArturoManzoli requested review from rafaellehmkuhl and removed request for rafaellehmkuhl December 9, 2024 12:59

rafaellehmkuhl requested changes Dec 10, 2024

View reviewed changes

ArturoManzoli force-pushed the 1350-allow-change-angle-of-attack-on-survey branch 3 times, most recently from f7861ca to 9d18306 Compare December 10, 2024 22:45

ArturoManzoli requested a review from rafaellehmkuhl December 10, 2024 22:51

rafaellehmkuhl requested changes Dec 11, 2024

View reviewed changes

ArturoManzoli force-pushed the 1350-allow-change-angle-of-attack-on-survey branch from 9d18306 to e8c7b59 Compare December 11, 2024 20:47

ArturoManzoli requested a review from rafaellehmkuhl December 11, 2024 20:48

rafaellehmkuhl requested changes Dec 11, 2024

View reviewed changes

src/components/mission-planning/ContextMenu.vue Outdated Show resolved Hide resolved

ArturoManzoli force-pushed the 1350-allow-change-angle-of-attack-on-survey branch from e8c7b59 to 5ae6f43 Compare December 12, 2024 09:31

ArturoManzoli requested a review from rafaellehmkuhl December 12, 2024 09:32

Views: Mission-planning: Add context menu and mission editing

6eb3ecf

Signed-off-by: Arturo Manzoli <[email protected]>

ArturoManzoli force-pushed the 1350-allow-change-angle-of-attack-on-survey branch from 5ae6f43 to 6eb3ecf Compare December 13, 2024 17:30

ArturoManzoli requested review from rafaellehmkuhl and removed request for rafaellehmkuhl December 13, 2024 17:32

rafaellehmkuhl mentioned this pull request Dec 13, 2024

Allow user to specify the angle of attack for a survey mission #1350

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mission Planning improvements #1469

Mission Planning improvements #1469

ArturoManzoli commented Nov 26, 2024 •

edited

Loading

rafaellehmkuhl commented Nov 26, 2024

rafaellehmkuhl left a comment •

edited

Loading

julled commented Dec 8, 2024

ArturoManzoli commented Dec 9, 2024 •

edited

Loading

rafaellehmkuhl commented Dec 9, 2024

rafaellehmkuhl left a comment •

edited

Loading

rafaellehmkuhl Dec 10, 2024

ArturoManzoli Dec 10, 2024

rafaellehmkuhl Dec 11, 2024

rafaellehmkuhl Dec 11, 2024

ArturoManzoli Dec 13, 2024

ArturoManzoli Dec 13, 2024

rafaellehmkuhl Dec 13, 2024

rafaellehmkuhl Dec 11, 2024

Mission Planning improvements #1469

Are you sure you want to change the base?

Mission Planning improvements #1469

Conversation

ArturoManzoli commented Nov 26, 2024 • edited Loading

rafaellehmkuhl commented Nov 26, 2024

rafaellehmkuhl left a comment • edited Loading

Choose a reason for hiding this comment

julled commented Dec 8, 2024

ArturoManzoli commented Dec 9, 2024 • edited Loading

rafaellehmkuhl commented Dec 9, 2024

rafaellehmkuhl left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ArturoManzoli commented Nov 26, 2024 •

edited

Loading

rafaellehmkuhl left a comment •

edited

Loading

ArturoManzoli commented Dec 9, 2024 •

edited

Loading

rafaellehmkuhl left a comment •

edited

Loading