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

mission: add delay before camera action #131

Merged
merged 3 commits into from
Nov 14, 2017
Merged

Conversation

julianoes
Copy link
Collaborator

This adds the possibility to add a delay before taking a picture or
starting a video. This is used to wait for a gimbal to reach its
orientation, or a vehicle to slow down.

The current implementation is not optimal because it is based on the
LOITER_TIME waypoint instead of the NAV_DELAY command which would be
more appropriate but is not supported (yet).

@coveralls
Copy link

Coverage Status

Coverage remained the same at 48.634% when pulling 52c0103 on add-mission-delay into e12c969 on develop.

if (!last_position_valid) {
// In the case where we get a delay without a previous position, we will have to
// ignore it.
LogErr() << "Can't set camera actoin delay without previous position set.";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo "actoin"

@hamishwillee
Copy link
Collaborator

LGTM.
Generally I hate delays as mechanisms for getting things ready - they either have to be tuned for particular use cases or made conservatively over-long. Is it possible/reasonable to instead provide checks that speed is correct and gimbal is in position (or don't they provide that feedback)?

@julianoes
Copy link
Collaborator Author

Generally I hate delays as mechanisms for getting things ready - they either have to be tuned for particular use cases or made conservatively over-long. Is it possible/reasonable to instead provide checks that speed is correct and gimbal is in position (or don't they provide that feedback)?

You're right. The problem is that we just send a command to the gimbal what it's setpoint is but we really don't know how long it will take it. Even if the gimbal would only acknowledge the command when it has reached the setpoint, the firmware might not even wait for a command to be completed before continuing its mission. What I'm saying is that this would require a bunch of changes to avoid a delay like this.

@hamishwillee
Copy link
Collaborator

Fair enough. I guess also that even if one camera gimbal provided position feedback, that wouldn't be true of all of them.
@mrpollo Want to review this?

Copy link
Member

@mrpollo mrpollo left a comment

Choose a reason for hiding this comment

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

👍

@mrpollo
Copy link
Member

mrpollo commented Nov 14, 2017

@julianoes please rebase and merge!

This adds the possibility to add a delay before taking a picture or
starting a video. This is used to wait for a gimbal to reach its
orientation, or a vehicle to slow down.

The current implementation is not optimal because it is based on the
`LOITER_TIME` waypoint instead of the `NAV_DELAY` command which would be
more appropriate but is not supported (yet).
After rebasing, support to download mission items was required and
therefore added here.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 17.476% when pulling ae8dede on add-mission-delay into d9bd982 on develop.

@julianoes julianoes merged commit 9c6e936 into develop Nov 14, 2017
@julianoes julianoes deleted the add-mission-delay branch November 14, 2017 21:03
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.

4 participants