Added gimbal plugin #124

julianoes · 2017-10-20T19:01:30Z

This adds a plugin which can set pitch and yaw angle of a gimbal.
Currently, the commands are sent to component ID 1, so the assumption is
that the gimbal is connected to the autopilot and not talking mavlink
itself. This could change or be made adaptable in the future.

coveralls · 2017-10-20T19:58:32Z

Coverage remained the same at 48.37% when pulling 6d77586 on add-gimbal into ce714f5 on develop.

hamishwillee · 2017-10-22T23:25:36Z

plugins/gimbal/gimbal.h

+        UNKNOWN
+    };
+
+    static const char *result_str(Result result);


No docs on any of this.

hamishwillee · 2017-10-22T23:26:20Z

plugins/gimbal/gimbal.h

+    static const char *result_str(Result result);
+
+    typedef std::function<void(Result)> result_callback_t;
+


Need to add docs and make clear co-ordinate frame used.

hamishwillee · 2017-10-22T23:28:27Z

plugins/gimbal/gimbal.h

+    explicit Gimbal(GimbalImpl *impl);
+    ~Gimbal();
+
+    enum class Result {


Why only this set? Can a camera not return CommandDenied, Busy or whatever?

hamishwillee · 2017-10-22T23:36:02Z

Currently, the commands are sent to component ID 1, so the assumption is that the gimbal is connected to the autopilot and not talking mavlink itself. This could change or be made adaptable in the future.

So is component ID = 1 the autopilot? - ie we're assuming that the gimbal is part of the autopilot?

Don't Yuneec use a MAVLink camera? I.e. don't we already have a case of a camera that might have a separate component ID? I'd be tempted to add a mechanism to support that now. If not, then this "limitation" should be part of the documentation.

A few other thoughts:

Is it possible for a vehicle to have multiple gimbals?
Is it possible that a gimbal will have connection semantics?
What else might we want to do other than control pitch and yaw?
- Any other axis?
- Power up/down? Retract?
What if a camera only allows control over a single axis? Errors reported?

coveralls · 2017-10-22T23:36:09Z

Coverage remained the same at 48.634% when pulling f3a65a3 on add-gimbal into bf80f02 on develop.

julianoes · 2017-10-23T13:56:54Z

So is component ID = 1 the autopilot? - ie we're assuming that the gimbal is part of the autopilot?

Don't Yuneec use a MAVLink camera? I.e. don't we already have a case of a camera that might have a separate component ID? I'd be tempted to add a mechanism to support that now. If not, then this "limitation" should be part of the documentation.

Both cases are possible. I'm not sure which one makes sense to implement, and an automatic mechanism needs a bit more work. For instance, you can check if you have received heartbeats from a gimbal component ID. If yes, you send the commands there, otherwise to the autopilot.

A few other thoughts:

- Is it possible for a vehicle to have multiple gimbals?

If you have different component IDs for the multiple gimbals, yes. Right now, only one exists MAV_COMP_ID_GIMBAL.

- Is it possible that a gimbal will have connection semantics?

It would be heartbeats being sent from the gimbal component ID, as suggested above.

- What else might we want to do other than control pitch and yaw?
    - Any other axis?

There is roll but I have never seen a drone that wants to have a roll axis different from horizontal 😄

    - Power up/down? Retract?

This is all hypothetical. We can add/implement this when there is an actual use case.

- What if a camera only allows control over a single axis? Errors reported?

No idea. I have to ignore anything that does not have a real use case right now. There are endless possibilities.

coveralls · 2017-10-23T15:13:08Z

Coverage remained the same at 48.634% when pulling 0986070 on add-gimbal into bf80f02 on develop.

coveralls · 2017-10-23T15:51:54Z

Coverage remained the same at 48.634% when pulling 0986070 on add-gimbal into bf80f02 on develop.

mrpollo · 2017-10-23T16:22:26Z

@julianoes please rebase

This adds a plugin which can set pitch and yaw angle of a gimbal. Currently, the commands are sent to component ID 1, so the assumption is that the gimbal is connected to the autopilot and not talking mavlink itself. This could change or be made adaptable in the future.

coveralls · 2017-10-23T17:58:25Z

Coverage remained the same at 48.634% when pulling 97f5fd7 on add-gimbal into 8df5d52 on develop.

hamishwillee · 2017-10-23T22:12:48Z

OK, so re that whole discussion, sure - no point implementing anything that has no real use case.

I still lean towards proper support for MAVLink camera though - on the basis that it sounds like we do have real use cases.

julianoes · 2017-10-24T14:31:50Z

Support for a mavlink camera? Absolutely! But this is just for a gimbal, and I don't know of a mavlink gimbal yet.

hamishwillee · 2017-10-24T22:37:02Z

OK, good point. I'll got back to my corner now.

Happy for this to go out. Just FMI, is it possible to get confirmation that the gimbal has reached its target attitude?

julianoes · 2017-10-25T19:43:42Z

Happy for this to go out. Just FMI, is it possible to get confirmation that the gimbal has reached its target attitude?

So if a gimbal sends MOUNT_ORIENTATION that's what you can use to track the actual angle.

hamishwillee · 2017-10-25T23:12:26Z

Just FMI, is it possible to get confirmation that the gimbal has reached its target attitude?

So if a gimbal sends MOUNT_ORIENTATION that's what you can use to track the actual angle.

Should we expose the current orientation through this API?
I guess you could argue that people know what orientation they want so they can set it and it will change if it is different, or nothing will happen if it is already on that orientation. I lean towards exposing the information, but then you end up with "nothing" if the gimbal isn't sending information. Hmm.

julianoes · 2017-10-26T12:16:49Z

@hamishwillee yes you're right, we should expose it. I'm just wondering if we should expose it in telemetry or gimbal?

julianoes · 2017-10-26T16:44:13Z

I think we can add that in telemetry sometime. I'll merge this for now.

hamishwillee · 2017-10-26T22:42:28Z

@hamishwillee yes you're right, we should expose it. I'm just wondering if we should expose it in telemetry or gimbal?

I think we can add that in telemetry sometime. I'll merge this for now.

Sounds good. I added #141 to track this. I tend to agree "Telemetry" is the right place for this, but it isn't a strong belief and you could easily convince me otherwise. Either way, if it goes in Telemetry we should add cross link/info in Gimbal docs.

Field options

julianoes requested review from mrpollo and hamishwillee October 20, 2017 19:01

hamishwillee reviewed Oct 22, 2017

View reviewed changes

plugins/gimbal/gimbal.h Outdated

UNKNOWN

};

static const char *result_str(Result result);

Copy link

Collaborator

hamishwillee Oct 22, 2017

Choose a reason for hiding this comment

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

No docs on any of this.

hamishwillee reviewed Oct 22, 2017

View reviewed changes

julianoes force-pushed the add-gimbal branch from f3a65a3 to 3ba432c Compare October 23, 2017 13:58

mrpollo approved these changes Oct 23, 2017

View reviewed changes

julianoes added 2 commits October 23, 2017 12:54

gimbal: added doxygen strings

97f5fd7

julianoes force-pushed the add-gimbal branch from 0986070 to 97f5fd7 Compare October 23, 2017 16:54

hamishwillee mentioned this pull request Oct 26, 2017

Add gimbal docs mavlink/MAVSDK-docs#48

Open

1 task

julianoes merged commit 97d98b9 into develop Oct 26, 2017

julianoes deleted the add-gimbal branch October 26, 2017 16:44

hamishwillee mentioned this pull request Oct 26, 2017

Add Gimbal telemetry #141

Closed

dlech pushed a commit to dlech/MAVSDK that referenced this pull request Jan 14, 2022

Merge pull request mavlink#124 from mavlink/default_value

29cd73c

Field options

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added gimbal plugin #124

Added gimbal plugin #124

julianoes commented Oct 20, 2017

coveralls commented Oct 20, 2017

hamishwillee Oct 22, 2017

hamishwillee Oct 22, 2017

hamishwillee Oct 22, 2017

hamishwillee commented Oct 22, 2017

coveralls commented Oct 22, 2017

julianoes commented Oct 23, 2017

coveralls commented Oct 23, 2017

coveralls commented Oct 23, 2017

mrpollo commented Oct 23, 2017

coveralls commented Oct 23, 2017

hamishwillee commented Oct 23, 2017

julianoes commented Oct 24, 2017

hamishwillee commented Oct 24, 2017

julianoes commented Oct 25, 2017

hamishwillee commented Oct 25, 2017

julianoes commented Oct 26, 2017

julianoes commented Oct 26, 2017

hamishwillee commented Oct 26, 2017

		static const char *result_str(Result result);

		typedef std::function<void(Result)> result_callback_t;

Added gimbal plugin #124

Added gimbal plugin #124

Conversation

julianoes commented Oct 20, 2017

coveralls commented Oct 20, 2017

hamishwillee Oct 22, 2017

Choose a reason for hiding this comment

hamishwillee Oct 22, 2017

Choose a reason for hiding this comment

hamishwillee Oct 22, 2017

Choose a reason for hiding this comment

hamishwillee commented Oct 22, 2017

coveralls commented Oct 22, 2017

julianoes commented Oct 23, 2017

coveralls commented Oct 23, 2017

coveralls commented Oct 23, 2017

mrpollo commented Oct 23, 2017

coveralls commented Oct 23, 2017

hamishwillee commented Oct 23, 2017

julianoes commented Oct 24, 2017

hamishwillee commented Oct 24, 2017

julianoes commented Oct 25, 2017

hamishwillee commented Oct 25, 2017

julianoes commented Oct 26, 2017

julianoes commented Oct 26, 2017

hamishwillee commented Oct 26, 2017