Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 service - add section on mission execution #529
base: master
Are you sure you want to change the base?
Mission service - add section on mission execution #529
Changes from 4 commits
5b7fb1d
0dafb8b
b3cedb1
8de8ed0
48126d8
7dd38c1
89ef2e2
39df0b2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it very strange that there's no coverage of
MAV_CMD_DO_PAUSE_CONTINUE
here! That's the more appropriate malvink-ish way of doing this behaviour, I'd think.Saying "do-set-mode" won't always stop the vehicle from executing waypoints - for example, ArduPlane when you've got a
DO_LAND_START
. If you switch modes to RTL it's essentially the same thing as changing waypoints - the Plane will jump to its landing sequence.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doh. That's my brain fade. Thanks. Will fix.
@peterbarker This is true on PX4 too but there is a nuance that waypoints are being executed but you're still in Return mode, not mission mode. Is it the same on ArduPlane?
The TLDR here is that PX4 RTL has settings to execute a return mode landing following a mission landing pattern. So you are following waypoints, but you're not in a mission.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe so. Unless behaviour has changed the vehicle will briefly appear to go into RTL mode but will then appear to go back to AUTO mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peterbarker What if you're not in auto mode to start with - will it go to RTL mode and still then go to AUTO mode to do the landing? That seems odd if the intent was to RTL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In any case, fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought so. What did you think - only on reboot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On reboot for sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And on re-upload, or manual reset to 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ArduPlane will not reset counters if you land the vehicle in auto. It's a bit more complicated because some missions have the vehicle land and disarm, but then be able to resume the mission from where they are in their sequence when they're armed in auto again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@julianoes Just to be clear,
What do you mean by this? Is this the same as ArduPilot - setting mission current to 0 resets the whole mission? If so, that would be good.
@peterbarker Thanks very much. This caveat is true of all PX4 vehicle too - its a configuration setting or a mission type setting. I will add a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ground station dependent. I think QGC does that. But if you just upload the whole mission, it will go back to 0.
I think so, except things like DO_JUMP counters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@julianoes
So it will remove the "complete" bit and restart the mission, but with the wrong counters? That's screwed up. Can we make it also reset the counters? (i.e. as part of this protocol) and make not resetting them non-standard.
FYI also @peterbarker my concern here is that we updated MAV_CMD_DO_SET_MISSION_CURRENT to add a reset option because there was not way to reset the counters. Now it seems like setting the mission item to 0 would do this. That's a better approach. So we should standardize.
I.e. I think we'd be best off removing that separate parameter and stating that setting to zero should reset the counters and the complete bit and reset to zero. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the way I made it in 2013 or so. It might have been fixed since. You could try it out...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt it. In any case, Peter commented somewhere here he prefers that counters are reset explicitly. So we probably need to implement that second parameter. I looked at it once but couldn't work out the right place to even add the command.