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

Terrain Estimator not working as intended on Release/RC2 #22357

Closed
ryanjAA opened this issue Nov 14, 2023 · 7 comments
Closed

Terrain Estimator not working as intended on Release/RC2 #22357

ryanjAA opened this issue Nov 14, 2023 · 7 comments
Assignees

Comments

@ryanjAA
Copy link
Contributor

ryanjAA commented Nov 14, 2023

Describe the bug

This was actually tested on RC2 but they should be near identical to Release.

This log is of a flight done last week where both the global position terrain estimate and the estimator global position iwould toggle the entire flight. The issue was then terrain data would timeout and you couldn't auto land. This was fixed by altering the flight plan but that's not a real solution. https://review.px4.io/plot_app?log=eba74e3c-bc58-4cf6-84d2-de9034a0ecbe

If you look at this log, the global position terrain estimate just goes true false the entire flight. The estimator global position is a little more stable on it but by no means mirrored. https://review.px4.io/plot_app?log=4a862996-18a1-4d92-8022-0d219fa2cf95

Going back slightly in versions (just PR, still on 1.14) and both terrain messages follow identically without waiver (and if need be I can provide logs of the same location, almost same flight plan on 1.14 main).https://review.px4.io/plot_app?log=eba74e3c-bc58-4cf6-84d2-de9034a0ecbe

Screen Shot 2023-11-13 at 6 32 28 PM

So what's going on here? This is RC2 with the RSSI from Ark added in and lidar sf30 driver and the i2c software reset. And the i2c reset pr has been added and checked extensively without issue. Logs available if needed.

Thanks

To Reproduce

To reproduce you need a terrain estmate

Expected behavior

Terrain estimate to stay valid the entire time...

Screenshot / Media

No response

Flight Log

https://review.px4.io/plot_app?log=eba74e3c-bc58-4cf6-84d2-de9034a0ecbe

Software Version

1.14 RC2

Flight controller

Pixhawk 6c

Vehicle type

Fixed Wing

How are the different components wired up (including port information)

No response

Additional context

No response

@tstastny @sfuhrer @dagar

@sfuhrer
Copy link
Contributor

sfuhrer commented Nov 14, 2023

Thanks for the report! @bresch fyi

@ryanjAA
Copy link
Contributor Author

ryanjAA commented Nov 14, 2023

Further tested this just now.

Three logs, all in SITL. Same flight plan.

1 - Flight Review Firmware with RSSI added in, SF30 driver and i2c reset. Based on 1.14 RC2 so only a few days prior to release but tested for good measure. Fails with terrain estimate valid/invalid (what was used in above flights not in SITL as well.

2 - Flight Review 1.14 Release (untouched). Same behavior as above, terrain estimate valid/invalid.

3 - Flight Review 1.14 Main (from 2 days ago) - Correct behavior.

@ryanjAA ryanjAA changed the title Terrain Estimator not working as intended on main/RC2 Terrain Estimator not working as intended on Release/RC2 Nov 15, 2023
@bresch
Copy link
Member

bresch commented Nov 20, 2023

@ryanjAA This might be the missing fix on 1.14: #22117

@ryanjAA
Copy link
Contributor Author

ryanjAA commented Nov 21, 2023

Thanks @bresch it does work in main but trying to test this against 1.14 release causes:

 fatal error: reference to non-static member function must be called; did you mean to call it with no arguments?
                                                                || (sq(_state.vel(0)) + sq(_state.vel(1)) > fmaxf(P.trace<2>(State::vel.idx), 0.1f));

which is one of the additions

|| (sq(_state.vel(0)) + sq(_state.vel(1)) > fmaxf(P.trace<2>(State::vel.idx), 0.1f));

so what's the best way to get around that or test against release?

Also this is changed (below). Since this should be be back ported or added to release version bump, just want to make sure we can do some testing, even if just sitl initially.

~~

#ifndef EKF_RANGE_FINDER_CONSISTENCY_CHECK_HPP
#define EKF_RANGE_FINDER_CONSISTENCY_CHECK_HPP

vs


~~

just a define.

@bresch
Copy link
Member

bresch commented Nov 21, 2023

@ryanjAA I made a patch here: #22423

@ryanjAA
Copy link
Contributor Author

ryanjAA commented Nov 21, 2023

Awesome! Thanks - will test ASAP. :)

@ryanjAA
Copy link
Contributor Author

ryanjAA commented Nov 21, 2023

Tested via SITL quite a bit- works as intended. Will close and reopen if issues after testing live but don't anticipate any.

Thanks @bresch :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants