-
Notifications
You must be signed in to change notification settings - Fork 16.8k
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
AP_Mission: Send jump tracking info to GCS #11120
Conversation
I see the usefulness of this and opinions on the dev team vary but I'm not a big fan of using send_text when everything is going as it should... |
Potential use case for #11108 |
I agree with @rmackay9. By the way, and in any case, @Jaaaky thank you for your first contribution! For this to go in, it would need to follow our commit message rule though (if you haven't read it, http://ardupilot.org/dev/docs/submitting-patches-back-to-master.html) - it can be done for you since you are still starting, but please take it in consideration for future PRs. |
@rmackay9 I think all sorts of mission information is currently sent as send_text() . Like; wp number, type, loiter time completed and many more others. So I think this change is consistent with the current code, until all of this is removed if another better/less bandwidth way is established. @OXINARF Thanks. I'm aware of it, but which rule I did not comply with to take care next time? |
@Jaaaky, yes, I see your point. I think we will discuss it on next Tuesday's dev call ('cuz Francisco added the DevTopicItem). Opinions in the dev team are split on the use of send-text so you may get your way :-) |
@Jaaaky yes, that's right, all commit messages need to start to with library/vehicle/folder that it belongs to. |
We discussed this on the dev call and the consensus was that this would be useful but it would be good to make the message display something like, "Jump 4 of 5". Also we'd like to get the condition covered where this path isn't run at all. |
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.
nice idea, I'd like two small changes though
First is to ensure we also log when we won't be jumping (ie. last jump)
and display should be something like
Jump to 17 (4 of 5)
I've formatted the jump message as request. Also made it consistent with other Mission items send_text() format. For the last jump, it's already there. As we send the message when it starts the jumping track not when it ends. Just test please and tell me if more changes are needed. |
c5162b1
to
520ff3d
Compare
@Jaaaky, thanks. One last change is that the commit message itself should be prefixed with "AP_Mission:". So it should become, "AP_Mission: send jump tracking info to GCS" or something similar. |
Send jump wp index and jump count to GCS to allow user to track it. Foramtted as other mission item messages
@rmackay9 Done. Thank you too for your patience. |
Merged, thanks! |
Send jump wp index and jump count to GCS to allow user to track it.