DroidPlanner / Tower

Ground Control Station for Android Devices
https://play.google.com/store/apps/details?id=org.droidplanner.android
Other
622 stars 554 forks source link

"Change Speed" waypoints confusing to work with #1306

Open kellyschrock opened 9 years ago

kellyschrock commented 9 years ago

I've been playing with the Change Speed waypoints, and I have to say I find it all somewhat confusing. The "DO_CHANGE_SPEED" command is not a waypoint, it's a command. So if I set a waypoint and then change it to a "Change Speed" waypoint, it disappears from the map. I understand why, but I wouldn't expect everyone to. RTL and Takeoff are also commands, but since they only occur at the ends of a mission, they don't cause the same sort of confusion Change Speed and similar "in flight" commands do. As a result of it working this way, I end up setting an extra waypoint on the map where I want it to change speed, change it to a "Change Speed" waypoint type, it disappears, etc. It's just kind of weird to deal with, and I found it somewhat difficult to get the results I wanted.

It would be more intuitive to make it where a Waypoint can have an optional command (or better yet, 0 or more) associated with it. So conceptually, it's "When I get to this waypoint, take a picture" vs. "Sometime after this normal waypoint, take a picture".

Anyway, I thought I would at least float the idea.

Thalek commented 9 years ago

I confess that I have found the Case of the Missing Waypoints confusing myself. I like the idea of being able to add commands to waypoints, too. "At this waypoint, climb to this altitude and change speed." Leaving the waypoints visible (with a small visual change to indicate it has layered commands for editing purposes) has appeal. Being able to change the order of the commands would be useful, too. "At this waypoint, halt, climb to this altitude, take a picture, then change speed and head for the next waypoint at the new altitude, changing altitude to whatever it's set for at the next waypoint."

The question is, how much of this is possible and practical? I like being able to finely control each step of the mission, but can it be implemented? But even if we didn't go as grandiose as I've indicated, it would be nice to have the waypoint stay visible, but perhaps with a different icon or icon color to indicate that a command (or commands) will be executed at that location. I don't like that my flight pattern apparently changes shape just because I added a command at a waypoint.

I would recommend, also, that a delay command be added, if stacked commands become possible. This would allow you to program a halt command, wait a second, take the picture, delay a second, THEN change altitude and speed. That way, the picture would always be taken at the same altitude and relative ground speed, which it might not, if the commands were simply executed as quickly as the processor could process them.

Or am I way out in left field, talking nonsense?

kellyschrock commented 9 years ago

I don't think it's out in left field. I like the idea of being able to execute a number of commands at a specific waypoint, and it would make more sense from a user's perspective to approach it that way.

There would be a question of how to implement it in the UI so it's manageable as a user, and what changes would need to occur in the code.

Code-wise, it would be a fairly extensive change, but not a huge change. It's definitely do-able.

I would probably start by creating a new superclass for the various "real" waypoint classes (Waypoint, SplineWaypoint, etc) that extends SpatialCoordItem, and put a list of MissionCMDs there.

Then I would pare down the list of selectable waypoint types in the UI to include just the waypoints, plus RTL and Takeoff. The UI for editing a waypoint would need to include a list of commands to run for that waypoint, plus some ability to specify the relevant parameters for them. The trick here would be to prevent the UI from getting too cluttered. It would be fun to work on, if I had A) time to work on it, and B) some sense that the resulting large pull request had a reasonable chance of being accepted.

m4gr3d commented 9 years ago

The waypoint UI (waypoint detail window) can be extended with a view pager, that will partition it into waypoint properties edit window, and commands edit window. So that commands can be added to any spatial waypoint, and edited using the commands edit window on that waypoint detail screen.

Code wise, editing the SpatialCoordItem class to take a list of commands should be all that's required, so I don't think the code change will be extensive.

With these updates, the flow would be for the user to add a spatial waypoint to the map. Then access the detail window for that waypoint and add commands if needed. If we move in that direction, I think both the 'takeoff' and 'RTL' commands can be dropped from the selection window as well. 'Takeoff' can be implicitly added at the beginning of each mission, whereas RTL could be an option for the Land waypoint. On Nov 29, 2014 7:59 AM, "Kelly Schrock" notifications@github.com wrote:

I don't think it's out in left field. I like the idea of being able to execute a number of commands at a specific waypoint, and it would make more sense from a user's perspective to approach it that way.

There would be a question of how to implement it in the UI so it's manageable as a user, and what changes would need to occur in the code.

Code-wise, it would be a fairly extensive change, but not a huge change. It's definitely do-able.

I would probably start by creating a new superclass for the various "real" waypoint classes (Waypoint, SplineWaypoint, etc) that extends SpatialCoordItem, and put a list of MissionCMDs there.

Then I would pare down the list of selectable waypoint types in the UI to include just the waypoints, plus RTL and Takeoff. The UI for editing a waypoint would need to include a list of commands to run for that waypoint, plus some ability to specify the relevant parameters for them. The trick here would be to prevent the UI from getting too cluttered. It would be fun to work on, if I had A) time to work on it, and B) some sense that the resulting large pull request had a reasonable chance of being accepted.

— Reply to this email directly or view it on GitHub https://github.com/DroidPlanner/droidplanner/issues/1306#issuecomment-64956059 .

kellyschrock commented 9 years ago

I like that approach, but I would definitely want to be able to access the Takeoff command details in a mission. It's nearly always important for me to be able to set the takeoff height. How do you envision that working?

m4gr3d commented 9 years ago

I was envisioning the takeoff height to be the height of the first spatial waypoint. I can't think of use cases where having the takeoff height different from the first spatial waypoint height would be useful. On Nov 30, 2014 6:59 AM, "Kelly Schrock" notifications@github.com wrote:

I like that approach, but I would definitely want to be able to access the Takeoff command details in a mission. It's nearly always important for me to be able to set the takeoff height. How do you envision that working?

— Reply to this email directly or view it on GitHub https://github.com/DroidPlanner/droidplanner/issues/1306#issuecomment-64987776 .

kellyschrock commented 9 years ago

My flying field has trees at the launch point, and I launch from a smooth/flat spot under those trees. I set my takeoff height to 2 meters, then the first spatial waypoint (farther out in the field) at a much higher altitude. The drone takes off, climbs to 2 meters, then takes off forward and climbs to the next waypoint. The first waypoint height on my missions is always different than takeoff height. So, for me, the ability to set the takeoff height independently is critical.

One idea is to specify "Takeoff Height" as a mission-level attribute. To me, it's conceptually cleaner and more flexible than associating takeoff height with the height of the first waypoint. It also opens the possibility for other mission-level attributes (e.g. name, mission-specific navigation speed, etc) that would also be useful.

AndyLeeRobinson commented 9 years ago

Commands are inherently tied to waypoints because there is no timing information, unlike a midi sequencer or the timeline for an animation in Blender for example. Adding commands to waypoints would get complicated - for DO_SET_SPEED, when the WP is reached would you want it to begin accelerating/decelerating to said speed, or have already attained the new speed? Like a real sequencer, I would like to see timed waypoints, and then let the computer work out the speeds and accelerations required to reach them, and warn if any parameters constrain achievability. Would make choreography far easier. For the spline code, I think we need some 3D graphics programmers involved - I see the path of a drone like a camera and target path in Blender, whereas those that developed ardupilot approached it from a navigation point of view. The two are not mutually exclusive, but present challenges in unifying the software for both mindsets.

m4gr3d commented 9 years ago

Merged into #1356

m4gr3d commented 9 years ago

@kellyschrock would you be interested in providing a PR for the feature as we discussed above?

kellyschrock commented 9 years ago

I would be, just a bit tight on time right now.

On Thu, Feb 5, 2015 at 12:05 PM, Fredia Huya-Kouadio < notifications@github.com> wrote:

@kellyschrock https://github.com/kellyschrock would you be interested in providing a PR for the feature as we discussed above?

— Reply to this email directly or view it on GitHub https://github.com/DroidPlanner/droidplanner/issues/1306#issuecomment-73095219 .