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

Update docs to new plugin archicture #88

Merged
merged 9 commits into from
Feb 7, 2018
Merged

Update docs to new plugin archicture #88

merged 9 commits into from
Feb 7, 2018

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Feb 5, 2018

This PR updates docs to match up with mavlink/MAVSDK#188

Examples and most of guide done (though there will be further comments inline). This also includes docs from mavlink/MAVSDK#242 (not yet merged). Writing plugins and extensions not yet done. Some outstanding questions/actions

  1. DevicePluginContainer removed. Need to check no longer referenced.
  2. Confirm whether PluginBase, and MavlinkHandlerTableEntry part of the public API? If so need docs for those.
  3. Device needs docs: Device API needs documentation MAVSDK#243

@julianoes Even though this is not "complete", provided it is accurate as is, it might be worth accepting and then doing how to write plugins etc as separate PRs.

@@ -24,7 +24,7 @@ The architecture has stubs for serial, TCP, and UDP connections. However, only U

### Why is libCURL a dependency?

libCURL will be required to download the camera definition file which is referenced in [CAMERA_INFORMATION](http://mavlink.org/messages/common#CAMERA_INFORMATION). It might also come in handy whenever any other REST resources are required.
libCURL will be required to download the camera definition file which is referenced in [CAMERA_INFORMATION](http://mavlink.org/messages/common#CAMERA_INFORMATION). It might also come in handy whenever any other REST resources are required.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@julianoes Where does DroneCore use CAMERA_INFORMATION?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hehe, that's still proprietary but I'll open source that camera plugin soon.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cool. Created #90 just to make sure this doesn't fall off the radar

@@ -64,7 +64,6 @@ The most important classes are:
- [Gimbal](/api_reference/classdronecore_1_1_gimbal.md): Control a gimbal.
- [FollowMe](/api_reference/classdronecore_1_1_follow_me.md): Drone tracks a position supplied by DroneCore.
- [Logging](/api_reference/classdronecore_1_1_logging.md): Data logging and streaming from the vehicle.
- [include/device_plugin_container.h.in](https://github.com/dronecore/DroneCore/blob/{{ book.github_branch }}/include/device_plugin_container.h.in): Auto-generated file that is required for DroneCore plugin development - see [DevicePluginContainer](/api_reference/classdronecore_1_1_device_plugin_container.md).
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Deleted the DevicePluginContainer. Need to know if need to add: PluginBase, and MavlinkHandlerTableEntry

Copy link
Contributor

Choose a reason for hiding this comment

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

Not just yet, I think.

* [Vehicle Information](guide/device_version.md)
* [Telemetry](guide/telemetry.md)
* [Taking Off and Landing](guide/taking_off_landing.md)
* [Actions - Take Off, Arm, ...](guide/taking_off_landing.md)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed to focus on "Actions" old name too specific for broad content of topic.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense.

@@ -0,0 +1,58 @@
# dronecore::PluginBase Class Reference
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@julianoes If this is public, need some docs.


The following code assumes that you already have included DroneCore (`#include <dronecore/dronecore.h>`) and that there is a [connection to a device](../guide/connections.md) obtained as shown below:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@julianoes Conscious decision here NOT to include mention of also having to link to the plugin library. Do you think this has to be mentioned? We do link to the using plugins doc that covers this sort of thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

You now need to include and link to each plugin that you require. So in this case we do require info.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had hoped that having that in the "Using Plugins" section would be sufficient (since all the sections otherwise have to duplicate similar instructions). Anyway, I've updated so that this is in every docs as "Create the Plugin" (rather than preconditions). OK?

@hamishwillee
Copy link
Collaborator Author

@julianoes What needs to change for local installation?
https://docs.dronecore.io/v/develop/en/guide/toolchain.html#dronecore_local_install

Do you think you could provide an example? I suspect it is just addition of the plugin libs to ${dronecore_lib}

@@ -64,7 +64,6 @@ The most important classes are:
- [Gimbal](/api_reference/classdronecore_1_1_gimbal.md): Control a gimbal.
- [FollowMe](/api_reference/classdronecore_1_1_follow_me.md): Drone tracks a position supplied by DroneCore.
- [Logging](/api_reference/classdronecore_1_1_logging.md): Data logging and streaming from the vehicle.
- [include/device_plugin_container.h.in](https://github.com/dronecore/DroneCore/blob/{{ book.github_branch }}/include/device_plugin_container.h.in): Auto-generated file that is required for DroneCore plugin development - see [DevicePluginContainer](/api_reference/classdronecore_1_1_device_plugin_container.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Not just yet, I think.

* [Vehicle Information](guide/device_version.md)
* [Telemetry](guide/telemetry.md)
* [Taking Off and Landing](guide/taking_off_landing.md)
* [Actions - Take Off, Arm, ...](guide/taking_off_landing.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense.

@@ -152,16 +161,18 @@ inline void offboard_log(const std::string &offb_mode, const std::string msg)
*/
bool offb_ctrl_ned(Device &device)
{
std::shared_ptr<Offboard> offboard = std::make_shared<Offboard>(&device);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is actually wrong. We should not create offboard twice but only once and then pass it on as std::shared_ptr<Offboard> offboard instead of Device &device.

I guess we need to fix the example first.

Copy link
Collaborator Author

@hamishwillee hamishwillee Feb 6, 2018

Choose a reason for hiding this comment

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

Update to example is here: mavlink/MAVSDK#246
Untested because I can't build dronecore mavlink/MAVSDK#245

I'll update docs after that example is merged.

@@ -24,7 +24,7 @@ The architecture has stubs for serial, TCP, and UDP connections. However, only U

### Why is libCURL a dependency?

libCURL will be required to download the camera definition file which is referenced in [CAMERA_INFORMATION](http://mavlink.org/messages/common#CAMERA_INFORMATION). It might also come in handy whenever any other REST resources are required.
libCURL will be required to download the camera definition file which is referenced in [CAMERA_INFORMATION](http://mavlink.org/messages/common#CAMERA_INFORMATION). It might also come in handy whenever any other REST resources are required.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hehe, that's still proprietary but I'll open source that camera plugin soon.


The following code assumes that you already have included DroneCore (`#include <dronecore/dronecore.h>`) and that there is a [connection to a device](../guide/connections.md) obtained as shown below:
Copy link
Contributor

Choose a reason for hiding this comment

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

You now need to include and link to each plugin that you require. So in this case we do require info.

@hamishwillee
Copy link
Collaborator Author

@julianoes I've updated in line with your feedback. More to do, but I'd like to get this PR in - can you check again please?

@julianoes
Copy link
Contributor

julianoes commented Feb 6, 2018

Thanks, your comments make sense. I'll look into your PR and issue.

@hamishwillee
Copy link
Collaborator Author

OK to merge this @julianoes ? There is more to be done - like updating for the Offboard example (mavlink/MAVSDK#246), and looking at the external example, but I'd like to get this in.

Copy link
Contributor

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Sure this can then go in.

@hamishwillee
Copy link
Collaborator Author

Thanks!

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