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

GCS_MAVLink: home position should always send #23636

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

magicrub
Copy link
Contributor

When a GCS requests the home position and we respond via send_home_position() we used to always send it if home was set. Now we'll ignore the request for many more seconds until home.get_vector_from_origin_NEU() returns true which is when the EKF origin is set. Is that really necessary? It's just populating another part of the home_position() message which is otherwise 0 and used to be hard-coded to 0. If a GCS really wants that info then it can re-request until it gets non-zero data there.

@rmackay9
Copy link
Contributor

I've always thought that there cannot be a home position until the EKF origin has been set so I'm not sure the current code needs to be changed.

@magicrub
Copy link
Contributor Author

magicrub commented May 2, 2023

well, it used to be once you get a GPS lock a GCS could ask you what your home location is. Now it takes a dozen seconds later and meanwhile swallows up your request without a NACK or anything

@magicrub
Copy link
Contributor Author

magicrub commented May 2, 2023

your home isn't what's changing.. it's the origin that does and it's just adding to the message. The home is valid while it's rejecting the request... and frankly a GCS doesn't care about the EKF origin in most cases

@magicrub
Copy link
Contributor Author

ping

@magicrub
Copy link
Contributor Author

This likely deserves a discussion at a dev call

@magicrub
Copy link
Contributor Author

magicrub commented May 22, 2023

PR that caused this new behavior: #21642
MAVLink msg: https://mavlink.io/en/messages/common.html#HOME_POSITION

Outcome from devcall discussion: The field should really be NaN as default instead of 0. Same goes for q[4] and we should update the MAVLink spec saying so

libraries/GCS_MAVLink/GCS_Common.cpp Outdated Show resolved Hide resolved
@magicrub
Copy link
Contributor Author

@peterbarker said he'll contact @hamishwillee regarding MAVLink changes

@hamishwillee
Copy link
Contributor

He did. Makes a lot of sense.

Regarding invalid values, by convention we use zero "if 0 is invalid in the context of the field" - I am assuming all zeros is an invalid quaternion.
Otherwise we use a max field value for ints/uints and NaN for floats.

Consider also making the Lat/Lon/Alt as NaN invalid too, if it is possible the message will be requested when you have absolute but no local position. Also it is possible this might be streamed at low rate in some flight stacks, when there is no home position, or only some data is available.

Lastly, if you get a request and you have no home position, NACK the request with MAV_RESULT_TEMPORARILY_REJECTED and don't send the message.

Can you PR the changes upstream please.

@hamishwillee
Copy link
Contributor

Actually there is already a PR to specify quaternions as NaN - @auturgy approved it, and I have merged it mavlink/mavlink#1843

@magicrub
Copy link
Contributor Author

I've fixed this per @tridge 's comments and did it in two commits so we can drop the Home one if necessary.

libraries/GCS_MAVLink/GCS_Common.cpp Outdated Show resolved Hide resolved
libraries/GCS_MAVLink/GCS_Common.cpp Outdated Show resolved Hide resolved
libraries/GCS_MAVLink/GCS_Common.cpp Outdated Show resolved Hide resolved
@magicrub magicrub merged commit 732cd31 into ArduPilot:master Aug 23, 2023
81 checks passed
@magicrub magicrub deleted the pr/gcs_check_home2 branch August 23, 2023 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants