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

iNav V5 - Barometer sanity check missing? #8224

Closed
robustini opened this issue Jul 13, 2022 · 10 comments
Closed

iNav V5 - Barometer sanity check missing? #8224

robustini opened this issue Jul 13, 2022 · 10 comments

Comments

@robustini
Copy link

robustini commented Jul 13, 2022

Current Behavior

Recently a fix was made for the Matek GNSS M8Q-CAN gps, the barometer was not detected.
Unfortunately this fix did not solve a problem: sometimes the barometer does not give any data in MSP, while the gps and mag data are there.
In fact in the first log it works perfectly, in the second it remains at 0.
Just then I had a digital video block, I activated the RTH which should have stopped at 30 meters high, just look at the log to understand what happened.
It was a miracle to get the racer back intact.

Steps to Reproduce

It is not easily reproducible but it is undeniable from the log that happened here.

Expected behavior

Here I involve the main developer @DzikuVx and the friend @JulioCesarMatias by asking him why there is no baro sanity check.
It would be enough to compare its data with the height detected by the gps, which will still be inaccurate but could allow the racer not to be sent into orbit during RTH if the baro fail.
I don't know if @matek has checked this issue, but it seems to me that he pointed out a possible similar problem.
I do not exclude that it may also be a gps firmware problem, if iNav has armed it means that the baro identified it.

Suggested solution(s)

Add a sanity check of the barometer in case of RTH or other automatisms that involve its use, and if it does not work, use the gps height.

Additional context

logs.zip


INAV/ZEEZF7 5.0.0 Jul 4 2022 / 08:25:36 (e3b3e2b9)
GCC-10.2.1 20201103 (release)

@JulioCesarMatias
Copy link
Collaborator

Hi @robustini, the Pull Request opened by Konstantin solves your problem. The resolution of this problem is already in Branch Master, you might want to do some flying and see if the problem really disappeared. And the idea of ​​adding a function to check if baro is sending acceptable readings that are meant to be used in autopilot modes is really good, I can try to develop this in the future... Thanks for your contributions, and again, I'm really a fan of your work at ArduPilot, I've learned a lot from your videos.

@robustini
Copy link
Author

robustini commented Jul 14, 2022

Thanks @JulioCesarMatias!
Ardupilot is certainly a much more complex software, and over time we have focused a lot on sanity check to always aim for safety.
RTH is a double-edged sword if a sensor malfunctions.
Better to crash the UAS right away than to send it into orbit.
I was very lucky yesterday, but it could have been worse.
I had a triple failure, the digital vtx which I still don't understand why it stopped transmitting, the barometer at 0 which fired the long racer into orbit and ELRS which did not give me a good signal.
In this triple bad luck only one thing helped me: the nickel plates of the Li-Ion pack melted while the racer had reached perhaps almost 200 meters in height.
This shut off the electronics, the racer with the very high center of gravity flew upside down and crashed inverted level on the sorghum.
If the battery contact had not opened with the remaining battery (full charged) and the climbing speed, I think it could have exceeded the calculations I made 3000 meters high: this must not happen.
So iNav should be able to understand that this is happening, since the gps still returns the height even if it is not as accurate as the barometer, he should make a choice.

immagine
immagine

@robustini
Copy link
Author

robustini commented Jul 14, 2022

Hi @robustini, the Pull Request opened by Konstantin solves your problem. The resolution of this problem is already in Branch Master, you might want to do some flying and see if the problem really disappeared. And the idea of ​​adding a function to check if baro is sending acceptable readings that are meant to be used in autopilot modes is really good, I can try to develop this in the future... Thanks for your contributions, and again, I'm really a fan of your work at ArduPilot, I've learned a lot from your videos.

@JulioCesarMatias in all honesty I don't understand how the baro_median_filter, which I had definitely ON, could have cut out the reading in MSP of the barometer only, leaving the data at 0.
iNav it does not arm if he doesn't see the barometer (I think), so how could he always read it at 0 because of the median filter?
The baro's sanity check does not mean that the baro is working, but it may still be malfunctioning.
So I'm curious to see what you have in mind to develop, but surely the gps height should be contemplated.

@robustini
Copy link
Author

robustini commented Jul 14, 2022

This log shows a flight of my tailsitter with Ardupilot.
The GPS is an M8N, so in line with what I use on the Matek iNav.
As you can see, the discrepancy between barometric height and gps height is really irrelevant, considering that the flight was made where the elevation of the ground is about -7 meters ASL.
So it's really reliable if there is a good fix, obviously for the barometer sanity check.

immagine

@JulioCesarMatias
Copy link
Collaborator

Baro sanity check is only done by looking if it is present on the I2C or UART (MSP). If it is detected, and it is returning bad data, Alt-hold and Pos-Hold modes may still be active, leading to crashes. Between the I2C or MSP there must be a function to check the health of the Baro, if the data is good, ok, use Baro... The iNav has the verification of the estimate of Position Z, the maximum error value accepted is configurable through of the inav_max_eph_epv parameter, but I can't tell if that works well (why it doesn't seem to, at least when the baro reading is always 0). To be honest, I still don't have a mind to develop...

@JulioCesarMatias
Copy link
Collaborator

This log shows a flight of my tailsitter with Ardupilot. The GPS is an M8N, so in line with what I use on the Matek iNav. As you can see, the discrepancy between barometric height and gps height is really irrelevant, considering that the flight was made where the elevation of the ground is about -7 meters ASL. So it's really reliable if there is a good fix, obviously for the barometer sanity check.

immagine

This ArduPilot graphic is very beautiful! Maybe one day we won't have him on iNav...

@JulioCesarMatias
Copy link
Collaborator

This log shows a flight of my tailsitter with Ardupilot. The GPS is an M8N, so in line with what I use on the Matek iNav. As you can see, the discrepancy between barometric height and gps height is really irrelevant, considering that the flight was made where the elevation of the ground is about -7 meters ASL. So it's really reliable if there is a good fix, obviously for the barometer sanity check.

immagine

Do you think we should have a parameter to compensate for the elevation of the field, and apply it to the baro calculation?

@robustini
Copy link
Author

robustini commented Jul 15, 2022

Do you think we should have a parameter to compensate for the elevation of the field, and apply it to the baro calculation?

I looked at the inav_max_eph_epv parameter, i.e. estimated position error thresholds but I don't understand if it is intended in XY or even in Z.
However even if contemplating the height in that case didn't work, he should have completely ignored the barometer during that RTH.
I don't think that the compensation of the elevation is fundamental, it would be enough to consider that at the arming the barometer is contemplated at 0 meters, and for example in my case the gps should detect a height of -7 meters ASL.
So -7 is the fixed gap between barometer and gps alt when arming.
If during an automatism that contemplates the barometer that -7 goes beyond a value of 10 meters of shift the barometer should be ignored and use the gps height, and it would be enough to check this even once every second.
So to give an example if the barometric altitude is 30 meters and the gps 20 is fine, if the gps returns 19 or 41 (also with a good HDOP), the automatism that excludes the barometer enters and use the gps altitude.
In Ardupilot there is a similar control, just look at the code, and also for the airspeed.
If there is an excessive discrepancy between airspeed and gps speed Ardupilot ignores the airspeed and uses the gps as speed estimate via EKF, this sanity check comes into action after a few seconds in the case of airspeed.
iNav it is certainly much less complex as code, but the ABC that aims at imho security should be there.

@robustini
Copy link
Author

robustini commented Jul 15, 2022

Obviously, a copter that performs an RTH only with the gps data will never be accurate in altitude, but at least it will bring the drone to the home and not in orbit like a rocket, a bit like Betaflight does if you only use the gps, without mag and barometer.
I don't know if iNav contemplates an RTH only with gps, without mag and barometer like Betaflight, but it should definitely be able to do it.
I looked at the log well and it seems to me that at the arming barometer worked, in the first seconds of the flight it seems to me that it reads the height well, shortly after it goes to 0 and does not move anymore.
What would Ardupilot do in that case? He would return a "baro error" immediately, on the SITL it is verifiable.
In that way the pilot immediately decides whether to continue the flight or land.
Now my question is: but iNav detects the gps height or not?

@breadoven
Copy link
Collaborator

I've always thought it makes sense to have some sort of basic sanity check for Baro based altitude position estimations using GPS altitude. GPS altitude may not be so accurate but with a decent lock it's usually within 10 - 20m which is fine for sanity checking to prevent moon shots on multirotors.

Fixed wing does have input from the GPS in this regard, inav_use_gps_no_baro.

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

Successfully merging a pull request may close this issue.

4 participants