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

Mavlink_receiver.cpp doesn't include signal_quality for distance_sensor messages #11133

Closed
tuloski opened this issue Jan 3, 2019 · 9 comments

Comments

@tuloski
Copy link

tuloski commented Jan 3, 2019

distance_sensor uORB message includes a signal_quality field.
In the mavlink_receiver.cpp when some mavlink messages (as DISTANCE_SENSOR #132) should be translated into uORB messages, that field is not filled, causing the message to not be accepted as valid.

It is also not very clear how to generate that field from mavlink messages, since there is not a field in the mavlink message that can be translated into a signal_quality. Just put 100 (perfect signal)? Or -1? Derive a value from covariance?

Example of missing field: https://github.com/PX4/Firmware/blob/73ed9deac507c86892afadee5bca2024b5b8da17/src/modules/mavlink/mavlink_receiver.cpp#L824-L834

@stale
Copy link

stale bot commented Jun 24, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented Jul 8, 2019

Closing as stale.

@stale stale bot closed this as completed Jul 8, 2019
@julianoes
Copy link
Contributor

Thanks for the issue, sorry for the late response. I would vote "Or -1?" given the fact that it's unknown. Are you interested in making a pull request?

@julianoes julianoes reopened this Jul 10, 2019
@stale stale bot removed the Admin: Wont fix label Jul 10, 2019
@tuloski
Copy link
Author

tuloski commented Jul 10, 2019

Yes I can make a pull request.
Probably in a few days.

@stale
Copy link

stale bot commented Oct 8, 2019

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@mcsauder
Copy link
Contributor

mcsauder commented Oct 8, 2019

UPDATED/EDITED: I just dug into this, there is a mismatch between the distance_sensor_t definition and the mavlink_distance_sensor_t definition. The signal_quality field is missing from the mavlink_msg_distance_sensor.h definition of mavlink_distance_sensor_t:

I've opened PR #13131 to address this issue and will open an RFC in the mavlink rfc repo as soon as I can to propose a future change.

@stale stale bot removed the stale label Oct 8, 2019
@tuloski
Copy link
Author

tuloski commented Oct 8, 2019

@mcsauder I don't think MAVlink should be modified. It's the autopilot that should adapt to MAVlink, unless those fields are really important for that message type.

@TSC21
Copy link
Member

TSC21 commented Oct 8, 2019

@mcsauder I don't think MAVlink should be modified. It's the autopilot that should adapt to MAVlink, unless those fields are really important for that message type.

The message fields are not modified: you can extend the message by adding new fields. I don't see any issue on adding a sensor quality field on the Mavlink message (the other three already exist).

Closing this as it was solved in #13131.

@TSC21 TSC21 closed this as completed Oct 8, 2019
@mcsauder
Copy link
Contributor

mcsauder commented Oct 8, 2019

Thanks @tuloski and @TSC21 !

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

No branches or pull requests

4 participants