-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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
Wind & airspeed scale estimator - places #7424 #7721
Conversation
Nice! @priseborough One thing to take into consideration: We would like to remove the control state topic and instead feed all sensor data directly through. We then add the offsets on the receiving side. This avoids the latency introduced by going through control state. That means it would be good to have EKF2 directly publish the wind estimate. |
|
||
// use small angle approximation, sin(x) = x for small x | ||
const float beta_pred = rel_wind(1) / rel_wind(0); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @priseborough Can i ask you that what beta_pred = rel_wind(1) / rel_wind(0)
means? Means the a slope, tilt or somethings others? Thank you. And if it can means the slope, why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Angle of sideslip calculated using a small angle approximation
If the ultimate output of this estimator is the scale would it make sense to keep this out of the sensors pipeline and run in a work queue instead? The estimator would save the scale factor, but it would be applied elsewhere. |
That makes sense given we are going to use the main EKF to provide the wind speed estimate. |
I'll wait until #7523 goes in before reworking the way that air data is published. |
#7773 has been merged so I'll rebase this. |
The rebase is getting messy and there are changes to the cmake configs that I don't understand, so will try rebasing #7424 instead |
I have pushed the first attempt here with build errors, but am now out of time. https://github.com/PX4/Firmware/tree/pr-wind-estimator2 The number of conflicts was large and affecting build files I was not familiar with. I will resume work on this next week time permitting, but would appreciate someone else having a look at the build errors before then. |
The removal of the control state has had the biggest impact so will work through that first. |
9663576
to
568ccfe
Compare
Remove commented code when testing complete.
568ccfe
to
b142439
Compare
@priseborough Hi Paul, I have @sanderux who is very much looking forward to testing this. If you don't mind I'll try to do the integration and will ask if I need some help. |
@Tumbili Thanks for picking this up again. |
@Tumbili I'm motivated to test it too! |
@dagar @priseborough I had a look at this and I need some input from you guys regarding the architecture. |
The original motivators for the separate estimator were to enable estimation of airspeed scale factor for calibration and to eventually pull pull all air data calculation into a module that could do data fusion and fault detection using multiple data sources. In the process of testing it you showed it did a good job of estimating wind for fixed wing vehicles. In the short term it makes sense to log the estimates and use it as a ride along calibration aid initially. We can switch to using it as the primary source of wind estimates for fixed wing vehicles once we get more flight logs and can show it has better performance. |
Coming back to this with a better understanding of the sensors module and system constraints in general I'm now thinking we probably don't want this in sensors. If appropriate I'd move it to a work queue. Let's first get it working again and I'll take care of that part later (if we agree). |
// wind_estimate.variance_east = status.covariances[23]; | ||
|
||
// if (_wind_pub == nullptr) { | ||
// _wind_pub = orb_advertise(ORB_ID(wind_estimate), &wind_estimate); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could publish multiple wind estimates for the transition.
Continued in #8273. |
Rebased version of #7424
TODO
ekf2_main.cpp needs to be reworked so that when the external wind speed estimator is not running, but the ekf is using the multi-rotor drag mode to do wind speed estimation, the wind speed and airspeed estimates are taken from ekf2.