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

Pr att bias cleanup2 #7773

Closed
wants to merge 34 commits into from

Conversation

Projects
None yet
9 participants
@LorenzMeier
Copy link
Member

commented Aug 11, 2017

No description provided.

@LorenzMeier LorenzMeier force-pushed the pr-att-bias-cleanup2 branch from 48d592d to 97121a1 Aug 11, 2017

@priseborough

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2017

I'll get the changes in that @sanderux requested for the fixed wing controller.

@sanderux

This comment has been minimized.

Copy link
Member

commented Aug 11, 2017

@priseborough thanks for addressing my comments.
I mentioned inconsistent use of airspeed_mode vs airspeed_disabled and you choose to use airspeed_mode consistently. reading the code perhaps airspeed_disabled would have been clearer, but not a big issue. I'm fine with it either way.

@priseborough

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2017

I can change it to be airspeed disabled everywhere assuming that it will always be used as a boolean and we will not be adding other mode selections (eg use none, use estimated, use measured).

@sanderux

This comment has been minimized.

Copy link
Member

commented Aug 11, 2017

considering now it is a boolean and if in the future that changes the code would have to change anyway, i say lets make it airspeed_disabled

@LorenzMeier

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2017

@PX4TestFlights Please thoroughly re-test this. Thanks!

@LorenzMeier

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2017

On all airframes that is - multicopter, VTOL, fixed wing.

dagar and others added some commits Jun 20, 2017

msgs : add sensor_corrected message with in-run bias estimates and co…
…rrected sensor readings from the estimator
ecl: update to latest master
Fixes build error caused by unnecessary double precision operations.
msg: Fix naming and descriptions in wind_estimate
Clarify units and definitions. Covariance is a measure of joint variability of two variables and should not be used to describe the variability of a single variable.

priseborough added some commits Aug 11, 2017

@mhkabir mhkabir force-pushed the pr-att-bias-cleanup2 branch from f3cbaa7 to d07e17a Aug 14, 2017

@mhkabir mhkabir referenced this pull request Aug 14, 2017

Closed

Attitude topics cleanup #7523

@mhkabir

This comment has been minimized.

Copy link
Member

commented Aug 14, 2017

Rebased.

@Avysuarez

This comment has been minimized.

Copy link

commented Aug 14, 2017

Flight with pixhawk 1(V3); Good flight, no issues
https://logs.px4.io/plot_app?log=70e4aca1-4882-4b38-9250-7480e5b96c53

Flight with pixhawk pro (V4PRO); Good flight, no issues
https://logs.px4.io/plot_app?log=7dc4c2cf-58f7-4e3e-b944-b811abe4f4e8

Flight with pixhawk 1 (V2); Good flight, no issues
https://logs.px4.io/plot_app?log=d89361b0-a187-4f83-a61b-3eba8ff3e25e

@mhkabir

This comment has been minimized.

Copy link
Member

commented Aug 14, 2017

@santiago3dr @Avysuarez Thanks! We still need coverage on VTOL and FW. :)

@mhkabir

This comment has been minimized.

Copy link
Member

commented Aug 14, 2017

Unrelated to the PR, but one of the logs have that annoying transformation bug. Those tracking errors would've constituted a flyaway in reality :
image

@dagar

This comment has been minimized.

Copy link
Member

commented Aug 14, 2017

Unrelated to the PR, but one of the logs have that annoying transformation bug. Those tracking errors would've constituted a flyaway in reality :

Any clues for where it's happening? When have you seen this bug?

@mhkabir

This comment has been minimized.

Copy link
Member

commented Aug 14, 2017

@dagar It's filed here : PX4/pyulog#13, but I don't think anyone is closer to finding the core problem.

@mhkabir

This comment has been minimized.

Copy link
Member

commented Aug 17, 2017

@santiago3dr Many thanks for the excellent test coverage!

@LorenzMeier @priseborough @dagar I wil merge this soon after a final review unless any of you have any objections. :)

@sanderux

This comment has been minimized.

Copy link
Member

commented Aug 17, 2017

Your change has some impact on airspeedsensorless vehicles. Do you think i should give it a quick test on the deltaquad?

@mhkabir

This comment has been minimized.

Copy link
Member

commented Aug 17, 2017

@sanderux Yes definitely! Please do test and let us know if airspeed-less operation works as expected.

@LorenzMeier

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2017

@sanderux Any chance to get this done today?

@sanderux

This comment has been minimized.

Copy link
Member

commented Aug 21, 2017

Sorry, no luck yet, planning and weather keep interfering. I will try in the next 2 days

@mhkabir

This comment has been minimized.

Copy link
Member

commented Aug 30, 2017

@sanderux Updates? If none, let's get this in anyway.

@dagar dagar self-assigned this Aug 30, 2017

@dagar

This comment has been minimized.

Copy link
Member

commented Aug 30, 2017

Rebasing

@r0gelion

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2017

@dagar does @PX4TestFlights need to re-test?

@dagar

This comment has been minimized.

Copy link
Member

commented Aug 30, 2017

Rebase became difficult and didn't seem worth it.
Continued here as a squashed commit, #7882.
We can split it into smaller commits if desired.

@dagar dagar closed this Aug 30, 2017

@LorenzMeier LorenzMeier deleted the pr-att-bias-cleanup2 branch Aug 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.