-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Double barometers proposal #9604
base: master
Are you sure you want to change the base?
Conversation
This is interesting.
With plenty of foam on the baro, you still have that problem? There is of course the concern that if BARO_1 is reading falling at 5 meters per second (from propwash), while BARO_2 is reading steady hover, averaging them still gives falling at 2.5 meters / second. Causing the aircraft to shoot upward. What has been your experience with that? |
Avaraging 2 baros is a naive strategy. The noise is usually a mounting problem and often mitigated by adding foam, as suggested by sensei. A way to detect if the baro is having issues (by comparing trends with other altitude sources, like gps and imu) would probably be prefered. If your fc can't be mounted in a way where your baro can work well, it is possible to add an external baro (different from the onboard one) with the current code, and switch to it. As a more general note, I think it would be cleaner to drop the BARO_MULTI ifdefs In this scenario, it may make sense to also refactor the baro drivers to probe multiple addresses, to allow multiple baros of the same kind to be used. |
I see this is a new contributor. Welcome! I want to let you know your work is appreciated! Depending on your background, the comments above could seem unwelcoming. They aren't meant to be, but rather to discuss the very best ways of doing things. INAV, like some other projects such as Moodle and the Linux kernel, traditionally discuss multiple viewpoints, and new features often evolve before they are added to a release. It's part of the process of making the very best software we can. Consider for example this recent PR from one of the top INAV contributors, one of the top programmers: You'll notice that had 60+ comments and multiple changes before it was merged. In the end, we got the best feature we could by putting our heads together. So I hope you're not discouraged. Perhaps consider Mosca's suggestion - if the baro / a baro says we're falling, but the GPS and accelerometer say we're hovering, we're probably hovering. Or close to it. If you did have two baros, you'd probably want to choose the one that agrees with the GPS and accelerometer. You might think of other things to consider. Perhaps if the throttle is much below hover_throttle, it's more likely we're falling. If increasing the throttle vs 1 seconds ago apparently leads to faster falling - that's suspect. I hope you continue to brainstorm and experiment. |
@sensei-hacker thank you for warm welcome worlds. I know how software usually constructed and how new features discussed and evolved. I have lots of collab experience in other private software projects. So don't worry, I'm fine with feedback and possible changes. The problem with @mmosca suggestion from my point of view is builds, like mine, which did not have GPS sensor at all. Even if my fly baby have GPS module installed - it is useless for me, because 80% of time it is jammed in our neighborhood. |
The problem with this simple implementation is that is won't be better than adding a well mounted baro. If baro 2 also suffers from noise, the noise is not cancellable with baro 1 noise and when you add both values, you are also doubling the noise. Two baros don't work like differential pairs (where the noise signal ends up inverted and canceling in the process of combining the pair). If you fix the mounting problems of the baro (either 1 or 2), it will work better with a single baro than by adding a noisy baro to a good baro. The baro is only one of the inputs for the altitude estimation, so there is already some mitigation in there for altitude estimation. |
@mmosca I did not pursue target to decline baro noise at all by adding second baro, my goals is next:
|
Barometer noise is not random, It is directly correlated with local factors that don't propagate at the speed of light (prop wash, direct airflow, etc), or don't propagate at all and can actually compound. Eg.: baro 1 has +1m error, baro 2 has +4m error, you will be worse than using just baro 1. Basically, bad data in, bad data out. The correct fix is not to just avarage 2 sensors. |
@mmosca Thank you, got your point. What options will be preferred if we agree to have >1 baro on the board? Kalman filtering? Or some similar technic? Your input will be appreciated and I'll try to fix this PR accordingly. |
The baro is already filtered using a PT1 filter in the position estimator. You can test different filters to see if they result in a better, more consistent result, but I still think the root cause here is still mainly a poorly mounted barometer in most cases. The way I see it at the moment:
|
@mmosca By the way, Marcelo, can you suggest me right way to hide additional configuration parameters behind define? as example - remove any additional CLI parameters if USE_BARO_MULTI not defined? |
BaroAlt should be an array with BARO_COUNT elements. That way the code can always look like. It will work the same way for BARO_COUNT = 1 or 2
The cli command for baro multi is not be needed. Log individual baros to blackbox and/or use debug values that can be seen on the osd. |
Using baro in multirotor can be hard due to air interferences from prop blades with baro on flight controller. That's why I decide to implement ability to use two barometers on I2C lane and dumped out second barometer to separate board.
My test setup is BMP280 baro on separate board and internal SPL06 baro, which come with Speedybee F405V3 flight controller. BMP280 switched to separate address (0x77)
Current logic is implemented to get mean value between 2 devices. Devices count and hardware list can be extended to support more barometers on same line, I just need to find more adequate way to define arrays in configuration.
Tested on my table setup, works fine, both barometers react on my breath and change value in Sensors tab accordingly.
Will add flight test DVR's little bit later.
How to test it:
Find BMP280 module and switch it to 0x77 address by soldering bridge between pin 5 and 3V3 power of baro.
Apply patch
define
in target file of your flight controller with onboard baro (in my case it was SPL06)
Update firmware on flight controller
Use CLI to change settings
multi_baro_hardware_1
andmulti_baro_hardware_2
according to your hardwareTry to whirl some wind into first and second barometer and ensure that both barometers affects sensor data line in Sensors tab if configurator.
Also added one more CLI command to review raw baro data, can be removed if not necessary.
P.S. My first pull request to INAV, don't judge me too much please )