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

Fix issue with MSP Baro (Box enum only) #8083

Merged
merged 2 commits into from
Jun 7, 2022

Conversation

JulioCesarMatias
Copy link
Collaborator

That solve #8080

I spent some time trying to understand how the new format of the iNav Configurator works... Now just update Settings.md and the settings are automatically changed in the Configurator. That was a very smart feat!

@robustini
Copy link

Fixed, thanks!
Please merge it.

@MATEKSYS
Copy link
Contributor

MATEKSYS commented Jun 6, 2022

image

seems this bug is still there, same with INAV4.1
Baro readout can't jump back after stop blowing

@MATEKSYS
Copy link
Contributor

MATEKSYS commented Jun 6, 2022

image

@robustini
Copy link

Baro readout can't jump back after stop blowing

Very true, I hope that this too will be fixed because having an useless barometer is like not having one.

@JulioCesarMatias JulioCesarMatias changed the title Fix issue with MSP Baro Fix issue with MSP Baro (Box enum only) Jun 6, 2022
@JulioCesarMatias
Copy link
Collaborator Author

JulioCesarMatias commented Jun 6, 2022

image

seems this bug is still there, same with INAV4.1 Baro readout can't jump back after stop blowing

Does this happen with other versions? For example, iNav 3.0.

Why looking at the commits in gps_msp.c nothing was done after Konstantin... Changes only occurred in fc_mps.c, maybe that's where the algorithm is broken (I think, not sure if I'm right)

Or could that line be the problem?

if ((millis() - mspBaroLastUpdateMs) > MSP_BARO_TIMEOUT_MS) {

maybe just checking if the previous update time is the same as the current one might be better than checking if the sensor responded within 250ms. It's just a hypothesis, not sure about it...

@MATEKSYS
Copy link
Contributor

MATEKSYS commented Jun 7, 2022

image

image

Tried INAV3.0.2, and 2.6.1. seems that this bug has always been there. just we didn't notice it happens with the negative height before.

@robustini
Copy link

image

image

Tried INAV3.0.2, and 2.6.1. seems that this bug has always been there. just we didn't notice it happens with the negative height before.

Please explain this kind of wrong behavior well, I'm trying the bench and it doesn't seem to work badly.

@DzikuVx DzikuVx added this to the 5.0 milestone Jun 7, 2022
@DzikuVx DzikuVx changed the base branch from master to release_5.0.0 June 7, 2022 10:55
@DzikuVx
Copy link
Member

DzikuVx commented Jun 7, 2022

OK, the detection of MSP baro will merge to 5.0
@MATEKSYS could you please make a separate ticket for the MSP baro readouts being wrong?

@DzikuVx DzikuVx merged commit 20c2b4a into iNavFlight:release_5.0.0 Jun 7, 2022
@JulioCesarMatias JulioCesarMatias deleted the FixBaroMSP branch June 7, 2022 15:37
@robustini
Copy link

@MATEKSYS did you create another issue for the barometer?
Yesterday in flight I noticed that the baro height was completely wrong, and the problem is that iNav certainly gives priority to the barometer in the automatisms, ignoring the height detected by the GPS.
Yesterday during a failsafe the quad shot into orbit, well beyond the height set, perhaps due to a problem with the barometer.
Unfortunately I think that iNav does not have a sanity check on the barometric data during flight.
Although the height given by the GPS may not be very accurate, iNav if there is a big gap (for example over 10 meters) between the barometer altitude and the GPS should ignore the atmospheric pressure data and use the GPS.
In my quad RTH it was set to 30 meters, during the failsafe it went over 60 meters and did not stop, luckily I recovered the link and exiting the failsafees I brought it back to the ground.

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 this pull request may close these issues.

4 participants