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

[WIP] Update docs following addition of QGC plan import #100

Merged
merged 4 commits into from
Feb 26, 2018
Merged

[WIP] Update docs following addition of QGC plan import #100

merged 4 commits into from
Feb 26, 2018

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Feb 16, 2018

Updates docs for new QGC plan file importer (mavlink/MAVSDK#235) as requested in #99.

Actions

  • API Ref changes
  • New example
  • Update guide with new APIs. Cross link to new tests.

@hamishwillee
Copy link
Collaborator Author

hamishwillee commented Feb 16, 2018

This is still WIP because:

[qgroundcontrol_sample.plan](https://github.com/dronecore/DroneCore/blob/{{ book.github_branch }}/example/fly_qgc_mission/qgroundcontrol_sample.plan)

```

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is empty while I wait to see where the example plan file lives - mavlink/MAVSDK#272

If it stays in the plugin then I update the cpp file and delete this block.

@shakthi-prashanth-m
Copy link

@hamishwillee, do you need full screenshot of the QGC plan for higher resolution ? I did crop the image just enough to cover mission items while uploading.

Copy link

@shakthi-prashanth-m shakthi-prashanth-m left a comment

Choose a reason for hiding this comment

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

Looks great. Just added few comments IMO.

@@ -1,6 +1,6 @@
# Missions

The DroneCore Mission API allows you to create, upload, download, run, pause, restart, jump to item in, and track missions. Missions can have multiple "mission items", each which may specify a position, altitude, fly-through behaviour, camera action, gimbal position, and the speed to use when traveling to the next position.
The DroneCore Mission API allows you to create, upload, download, import from *QGroundControl*, run, pause, restart, jump to item in, and track missions. Missions can have multiple "mission items", each which may specify a position, altitude, fly-through behaviour, camera action, gimbal position, and the speed to use when traveling to the next position.

Choose a reason for hiding this comment

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

Missions can have multiple "mission items"

Shouldn't this be A mission can have multiple "mission items" ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, it is correct (what you have suggested is also fine).

* [MAV_CMD_IMAGE_STOP_CAPTURE](https://mavlink.io/en/messages/common.html#MAV_CMD_IMAGE_STOP_CAPTURE)
* [MAV_CMD_VIDEO_START_CAPTURE](https://mavlink.io/en/messages/common.html#MAV_CMD_VIDEO_START_CAPTURE)
* [MAV_CMD_VIDEO_STOP_CAPTURE](https://mavlink.io/en/messages/common.html#MAV_CMD_VIDEO_STOP_CAPTURE)
* [MAV_CMD_NAV_LOITER_TIME](https://mavlink.io/en/messages/common.html#MAV_CMD_NAV_LOITER_TIME)

Choose a reason for hiding this comment

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

Don't we need to mention first mission item in QGroundControl plan: MAV_CMD_NAV_TAKEOFF ? OR Is it implicit as part of MAV_CMD_NAV_WAYPOINT ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now this is an interesting question - and a possible bug in implementation.

If you check out MissionItem there is no API to land or takeoff! (as an aside, on PX4 if the takeoff command is omitted from mission an MC vehicle will take off without it)

That is why I have this note in the guide:

The Mission API does not (at time of writing) support "repeat", takeoff, return to land etc. DroneCore provides some omitted functionality through the Action API.

And if you look at the code we create the missionitem but do nothing for the land/takeoff handling. I think that is a bug.

Most mission created by Mission Planner will have a takeoff as their first item and land as the last item. IMO we should implement land/takeoff because they are reasonable things to want to declare, and it isn't sensible to reject import because they are missing.

@julianoes Got a better idea?

Choose a reason for hiding this comment

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

OK. We've considered takeoff and land while importing QGC mission. But however these commands are "looked" as waypoint mission items with lat, lon, alt parameters and I remember @julianoes mentioned once that mission items are sent to firmware and firmware will simply execute them as waypoint command. @julianoes, please confirm this.

However, I feel it is better to send mission items as "takeoff", "land" commands to firmware. Reason is that

  • Even after land command, vehicle stays in "Mission" & "Armed" state unless application commands for "RTL".
  • Firmware won't show "Landing detected" in PX4 logs.

Copy link
Contributor

Choose a reason for hiding this comment

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

@julianoes mentioned once that mission items are sent to firmware and firmware will simply execute them as waypoint command. @julianoes, please confirm this.

I think what I meant is that you don't need to add a takeoff waypoint because the firmware will automatically insert a takeoff waypoint, so DroneCore shouldn't worry about it.

  • Even after land command, vehicle stays in "Mission" & "Armed" state unless application commands for "RTL".

That sounds like a bug. At least if auto disarm is on it should disarm after landing.

  • Firmware won't show "Landing detected" in PX4 logs.

That would also be a bug.

However, I think adding an RTL or land command at the end of the mission is just not supported by DroneCore yet.

Choose a reason for hiding this comment

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

I tested with a QGC plan containing only takeoff and land as mission items. First I tried upload and fly using QGC. After mission accomplished, it left the vehicle in Mission Disarmed state. QGC Mission editor on the right showed command IDs: Takeoff, Land. Firmware shows Landing detected when it executes last mission item.
With DroneCore mission interface, a difference as we know is that QGC Mission editor shows both takeoff and land as Waypoint mission items. Firmware does NOT show Landing detected at the end.

Choose a reason for hiding this comment

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

However, I think adding an RTL or land command at the end of the mission is just not supported by DroneCore yet.

Yes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, so I've never though omitting takeoff and was sensible - if only because Plane/VTOL don't auto insert them like MC does, and a waypoint at "0" altitude is not a landing - it is "very low flight" :-).

Upshot, I agree the problem is that we don't support them yet, so let's do so: mavlink/MAVSDK#286
I haven't bothered to create issue for missing RTL because that seems very "optional" to me.

With respect to this particular issue, my "solution" is to add a note that these are "supported for import only": ae773b3 - I could go into more detail on what happens in detail for import, but lets just fix this going forward.

Merging this now and the current docs accurately reflect the API.


> **Note** To export a mission plan from the *QGroundControl* use the [Sync Tool](https://docs.qgroundcontrol.com/en/PlanView/PlanView.html#sync) (**Plan View > Sync Tool**, and then select **Save to File**).

The mission is imported using the static [import_qgroundcontrol_mission](../api_reference/classdronecore_1_1_mission.md#classdronecore_1_1_mission_1a7c73e97e5c1395a7451bb659d03e5f57) method. The method will fail with an error if the plan file cannot be found, cannot be parsed, or if it contains mission items that are [not supported](#supported_mission_commands).

Choose a reason for hiding this comment

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

The method will fail with an error if the plan file cannot be found, cannot be parsed, or if it contains mission items that are not supported.

The method will fail with an error if plan file cannot be found, or it cannot be parsed, or if it contains mission items that are not supported.
(Added or )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My way is correct (so is yours, but I only change things if the alternative is "better").

@hamishwillee
Copy link
Collaborator Author

@hamishwillee, do you need full screenshot of the QGC plan for higher resolution ? I did crop the image just enough to cover mission items while uploading.

No. Ideally I like "full version" with QGC shrunk as far as is reasonable and map zoomed to fill screen -
essentially then you get QGC just being a "frame" around the flight path. You can kind of get the idea from this: https://docs.dronecore.io/v/develop/en/examples/transition_vtol_fixed_wing.html

But the cropped version you provided is fine too! Certainly not worth your effort to regenerate at this point.

@hamishwillee
Copy link
Collaborator Author

OK, so now this is just waiting on discussion of how mission import of takeoff/land should be implemented (#100 (comment)) and any associated docs changes. Perhaps we should just merge this now and carry on the discussion separately?

@hamishwillee hamishwillee merged commit a45ff63 into mavlink:develop Feb 26, 2018
@hamishwillee hamishwillee deleted the mission_import branch February 26, 2018 03:38
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.

3 participants