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

AP_Airspeed: Refuse to calibrate sensors if a device was not initialized #28471

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WickedShell
Copy link
Contributor

This helps catch the case where a GCS commands a calibration, and the command is reported as MAV_RESULT_ACCEPTED, even though the actual state is that at least one sensor backend failed. We are still letting the other sensors be calibrated in this situation as a GCS may not be paying proper attention to what happens with the MAV_RESULT and a user could miss the status text, so it's safer to calibrate everything we can, and then report a failure afterwards.

This was initially found with a dronecan airspeed sensor found, but not connected to the bus. The existing status text was insufficient as it was easily lost amongst other messages (prearm checks and the like). Tested on an autopilot on the desk with a I2C sensor connected, disconnected, and a disconnected dronecan sensor.

This helps catch the case where a GCS commands a calibration, and the
command is reported as MAV_RESULT_ACCEPTED, even though the actual state
is that at least one sensor backend failed. We are still letting the
other sensors be calibrated in this situation as a GCS may not be paying
proper attention to what happens with the MAV_RESULT and a user could
miss the status text, so it's safer to calibrate everything we can, and
then report a failure afterwards.
@peterbarker
Copy link
Contributor

I like the change.

I vaguely recall we couldn't do this because we default to having an MS4525 and we'd fail a calpress all the time if we did this, 'though?

@WickedShell
Copy link
Contributor Author

I'm not sure, I'm away from a board to test this with now for that sensor, I'll test it once I have hardware on hand again (or a sim). I'd argue though that in that case it's still a misconfiguration/error. But I do get that we probably historically set it as a default and now it's hard to fix.

Any thoughts on a good approach to mitigate that if it does pan out to be true? To be fair it doesn't make anything not calibrate, it just actually reports the failure back as MAV_RESULT in addition.

If we wanted to change the default off MS4525 the only path that stands out to me would be to see if that's still the default at construction, and do a probe to see if is on the bus, and if it's not found change the type to 0. That would cover most cases were vehicles are fully connected and assembled, but definitely is not going to catch all of them.

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.

2 participants