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

Camera and gimbal improvements #635

Merged
merged 23 commits into from Nov 7, 2016

Conversation

Projects
None yet
3 participants
@julianoes
Contributor

julianoes commented Oct 11, 2016

This is an attempt to improve and extend the mavlink camera and gimbal API.

The command DIGICAM_CONFIGURE is deprecated in favor of CAMERA_INFORMATION, CAMERA_CAPTURE_STATUS, CAMERA_SETTINGS_1, and CAMERA_SETTINGS_2.

For the camera storage STORAGE_INFORMATION and STORAGE_FORMAT has been added.

To control the gimbal, the lat/lon fields have been moved to param5/param6 for int32 accuracy, and a MOUNT_STATUS is added for the realtime feedback about a gimbal (or other mount).

Please review:
@LorenzMeier, @mcharleb

Show outdated Hide outdated message_definitions/v1.0/common.xml
Show outdated Hide outdated message_definitions/v1.0/common.xml
Show outdated Hide outdated message_definitions/v1.0/common.xml
<field type="float" name="used_capacity">Used capacity in MiB</field>
<field type="float" name="available_capacity">Available capacity in MiB</field>
<field type="float" name="read_speed">Read speed in MiB/s</field>
<field type="float" name="write_speed">Write speed in MiB/s</field>

This comment has been minimized.

@vooon

vooon Oct 12, 2016

Member

Why MiB/s? For machine i think better B/s.

@vooon

vooon Oct 12, 2016

Member

Why MiB/s? For machine i think better B/s.

This comment has been minimized.

@julianoes

julianoes Oct 12, 2016

Contributor

For the machine I agree, but just for plotting in QGC by a human I'd prefer MiB/s. It's what we're used to. Also, it's a float and we don't need something terribly accurate here but just approximate.

@julianoes

julianoes Oct 12, 2016

Contributor

For the machine I agree, but just for plotting in QGC by a human I'd prefer MiB/s. It's what we're used to. Also, it's a float and we don't need something terribly accurate here but just approximate.

Show outdated Hide outdated message_definitions/v1.0/common.xml
Show outdated Hide outdated message_definitions/v1.0/common.xml
@julianoes

This comment has been minimized.

Show comment
Hide comment
@julianoes

julianoes Oct 12, 2016

Contributor

@vooon thanks a lot for the review. I answered inline.

Contributor

julianoes commented Oct 12, 2016

@vooon thanks a lot for the review. I answered inline.

@vooon

This comment has been minimized.

Show comment
Hide comment
@vooon

vooon Oct 12, 2016

Member

Now looks a bit better, but:

  1. Quaternion should be float[4] (separate q1...4 bad thing, in new messages better to use array)
  2. Command lat/long/alt should use same rule as all other commands (see NAV).
Member

vooon commented Oct 12, 2016

Now looks a bit better, but:

  1. Quaternion should be float[4] (separate q1...4 bad thing, in new messages better to use array)
  2. Command lat/long/alt should use same rule as all other commands (see NAV).
@vooon

This comment has been minimized.

Show comment
Hide comment
@vooon

vooon Oct 12, 2016

Member

Messages now looks good to me, 👍

But command enum changes need more discussion.

Member

vooon commented Oct 12, 2016

Messages now looks good to me, 👍

But command enum changes need more discussion.

@LorenzMeier

This comment has been minimized.

Show comment
Hide comment
@LorenzMeier

LorenzMeier Oct 17, 2016

Member

This PR is related: #620

Member

LorenzMeier commented Oct 17, 2016

This PR is related: #620

@julianoes

This comment has been minimized.

Show comment
Hide comment
@julianoes

julianoes Oct 18, 2016

Contributor

@LorenzMeier I don't think #620 is conflicting. It just adds some functionality to it.

Contributor

julianoes commented Oct 18, 2016

@LorenzMeier I don't think #620 is conflicting. It just adds some functionality to it.

@julianoes

This comment has been minimized.

Show comment
Hide comment
@julianoes

julianoes Oct 23, 2016

Contributor

Rebased on master again.

Contributor

julianoes commented Oct 23, 2016

Rebased on master again.

julianoes added some commits Oct 12, 2016

common.xml: prepend work in progress (WIP) flag
The proposed changes are now flagged WIP so that they can be merged but
the discussion can still be continued, so the new messages and the
changes are still subject to change.
@julianoes

This comment has been minimized.

Show comment
Hide comment
@julianoes

julianoes Nov 7, 2016

Contributor

Rebased, since there was no more comments, @LorenzMeier let's get it it.

Contributor

julianoes commented Nov 7, 2016

Rebased, since there was no more comments, @LorenzMeier let's get it it.

@LorenzMeier

This comment has been minimized.

Show comment
Hide comment
@LorenzMeier

LorenzMeier Nov 7, 2016

Member

Ok. I will keep this floating for ~3 months to allow others to suggest adjustments of the spec. @julianoes It would also be critical you provide a complete write-up in the MAVLink specs so other stakeholders can more easily comment and suggest changes they might need.

Member

LorenzMeier commented Nov 7, 2016

Ok. I will keep this floating for ~3 months to allow others to suggest adjustments of the spec. @julianoes It would also be critical you provide a complete write-up in the MAVLink specs so other stakeholders can more easily comment and suggest changes they might need.

@LorenzMeier LorenzMeier merged commit 2739464 into mavlink:master Nov 7, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@julianoes julianoes deleted the julianoes:feature_camera_api branch Nov 7, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment