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

ekf2_main - Add optical flow innovation pre-flight check #13036

Merged
merged 18 commits into from
Oct 22, 2019
Merged

Conversation

bresch
Copy link
Member

@bresch bresch commented Sep 27, 2019

Currently, only the innovations from the vel_pos_innov array are checked. Since the optical flow innovations are in flow_innov, they are never checked and when flying optical flow only, the GPS velocities innovations are still checked.
To fix this inconsistency, this PR adds optical flow innovation checks and generally, the checks are only performed if a the sensor is fused.
I also refactored a bit the whole pre-flight check part of this module to make it more scalable and readable. Adding a new check (e.g.: adding a new check should be easy).

The check is also the same for all the innovations:
fail if (low-pass(innov) > test or innov > 2x test)

{
bool has_failed = false;

bool doing_ne_aiding = control_status.flags.gps || control_status.flags.ev_pos;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool doing_ne_aiding = control_status.flags.gps || control_status.flags.ev_pos;
bool doing_hpos_aiding = control_status.flags.gps || control_status.flags.ev_pos;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhkabir Since you want to rename that hpos because EV fusion doesn't mean NE navigation, it means that you don't actually really care of heading. Should we then use the test limit in the else part of the selection?

Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good cleanup.

src/modules/ekf2/Utility/InnovationLpf.cpp Outdated Show resolved Hide resolved
src/modules/ekf2/ekf2_main.cpp Outdated Show resolved Hide resolved
src/modules/ekf2/ekf2_main.cpp Outdated Show resolved Hide resolved
src/modules/ekf2/ekf2_main.cpp Outdated Show resolved Hide resolved
src/modules/ekf2/ekf2_main.cpp Outdated Show resolved Hide resolved
src/modules/ekf2/ekf2_main.cpp Outdated Show resolved Hide resolved
src/modules/ekf2/ekf2_main.cpp Outdated Show resolved Hide resolved
src/modules/ekf2/Utility/InnovationLpf.cpp Outdated Show resolved Hide resolved
@bresch bresch force-pushed the dev-ekf-prearm branch 2 times, most recently from 4ddbac3 to acdd046 Compare October 1, 2019 12:28
@bresch
Copy link
Member Author

bresch commented Oct 1, 2019

@bkueng Thanks for the review, I did the corrections, can you review again please?

@bkueng
Copy link
Member

bkueng commented Oct 2, 2019

Looks good.

@priseborough
Copy link
Contributor

How has this been tested? Can you supply a log?

@priseborough
Copy link
Contributor

it could be tested using replay with post filtered innovations printed to file at a reduced rate, eg 1Hz and we could then compare the files before and after to check that the filtering is still working the same.

@CarlOlsson
Copy link
Contributor

Nice! Would be awesome if we could add a bitfield for which check that is failing since it can be a bit annoying to debug since we don't log the low pass filtered innovations

@bresch bresch force-pushed the dev-ekf-prearm branch 2 times, most recently from 71efbea to d029973 Compare October 16, 2019 09:02
@bresch bresch marked this pull request as ready for review October 16, 2019 09:07
@bresch
Copy link
Member Author

bresch commented Oct 16, 2019

@CarlOlsson I changed the "master check" for a bitfield

@CarlOlsson
Copy link
Contributor

@CarlOlsson I changed the "master check" for a bitfield

Awesome!

msg/estimator_status.msg Outdated Show resolved Hide resolved
bresch added 11 commits October 21, 2019 12:17
spike limit during the update call to avoid storing it here
- set spike_lim constants as static constexpr, set innovation
- set checker helper functions as static
- rename the mix of heading and yaw as heading to avoid confusion
pre-flight checks.
Add simple unit testing
Use bitmask instead of general flag to have more granularity
…sending booleans in the update function

This makes it more scalable as more checks will be added
@dagar dagar requested a review from a team October 21, 2019 16:35
@jorge789
Copy link

jorge789 commented Oct 21, 2019

Tested on PixRacer V4:

Modes Tested
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

- Procedure
Arm and Takeoff in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceeds to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint activate RTL and see landing behavior.

Notes:
No issues noted good flight in general.

Log:
https://review.px4.io/plot_app?log=41494978-b2c3-4db3-825d-f932e4031101

https://review.px4.io/plot_app?log=bce2449c-7829-4545-a932-53449c2a84f4

https://review.px4.io/plot_app?log=ffbd784e-7202-4edd-98fe-636e666ee03e

Master comparison:
https://review.px4.io/plot_app?log=2de4cc7b-b4b8-4b84-8a0e-0378f86dafcc

https://review.px4.io/plot_app?log=307eac73-9648-4526-8b35-5b52a1ebb4ae

https://review.px4.io/plot_app?log=c06841d2-0474-43f6-979f-9cc3796b9705

@jorge789
Copy link

jorge789 commented Oct 21, 2019

Tested on Pixhawk 4 V5:
Modes Tested:
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

Procedure
Arm and Takeoff in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceeds to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint activate RTL and see landing behavior.

Notes:
No issues noted good flight in general.

Log:
https://review.px4.io/plot_app?log=22706bcb-7b91-43a4-8a04-caf20ccb4b7f&fbclid=IwAR0i9VIsraI2eXvt6FxgFEF_VcoUhi4gJPCJl1PD_QUd8c8g6vc0D6rVaqk

Tested on Pixhawk pro v4:
Modes Tested

Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

Procedure
Arm and Takeoff in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceeds to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint activate RTL and see landing behavior.

Notes:
No issues noted good flight in general.

Log:
https://review.px4.io/plot_app?log=73d259d0-4fae-4812-bb5b-31410a5a4207&fbclid=IwAR1J1apCKJH3pmBfEHa24zer0ce6HjyM2Ug_A1PYMNd-2RAVQ47KxeeYcFc

@bresch
Copy link
Member Author

bresch commented Oct 22, 2019

Thanks @jorge789 !

@bresch
Copy link
Member Author

bresch commented Oct 22, 2019

@dagar Can you approve and merge? I also made a few tests on a bench setup to see the error messages.

@bresch bresch merged commit 549fb0d into master Oct 22, 2019
@bresch bresch deleted the dev-ekf-prearm branch October 22, 2019 14:22
julianoes pushed a commit that referenced this pull request Oct 25, 2019
* ekf2: Add FirstOrderLpf and InnovationLpf classes for innovation lowpass filtering

* ekf2: use InnovLpf filter class in preflight checks

* ekf2: move selection of yaw test limit for pre-flight check in function

* ekf2: Move pre-flight checks into separate function

* ekf2: use static constexpr insetead of inline for sq (square) function

* ekf2: Split pre-flight checks in separate functions
Also use the same check for all the innovations:
innov_lpf < test and innov < 2xtest

* ekf2: Add optical flow pre-flight check

* ekf2: Combine FirstOrderLpf and InnovationLpf in single class

* ekf2: check vel_pos_innov when ev_pos is active as well

* ekf2: transform InnovationLpf into a header only library and pass the
spike limit during the update call to avoid storing it here

* ekf2: Static and const cleanup
- set spike_lim constants as static constexpr, set innovation
- set checker helper functions as static
- rename the mix of heading and yaw as heading to avoid confusion

* ekf2: use ternary operator in selectHeadingTestLimit instead of if-else

* ekf2: store intermediate redults in const bool flags. Those will be used for logging

* ekf2: set variable const whenever possible

* ekf2: create PreFlightChecker class that handle all the innovation
pre-flight checks.
Add simple unit testing
Use bitmask instead of general flag to have more granularity

* PreFlightChecker: use setter for the innovations to check instead of sending booleans in the update function
This makes it more scalable as more checks will be added

* ekf: Use booleans instead of bitmask for ekf preflt checks
Rename "down" to "vert"
dagar pushed a commit that referenced this pull request Oct 28, 2019
* ekf2: Add FirstOrderLpf and InnovationLpf classes for innovation lowpass filtering

* ekf2: use InnovLpf filter class in preflight checks

* ekf2: move selection of yaw test limit for pre-flight check in function

* ekf2: Move pre-flight checks into separate function

* ekf2: use static constexpr insetead of inline for sq (square) function

* ekf2: Split pre-flight checks in separate functions
Also use the same check for all the innovations:
innov_lpf < test and innov < 2xtest

* ekf2: Add optical flow pre-flight check

* ekf2: Combine FirstOrderLpf and InnovationLpf in single class

* ekf2: check vel_pos_innov when ev_pos is active as well

* ekf2: transform InnovationLpf into a header only library and pass the
spike limit during the update call to avoid storing it here

* ekf2: Static and const cleanup
- set spike_lim constants as static constexpr, set innovation
- set checker helper functions as static
- rename the mix of heading and yaw as heading to avoid confusion

* ekf2: use ternary operator in selectHeadingTestLimit instead of if-else

* ekf2: store intermediate redults in const bool flags. Those will be used for logging

* ekf2: set variable const whenever possible

* ekf2: create PreFlightChecker class that handle all the innovation
pre-flight checks.
Add simple unit testing
Use bitmask instead of general flag to have more granularity

* PreFlightChecker: use setter for the innovations to check instead of sending booleans in the update function
This makes it more scalable as more checks will be added

* ekf: Use booleans instead of bitmask for ekf preflt checks
Rename "down" to "vert"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants