-
Notifications
You must be signed in to change notification settings - Fork 52
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
fix for controller automatically reconnecting after disconnect, correct oneshot timer for controller feedback, moved rclpy.shutdown() in the SignalHandler #19
Conversation
updated Timers usage to new rclpy version
….create_timer(). Added new bringup script for the Docker container
… the original project author
…now() to rclpy.shutdown()
…connect on disconnecting and killing the node gracefully on SIGINT from user
Thanks for contributing again! (Sorry for the late response; somehow the first email ended up being archived) I'll try and test out your patch on a docker image within the next few days! |
@naoki-mizuno No worriez! Let me know how the tests go once you have time to do them! |
… into naoki-mizuno-foxy-devel
@naoki-mizuno I have just seen that my |
I was able to test this PR on a docker image and confirmed that In the container I ran |
Hello @naoki-mizuno and thanks for the quick reply! Now this is where things start getting weird. I have checked out the exact same commit you have specified there and with the same command specified by you there the controller does not reconnect for me. On top of this, when running the node with the default configuration, so with |
Can still confirm that with the latest patch in this pr the aforementioned issues do not appear anymore, in either modes so with or without |
To further clarify this issue, I believe that the place in code in |
Thanks for looking into this!
I agree. There is no I'll go ahead and merge this PR into |
Addresses #20
After further development on my personal project I came to the realization that there were still improvements to be done for the
foxy-devel
branch.rclpy.shutdown()
is then handled in theSIGINT
handler in theds4_driver_node.py
. Previously the spinning was done right after the device was discovered and set-up and this rendered the re-discovery of the controller after disconnect unable to work properly since spinning was keeping the thread busy. I have tested many approaches which can be seen in the older commits on the working tree and thus far this method of spinning in a separate thread has been the best solution for this issue.controller_ros.py
had a mistake at the line where i was destroying it after the first callback call. This is now fixed as well and the one-shot timer for the feedback works as intended.Sorry for the big number commits, these can be squished after the merge. I went through lots of experimentation in order to be able to have the device discovery and spin run correctly together and to also have the node die gracefully on
SIGINT
.@naoki-mizuno I know this merge request might be a little confusing as I am not sure if the automatic controller re-discovery was a feature intended originally for this package. To me it has been really useful. Kindly check this whenever you can and let me know if any clarifications are needed. This is I believe the best and most stable version of the package for foxy and I would very much appreciate if it could be merged to the master repository so then I won't need to keep track of my personal forked version in the .repos of my bigger project.
Cheers!