mavlink / qgroundcontrol

Cross-platform ground control station for drones (Android, iOS, Mac OS, Linux, Windows)
http://qgroundcontrol.io
3.19k stars 3.53k forks source link

PlanView Bugs and Improvements #7765

Open tilaktilak opened 5 years ago

tilaktilak commented 5 years ago

I am working on some issues on PlanView at the moment, I create this issue to keep a track on this and discuss how to address it.

Here what I found from the Daily version of yesterday :

Observations

1. Terrain Altitude of Planned Home is not determined properly

At first it's not determined, but works if we move it. Here the error shown :

TerrainQueryLog: Error in fetching elevation tile. Empty response.

image

2. When I click to add a Waypoint this error shows up :

qrc:/qml/QGroundControl/Controls/ToolStrip.qml:87: ReferenceError: _root is not defined

Not sure but I think it's because the id of ToolStrip is overwritten by PlanView.qml Here and _root does not exist no more. Can someone confirm ? Doesn't look that simple

3. Spline/Land can result in WP at (0,0)

If we change the type of the Takeoff item (just after the Mission Start) to spline or land (Maybe some other type of WP), it create a WP at (0,0) image

4.Error while moving PlannedHome Point

If I create a 2 waypoints mission, the first is defined as PlannedHome, the other as WP2. This error shows while moving the PlannedHome :

DelegateModel::cancel: index out range 0 0

image

5. Mission has to start by a TakeOff Point :

When we first add a Waypoint to the item list, it add an automatic Takeoff point.
But it's not the case while we start by a Survey or any complex item. As a consequence Ardupilot does not accept the mission that does not start by a TakeOff which make perfectly sense to me btw.
Also it's possible to remove the TakeOff Point added by the first WP at any time. We should figure out something to insure a mission start by a Takeoff or stick this Takeoff to the PlannedHome.

6. Layer shown while editing Mission/Fence/Rally

When we edit the Fence or Rally point the Mission Layer is hidden. It's useful to show all layer not depending on what we are editing (Fence, Rally or Mission). Other layers that are not editable at the moment could be put in gray.

7. ToolStrip File

I am asking myself if it's consistent to name this 'File' as it give access to tools like "Clear Vehicle Mission, Upload, Download"and not so File-specific tools. image

8. PlanToolbar and Duplicate PaperPlane Button

This button to go back to FlyView is duplicate and close one to another: image Also, why do we change to Toolbar to PlanToolbar ? I think there is enough place to shows this : image on the main toolbar.

Open Points

1.Type of Waypoints

It's not clear to me that some WP type like TakeOff or ReturnToHome are not Item defined by coordinates as adding them will result in "tagging" the previous Waypoint in the UI. The first thing that I think of here is that it has to be added in another manner than clicking on the map to add a WP and change the type which could be confusing as the last WP will disappear.

It might be a way to shows through the UI that this item will appear on the map and this one will not. It could be some statement like : "We cannot change the type of an item created by the ToolStrip" and "All items created by the ToolStrip appears on the map" and to create other type of item we need to insert a new element on the list and define its type.

2. Planned Home

I understand the convenience of adding the Planned home automatically to the first WP. Therefore I think we may need 2 more features :

I am working on adding a "Home" button to the ToolStrip to create the PlannedHome This is incomplete at the moment but the idea is to force the first item to be PlannedHome (by hiding/disabling the other tools for instance) and maybe add a non-removable TakeOff item at the same time. Also my plan is to hide/disable this Home tool straight after PlannedHome being set.

Peek 2019-09-05 14-31

3. Nice to Have

Could be cool to delete the selected item by pressing the "Delete" key on the keyboard

@edwinhayes

edwinhayes commented 5 years ago

1.Type of Waypoints

It's not clear to me that some WP type like TakeOff or ReturnToHome are not Item defined by coordinates as adding them will result in "tagging" the previous Waypoint. The first thing that I think of here is that it could be a "property" of a WP and added in another manner than clicking on the map which could be confusing as the last WP disappear when we select TakeOff for instance.

One thing about QGC that I don't particularly like is that the list of things on the RHS does not have a 1:1 relationship with the list of actual mission items. Setting gimbal angle is treated by the UI as a property of a waypoint, but that's not true at all, it's actually a separate mission item. This behaviour seems to me like it could easily result in a bunch of weird corner cases when restarting a mission or jumping to a particular waypoint number.

edwinhayes commented 5 years ago

Could be cool to delete the selected item by pressing the "Delete" key on the keyboard

In general, QGC is rather focussed on tablet operation; I think we might need to have a larger discussion at some point about changes to improve the user experience when operating from a desktop with mouse and keyboard, such as keyboard shortcuts, context menus etc.

edwinhayes commented 5 years ago
  1. Planned Home

Since this is one of the areas where there is significant incompatibility between AP and PX4, need to consider what is actually supported by the flight controller.

DonLakeFlyer commented 5 years ago

TerrainQueryLog: Error in fetching elevation tile. Empty response.

In general the terrain query is very flaky. We plan to move to a new terrain backend which will fix on this. This will be happening soon.

qrc:/qml/QGroundControl/Controls/ToolStrip.qml:87: ReferenceError: _root is not defined

This is a new bug which showed up with all the recent changes in master.

If we change the type of the Takeoff item (just after the Mission Start) to spline or land (Maybe some other type of WP), it create a WP at (0,0)

This is likely a fixed wing only bug. ArduPilot multi-rotor takeoff item does not support specifying a lat/lon with it, just altitude. Does fixed wing allow lat/lon?

When we first add a Waypoint to the item list, it add an automatic Takeoff point. But it's not the case while we start by a Survey or any complex item.

Bug.

When we edit the Fence or Rally point the Mission Layer is hidden. It's useful to show all layer not depending on what we are editing (Fence, Rally or Mission). Other layers that are not editable at the moment could be put in gray.

Possibly. The other layers are hidden to make ti easier to move things around on fence/rally. With the work I'm currently doing to show less detailed complex item visuals when not selected this may be even more feasible now.

It's not clear to me that some WP type like TakeOff or ReturnToHome are not Item defined by coordinates as adding them will result in "tagging" the previous Waypoint in the UI.

Because they don't include a lat/lon position in them.

I understand the convenience of adding the Planned home automatically to the first WP.

Planned Home has always been confusing. I would leave this alone for now so I can do a more ground up rethink on this.

Could be cool to delete the selected item by pressing the "Delete" key on the keyboard

Keyboard support has been request all over the place. This is just one good example. The reason keyboard is not there is that Qt keyboard support is a bit of a nightmare. I've never been able to make it work consistently without causing all sorts of other problems. I've tried an failed multiple times unfortunately.

DonLakeFlyer commented 5 years ago

In general, QGC is rather focussed on tablet operation

See my explanation above as to why QGC is lacking better keyboard support.

DonLakeFlyer commented 5 years ago

Planned home: Since this is one of the areas where there is significant incompatibility between AP and PX4, need to consider what is actually supported by the flight controller.

The concept of "Planned Home" has nothing to do with firmware. The reason for it is to provide the user with a more accurate representation of the flight path for the mission. Also without a planned home you cannot calculate things like flight duration/length/battery count.

DonLakeFlyer commented 5 years ago

I am asking myself if it's consistent to name this 'File' as it give access to tools like "Clear Vehicle Mission, Upload, Download"and not so File-specific tools.

Maybe yes, maybe no. It used to be called Sync.

DonLakeFlyer commented 5 years ago

@edwinhayes @tilaktilak How important is terrain for your usage? For example currently QGC does not support ArduPilot vehicle terrain requests where terrain maps are uploaded to the vehicle such that things like an RTL and follow terrain. If that's important a big work item which needs to be done is changing to a new terrain server and supporting vehicle terrain requests.

DonLakeFlyer commented 5 years ago

One thing about QGC that I don't particularly like is that the list of things on the RHS does not have a 1:1 relationship with the list of actual mission items. Setting gimbal angle is treated by the UI as a property of a waypoint, but that's not true at all, it's actually a separate mission item. This behaviour seems to me like it could easily result in a bunch of weird corner cases when restarting a mission or jumping to a particular waypoint number.

This is done on purpose to make things simpler. Mere mortals do not understand the concept of individual mission items. They understand the concept of make the vehicle do some work. They do understand the concept of a waypoint. The ability to jump to an individual camera command as opposed to the waypoint before it doesn't seem worth the additional complexity it brings along with it. Not to mention how a user would ever remember that item #57 changes the gimbal angle say so they need to pick item #57 from the fly view.

DonLakeFlyer commented 5 years ago

@tilaktilak Can you move your Planned Home position ideas to a separate issue? I think that is worth a full discussion.

DonLakeFlyer commented 5 years ago

Another major work item which would be nice to have someone work through is keyboard support all over the place. Is that something you would be interested in taking on?

DonLakeFlyer commented 5 years ago

Another major work item which would be nice to have someone work through is keyboard support all over the place. Is that something you would be interested in taking on?

If so, would be good to create a separate issue for discussion on that. It's been a feature request for years now with piles of folks asking for it.

DonLakeFlyer commented 5 years ago

@tilaktilak Also if you could look a the bugs which have been identified here that would be awesome.

DonLakeFlyer commented 5 years ago

@tilaktilak Can you move the Plan/Fence/Rally always shown thing into a separate issue as well. We can then continue discussion on that one there.

tilaktilak commented 5 years ago

Thanks for all of this feedback ! Let me split this in several issues. And yes I'm on most of this, I may need some inputs on some points as well.

edwinhayes commented 5 years ago

Oh haha, I didn't notice this was a ticket on the mainline, rather than our repo, when I was commenting. Ahhh, so whenever I say "we", just assume I mostly mean @tilaktilak !

How important is terrain for your usage? For example currently QGC does not support ArduPilot vehicle terrain requests

It's super important; some of our larger surveying customers depend on the ArduPilot terrain following behaviour, so this is one key item in the list of things we need to have done before we can migrate from Mission Planner to QGC.

Another major work item which would be nice to have someone work through is keyboard support all over the place. Is that something you would be interested in taking on?

Yep, it should be something we can look at, but probably not in the current tranche of development; further, like much other stuff we'll do, it'll probably be designed targeting power users rather than "mere mortals", and only targeting the platforms we care about.

Mere mortals do not understand the concept of individual mission items.

Perhaps, but while this abstraction might make things simpler for "mere mortals", it makes the user experience pretty weird for power users. Anyway, it's not like we're going to attempt to change this, was mostly just background for @tilaktilak.

DonLakeFlyer commented 5 years ago

Perhaps, but while this abstraction might make things simpler for "mere mortals", it makes the user experience pretty weird for power users.

I don't necessarily disagree with that. Especially if you are coming from Mission Planner. The really important thing is whether the QGC ui really is detrimental to solving important usage scenarios for a Plan. I tend to think more in terms of people coming from DJI (or say like DroneDeploy) to QGC than MP to QGC as being important. QGC just isn't targetted to making MP power users happy. MP already does a fantastic job of that and it the gold standard for that user group.

All of that said you still need to be able to get useful things done. So feedback on usability problems is critical. But It's important to "live" with using QGC for a bit before generating a solid opinion. Otherwise folks tends to just want to make QGC look like the thing they are used to. But after they live with it for awhile they may changes their mind on whether something is really a problem or not. And/or truly determine the QGC is crap for what they want to get done. Hence change may be needed.

DonLakeFlyer commented 5 years ago

Also here is a write up which provides details on some of the QGC ui principles: https://dev.qgroundcontrol.com/en/ui_design/multi_device_pattern.html

DonLakeFlyer commented 5 years ago

@tilaktilak If you are going to work on terrain load/follow can you creata a new issue for that as well?

hamishwillee commented 5 years ago

I am asking myself if it's consistent to name this 'File' as it give access to tools like "Clear Vehicle Mission, Upload, Download"and not so File-specific tools.

Maybe yes, maybe no. It used to be called Sync.

As noted, this contains both File and Sync tools. There isn't any one word that captures this. If there is room you might consider File/Sync. Otherwise "Manage"

AndKe commented 2 years ago

Also: Spline waypoints are not visualized, the lines between spline-wp are not curved.