AntennaTracker: fortify pressure comparison logic#33120
Open
landswellsong wants to merge 1 commit into
Open
Conversation
Passing 0 to either own or target's pressure would cause an internal error if the 1976 standard atmopshere model is being used.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Protects the Tracker code against spurious intenal errors whenever either of the pressures in the pressure comparison are zero.
Classification & Testing (check all that apply and add your own)
Description
I was running into rare random errors with Tracker whenever the target UAV was powered before the tracker itself. In particular I got this:
Of all the sources for my version, this line corresponds to the following: https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_Baro/AP_Baro_atmosphere.cpp#L167
What I assume is happening, we get a pressure mavlink packet before the baro calibration / initialization of any kind is over and thus we end up supplying a zero input to
AP_Baro::get_altitude_from_pressurewhich of course leads to the internal error branch. I've added a bunch of logging to validate and indeed the error happens on zero input to this particular method. I didn't really test if it was the Tracker's own pressure or the incoming one from mavlink, since in my opinion, badly formed mavlink shouldn't result in the Tracker going into an intenal error state. If anything the error would be external in this case. The code is pretty trivial so I haven't tested it on HW yet (will report here when I do), but it shouldn't really break a thing. The fact I'm only getting this error on Tracker and only if the plane was powered first implies I'm looking at the right issue I think.I believe this issue was masked in previous releases (e.g. 4.5.x) because the atmosphere code only appeared "recently" in 4.6 so the internal error clause wasn't there before.