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

land_detector initiate cycle immediately #12196

Merged
merged 1 commit into from
Jun 16, 2019
Merged

land_detector initiate cycle immediately #12196

merged 1 commit into from
Jun 16, 2019

Conversation

dagar
Copy link
Member

@dagar dagar commented Jun 6, 2019

@dagar dagar added the bug label Jun 6, 2019
@dagar dagar requested a review from julianoes June 6, 2019 01:11
@dagar dagar mentioned this pull request Jun 6, 2019
Closed
@dagar dagar force-pushed the pr-land_detector_misc branch from 03748dc to 6e860e7 Compare June 6, 2019 01:19
@julianoes
Copy link
Contributor

This did not work for me, same error message.

@dagar dagar force-pushed the pr-land_detector_misc branch 2 times, most recently from 4d56146 to 30c164d Compare June 7, 2019 13:32
@dagar
Copy link
Member Author

dagar commented Jun 7, 2019

How about now? With uORB::Subscription we no longer have to worry about subscribing in the thread of execution, so some of the awkward startup dance goes away.

@dagar dagar force-pushed the pr-land_detector_misc branch from 30c164d to 12b2b85 Compare June 7, 2019 14:03
@julianoes
Copy link
Contributor

Not sure what make of this 😄:

Thread 62 "px4" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fffbaffd700 (LWP 32129)]
0x00007ffff7a81eb5 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install libgcc-9.1.1-1.fc30.x86_64 libstdc++-9.1.1-1.fc30.x86_64
(gdb) bt
#0  0x00007ffff7a81eb5 in raise () from /lib64/libc.so.6
#1  0x00007ffff7a6c895 in abort () from /lib64/libc.so.6
#2  0x00007ffff7e0f6da in ?? () from /lib64/libstdc++.so.6
#3  0x00007ffff7e1b61c in ?? () from /lib64/libstdc++.so.6
#4  0x00007ffff7e1b677 in std::terminate() () from /lib64/libstdc++.so.6
#5  0x00007ffff7e1c415 in __cxa_pure_virtual () from /lib64/libstdc++.so.6
#6  0x0000000000469137 in land_detector::LandDetector::_check_params (this=0x7fffac001a10, force=<optimized out>)
    at /home/julianoes/src/Firmware/src/modules/land_detector/LandDetector.cpp:156
#7  0x000000000046a2e9 in land_detector::MulticopterLandDetector::MulticopterLandDetector (this=0x7fffac001a10)
    at /home/julianoes/src/Firmware/src/modules/land_detector/MulticopterLandDetector.cpp:73
#8  0x0000000000468d0e in land_detector::LandDetector::task_spawn (argc=argc@entry=2, argv=argv@entry=0x7fffbaffcba8)
    at /home/julianoes/src/Firmware/src/modules/land_detector/land_detector_main.cpp:74
#9  0x0000000000419816 in ModuleBase<land_detector::LandDetector>::start_command_base (argv=0x7fffbaffcba8, argc=2)
    at /home/julianoes/src/Firmware/src/platforms/px4_module.h:317
#10 ModuleBase<land_detector::LandDetector>::main (argc=3, argv=0x7fffbaffcba0) at /home/julianoes/src/Firmware/src/platforms/px4_module.h:140
#11 0x00000000005af402 in px4_daemon::Pxh::process_line (line="land_detector start multicopter", silently_fail=silently_fail@entry=true)
    at /usr/include/c++/9/bits/basic_string.h:401
#12 0x00000000005af810 in px4_daemon::Pxh::process_line (line="land_detector start multicopter", silently_fail=silently_fail@entry=true)
    at /usr/include/c++/9/bits/basic_string.h:1031
#13 0x00000000005b07f2 in px4_daemon::Server::_handle_client (arg=0x7ffff0000b40) at /home/julianoes/src/Firmware/platforms/posix/src/px4_daemon/server.cpp:278
#14 0x00007ffff7f835a2 in start_thread () from /lib64/libpthread.so.0
#15 0x00007ffff7b45163 in clone () from /lib64/libc.so.6

mcsauder
mcsauder reviewed Jun 8, 2019
View reviewed changes
src/modules/land_detector/LandDetector.cpp
_landDetected.freefall = false;
_landDetected.landed = true;
_landDetected.ground_contact = false;
_landDetected.maybe_landed = false;
Copy link
Contributor

@mcsauder mcsauder Jun 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Is it possible to default initialize these values at the struct definition instead?
(Edit: Except for the timestamp, of course.)

mcsauder
mcsauder reviewed Jun 8, 2019
View reviewed changes
src/modules/land_detector/land_detector_main.cpp Outdated
int ret = obj->start();

if (ret < 0) {
_object.store(nullptr);
_task_id = -1;
delete obj;
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's not a change in this PR, but, do we need to add an obj = nullptr here?

@mcsauder
Copy link
Contributor

mcsauder commented Jun 8, 2019 •
edited
Loading

Hi @dagar, I flight tested this PR and it's not working quite right, it doesn't detect properly:
https://review.px4.io/plot_app?log=b5b3bc76-c1d3-445c-a29a-484a254a70ae

@weekly-digest weekly-digest bot mentioned this pull request Jun 9, 2019
Closed
@dagar dagar force-pushed the pr-land_detector_misc branch from 12b2b85 to a2e264d Compare June 16, 2019 00:29
@dagar
Copy link
Member Author

dagar commented Jun 16, 2019

The problem was the _check_params(true) call in the constructor of LandDetector() (the base class). LandDetector::_update_params() is pure virtual.

@dagar dagar force-pushed the pr-land_detector_misc branch from a2e264d to 7cb520d Compare June 16, 2019 00:43
@dagar dagar merged commit 106ee28 into PX4:master Jun 16, 2019
@dagar dagar deleted the pr-land_detector_misc branch June 16, 2019 01:30
@weekly-digest weekly-digest bot mentioned this pull request Jun 23, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcsauder mcsauder mcsauder left review comments

@julianoes julianoes Awaiting requested review from julianoes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

land_detector complains about timeout
3 participants
@dagar @julianoes @mcsauder