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

Remove need for restart after mission upload #60

Merged
merged 14 commits into from
Nov 1, 2024

Conversation

Fredrikbjornland
Copy link

@Fredrikbjornland Fredrikbjornland commented Oct 17, 2024

This PR adds multiple features to prevent us from flying with unexpected mission/rallypoint/geofence:

  • Block arming when sync is in progress
  • Block arming when mission is dirty
  • Block arming if error happened during sync
  • Display sync status of rally points and geofence
  • Display sync status in both flyview and planview
  • Remove popup asking to discard or overwrite current plan, change to always keep current plan

Also some minor fixes:

  • Clear missionfilename when downloading new mission from kyte
  • Clear missionfilename when clearing mission

@Fredrikbjornland Fredrikbjornland marked this pull request as draft October 17, 2024 07:05
@Fredrikbjornland Fredrikbjornland force-pushed the aviant/remove-need-for-restart branch 2 times, most recently from 51efdfe to 7dc7db2 Compare October 17, 2024 12:01
@Fredrikbjornland Fredrikbjornland marked this pull request as ready for review October 17, 2024 12:38
Copy link
Collaborator

@kibidev kibidev left a comment

Choose a reason for hiding this comment

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

I have not finished the review, but post some initial things now, and will continue after the weekend.

src/PlanView/PlanToolBarIndicators.qml Show resolved Hide resolved
src/ui/toolbar/MainToolBar.qml Outdated Show resolved Hide resolved
src/MissionManager/PlanMasterController.cc Outdated Show resolved Hide resolved
src/FlightDisplay/GuidedActionsController.qml Show resolved Hide resolved
@Fredrikbjornland Fredrikbjornland force-pushed the aviant/remove-need-for-restart branch 2 times, most recently from 8e083be to c873896 Compare October 23, 2024 10:48
@Fredrikbjornland Fredrikbjornland force-pushed the aviant/remove-need-for-restart branch 4 times, most recently from 0d64612 to 17837c4 Compare October 28, 2024 13:49
The filename from the previous mission would stay after downloading a new mission from kyte backend
If not, the mission name is still displayed in indicator bar after clearing missin
Avoids cannot read property coordinate and property type of unedfined when clearing delivery point
Displaying mission sync status in maintoolbar instead of PlanToolBarIndicators has two advantages.
1. We want to see the mission sync status at all times, such as in the fly view, not just when the plan view is open. This is the main reasson.
2. This makes it fill the entire window. Currently, the plantoolbar fills ~60% of the screen width, which is missleading for a loading bar
If mission, rally point or geofence upload to drone fails, we dont want to allow arming.
Arming while still syncing mission to drone can lead to unexpected mission on drone while flying.
Check for synInProgress before error in MainStatusIndicator to not display "Error" status when syncing mission plan.
Copy link
Collaborator

@kibidev kibidev left a comment

Choose a reason for hiding this comment

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

Everything looks good, but we need a design decision on the last commit before merge.

This is done as we never want the mission from the drone to overwrite the mission in plan view. Removing the popup removes one action for the pilot, which is nice.
Copy link
Collaborator

@kibidev kibidev left a comment

Choose a reason for hiding this comment

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

Looks good, given that OPS agree

@Fredrikbjornland Fredrikbjornland merged commit 99c7194 into aviant/V4.2 Nov 1, 2024
4 checks passed
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.

2 participants