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.

Sign up for GitHub

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

Jump to bottom

bad altitude hold v1.11.0-beta1 vs v1.10.0 #14640

Closed
dagar opened this issue Apr 10, 2020 · 32 comments
Closed

bad altitude hold v1.11.0-beta1 vs v1.10.0 #14640

dagar opened this issue Apr 10, 2020 · 32 comments
Assignees
@priseborough @RomanBapst @bresch
Labels
bug Multirotor 🛸
Milestone
Release v1.11.0

Comments

@dagar
Copy link
Member

dagar commented Apr 10, 2020 •
edited
Loading

User reported worse altitude hold after upgrading from PX4 stable v1.10.0 to the current v1.11 beta.

v1.10.1

master (slightly before v1.11.0-beta1 with old IMU drivers)

v1.11.0-beta1

Local Position Z (v1.10.0 vs v1.11.0-beta1)

Screenshot from 2020-04-10 11-16-18

Velocity Z (v1.10.0 vs v1.11.0-beta1)

Screenshot from 2020-04-10 11-18-58
@dagar dagar added this to the Release v1.11.0 milestone Apr 10, 2020
@dagar dagar self-assigned this Apr 10, 2020
@MaEtUgR
Copy link
Member

MaEtUgR commented Apr 10, 2020

On the first glance it looks like position control in all axes has more error. I suspect the total gain to have changed. Maybe that's because I'm scaling the velocity control gains to be backwards compatible using the hover thrust which might change because of the hover thrust estimator. Just a wild guess for now.

@AlexKlimaj
Copy link
Member

This is on my 650 quad. Very noticeable poor altitude performance from the last release to master. Been flying for the past few weeks always on master and it has pretty much been the same over that period of time.

@MaEtUgR
Copy link
Member

MaEtUgR commented Apr 10, 2020 •
edited
Loading

@AlexKlimaj Thanks a lot for the feedback! That's really undesired, I'll have a look.

@saengphet
Copy link

Today I test firmware v1.11.0-beta1 with my tiltrotor + pixhawk 4, sometimes I cannot change flight mode to position hold, "REJECT POSITION CONTROL", and sometimes I can change to position hold, however in several second, the flight mode automatically bounces back to altitude hold again (by the system, not a pilot). I have checked the "gps status" and found something went wrong. the GPS info is unstable. So that, I re-uploaded firmware v1.10.2. This problem seems completely gone. I tested more than 5 times. From the log message, I found these:
-- | -- | --
2:02:251 | INFO | [ecl/EKF] reset velocity to GPS
2:02:251 | INFO | [ecl/EKF] commencing GPS fusion
2:05:397 | WARNING | [gps] ubx msg 0x0112 invalid len 45056
2:10:877 | WARNING | [commander] Failsafe enabled: no local position
2:15:864 | INFO | [ecl/EKF] reset velocity to zero
2:15:865 | WARNING | [ecl/EKF] stopping navigation
-- | -- | --

Hardware is the same, I change on firmware from v1.10.2 to v1.11.0-beta1. in v1.10.2 problem not found even single time but for v1.11.0-beta the problem was found every flight.

@dagar
Copy link
Member Author

dagar commented Apr 10, 2020

@saengphet please open a new issue (https://github.com/PX4/Firmware/issues/new/choose) and share your logs like in this issue (working v1.10.2 vs v1.11.0-beta1).

@saengphet
Copy link

@saengphet please open a new issue (https://github.com/PX4/Firmware/issues/new/choose) and share your logs like in this issue (working v1.10.2 vs v1.11.0-beta1).

Hi @dagar , I have already opened the new issue. I found (guess) that it is about serial/UART port issue. More info is available on my open issue.

@AlexKlimaj
Copy link
Member

AlexKlimaj commented Apr 15, 2020 •
edited
Loading

Here is a flight from today with v1.11.0-beta1. Issue is still present, altitude hold is very poor. Very apparent when hovering.

https://review.px4.io/plot_app?log=047d7d42-f16f-4dfd-a9a2-33613455eeeb

Video shows a translate right, left, right. On right translate, it first loses altitude then climbs back to the starting altitude. Translate left does not show the same behavior.
https://youtu.be/DCiX40yz-gg

image

@dagar dagar assigned RomanBapst and unassigned dagar and MaEtUgR Apr 21, 2020
@MaEtUgR MaEtUgR mentioned this issue Apr 21, 2020
Closed
@RomanBapst
Copy link
Contributor

@AlexisTM In order to isolate the problem I would suggest that you try using GPS altitude and see what effect it has on the altitude hold. I checked your log and the barometer innovations from the estimator correlate with ground speed which usually means that the barometer is exposed to direct airflow.

@AlexisTM
Copy link
Contributor

@AlexKlimaj, Wrong Alexis ;)

@dagar
Copy link
Member Author

dagar commented Apr 22, 2020

Note: ecl/EKF baro dynamic pressure compensation seems broken

@dagar
Copy link
Member Author

dagar commented Apr 22, 2020 •
edited
Loading

New test logs from @AlexKlimaj. Added to the initial issue content as well.

@AlexKlimaj
Copy link
Member

It was pretty obvious today that the pressurization coefficients weren't doing anything but turning up the EKF2_BARO_NOISE value greatly improved the altitude hold. You can see it in the third log "Local Z Position".

@RomanBapst
Copy link
Contributor

@AlexKlimaj In the logs that you shared the coefficients for baro compensation were all set to zero, so it's expected to do nothing. Am I missing something?

@dagar
Copy link
Member Author

dagar commented Apr 23, 2020

@AlexKlimaj In the logs that you shared the coefficients for baro compensation were all set to zero, so it's expected to do nothing. Am I missing something?

You can see a range of values tried in the last log. https://review.px4.io/plot_app?log=d178c0b5-b863-4038-bead-9026c0a0a598

@dakejahl
Copy link
Contributor

Here are another 2 logs comparing v1.10.1 and v1.11.0 with the exact same parameter set.

v1.10.1
https://review.px4.io/plot_app?log=e07547d3-ca65-4b20-b7e6-dc05109a6fe0

v1.11.0
https://review.px4.io/plot_app?log=abde49b7-8b99-4b95-b381-20f9959ff2be

My airframe setup has never been great for me due to vibration (Holybro S500 kit), but as you can see the local_z on 1.11 has much greater swing as compared to 1.10.

@RomanBapst
Copy link
Contributor

@dakejahl I had a look at your logs and assuming that it was a side by side comparison I'm convinced there is something really off. We will investigate.

@dakejahl
Copy link
Contributor

Thanks! Yeah it was side by side, minutes apart, flights separated by a firmware update.

@priseborough
Copy link
Contributor

Could be related to #14735

@priseborough
Copy link
Contributor

@RomanBapst are you still looking at this issue? Would you like me to have a look at the estimator performance in these logs?

@RomanBapst
Copy link
Contributor

@priseborough Yes, please. Would be good to get your opinion on these.

@dakejahl
Copy link
Contributor

dakejahl commented May 9, 2020 •
edited
Loading

Here's another log from master as of a few days ago, this flight actually was pretty good. Gusts of wind still seem to affect the altitude hold, but that's always been an issue for me. I don't fly a lot, so this is pretty anecdotal but, the flight performance of this flight (after I configured the notch filter) seemed on par if not better than previous flights I've had with this airframe in the past. In the past however, this airframe has always flown like crap :/
https://review.px4.io/plot_app?log=97e22101-44b0-4ed4-bd33-0b06f97dc176

@RomanBapst
Copy link
Contributor

@dakejahl Thanks for testing. The data in the log does look much better to what I've seen before.

@RomanBapst
Copy link
Contributor

@dagar @dakejahl
Flight from @dakejahl from 15 days ago:
image

Last flight:
image

The last flight you did is much shorted so it's not a totally fair comparison. I do get the impression that the data looks better overall.

@AlexKlimaj
Copy link
Member

I'm hearing from other users that this is a major issue in the beta. Any update from the dev team?

@dagar
Copy link
Member Author

dagar commented May 26, 2020

I'm hearing from other users that this is a major issue in the beta. Any update from the dev team?

I was under the impression it was greatly improved as of beta2. Do you have more recent data?

@mcsauder
Copy link
Contributor

Here's a video under high disturbances on the new beta, please read further to obtain a better dataset for your opinions: https://youtu.be/sKiYagFrxVc

Today's flights, first on the new beta, which was actually better than I've seen it a lot lately, (but I think that when I've seen it the worst is in the presence of a lot of gust disturbance like the video above), was pretty good under ideal conditions. I flew 1.11beta2 this morning after the dev call under absolutely calm, simulator perfect conditions but still saw more drift than I am used to seeing, here is that video: https://youtu.be/OXU47TlR2Vc

v1.10 stable log and video: https://review.px4.io/plot_app?log=df3d86fc-b00f-4899-9e40-5efd84cb5638

v1.11beta2 log and video: https://review.px4.io/plot_app?log=67a404c1-726d-4721-b892-8596af6c0f52

I don't have any good answers on how to fix this. :/

@mcsauder
Copy link
Contributor

mcsauder commented May 28, 2020 •
edited
Loading

At least you know how it's real!
(BTW,,, Utah is a horrible place that none of you ever want to live in... ever.. period. :) )

@mcsauder
Copy link
Contributor

P.S. @bkueng , those are your generic 250 racer values from factory reset (airframe reset)... just take a look at those plots and bask in the glory! :) Nice work!

@priseborough
Copy link
Contributor

priseborough commented May 28, 2020 •
edited
Loading

@RomanBapst and I have gone through the logs and code and discovered that there was a bug in the handling of the GPS receivers published accuracy in v1.10 stable that effectively resulted in the EKF trusting the GPs velocity more than v1.11beta2 does without the bug. With your setup that has better GPS relative to IMU and baro this resulted in worse height estimation accuracy for the new version.

We recommend you try reducing the value of EKF2_GPS_V_NOISE in increments of 0.1 down to a minimum value of 0.2 and see if that improves the behaviour.

@priseborough
Copy link
Contributor

This unintentional change in tuning was discovered when we compared the vertical velocity innovation variances between the two logs.

@RomanBapst RomanBapst mentioned this issue May 28, 2020
Merged
@dagar dagar mentioned this issue May 28, 2020
Merged
@AlexKlimaj
Copy link
Member

Flew current master today. Seems to perform as well as v1.10.1.

https://review.px4.io/plot_app?log=5b5b632a-c658-41da-b385-84f168fa0221

@dagar
Copy link
Member Author

dagar commented Jun 3, 2020

Thanks everyone!

@dagar dagar closed this as completed Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@priseborough priseborough

@RomanBapst RomanBapst

@bresch bresch

Labels
bug Multirotor 🛸
Projects
None yet
Milestone
Release v1.11.0
Development

No branches or pull requests
10 participants
@dagar @AlexKlimaj @mcsauder @priseborough @MaEtUgR @AlexisTM @RomanBapst @saengphet @bresch @dakejahl