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

Add service to reinitialize global localization #157

Merged
merged 13 commits into from
May 10, 2023

Conversation

olmerg
Copy link
Collaborator

@olmerg olmerg commented Mar 30, 2023

Closes #141.

Summary

  • Implement a service to reinitialize_global_localization, which restart the particle through a uniform distribution.
  • create a parameter always_reset_initial_pose. control if the position is reset when the node enter on_configure.

To complete this functionality a variable initial_pose_is_known_ in amcl_node control when should be published the pose and the particles of the filter.

Checklist

  • Read the contributing guidelines.
  • Configured pre-commit and ran colcon test locally.
  • Signed all commits for DCO.
  • Added tests (regression tests for bugs, coverage of new code for features).
  • Updated documentation (as needed).
  • Checked that CI is passing.

Copy link
Member

@nahueespinosa nahueespinosa left a comment

Choose a reason for hiding this comment

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

@olmerg First pass. It looks like you haven't signed your commits, and you haven't run the linter or the tests. Please do it.

beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
beluga/include/beluga/algorithm/particle_filter.hpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
@nahueespinosa
Copy link
Member

nahueespinosa commented Mar 30, 2023

Please, update the PR title following the contributing guidelines:

  • Limit the subject line to 50 characters.
  • Capitalize the subject line.
  • Do not end the subject line with a period.
  • Use the imperative mood in the subject line.

@olmerg olmerg changed the title reinitialize_global_localization Reinitialize global localization ros service Mar 30, 2023
@nahueespinosa nahueespinosa added enhancement New feature or request ros Related to ROS labels Mar 30, 2023
@nahueespinosa nahueespinosa changed the title Reinitialize global localization ros service Add service to reinitialize global localization Mar 30, 2023
@olmerg
Copy link
Collaborator Author

olmerg commented Apr 3, 2023

I make a new commit with some change in amcl_node but beluga system test is not working.

 [ERROR] [1680529048.150431725] [rosbag2_storage]: No storage id specified, and no plugin found that could open URI
    
    closing.
    
    closing.
    unknown file: Failure
    C++ exception with description "No storage could be initialized. Abort" thrown in the test body.

@ivanpauno
Copy link
Collaborator

ivanpauno commented Apr 3, 2023

I make a new commit with some change in amcl_node but beluga system test is not working.

Check #146 (comment) and #154

@olmerg
Copy link
Collaborator Author

olmerg commented Apr 26, 2023

@hidmic @nahueespinosa @glpuga

I am going to describe my blocking test:

ros2 lifecycle set /amcl deactivate
ros2 lifecycle set /amcl configure
ros2 lifecycle set /amcl activate

When I deactivate the particle filter stop to work but the new state is uncofigured and not inactive. The on_cleanup is never called (which I am looking to test)
Now, I try configure and activate sometimes the PF start again but always break after sometime. This same error occur if I try put autostart in false in common_nodes_launch.py and try to start the nodes by commands.

Another unsucessful test is after configure test cleanup state, but the action call (ros2 lifecycle) never finish.

after review of the log amcl finish with -6 error (line 2903)

[amcl_node-5] terminate called after throwing an instance of 'statemap::TransitionUndefinedException'
[amcl_node-5]   what():  no such transition in current state
[ERROR] [amcl_node-5]: process has died [pid 6376, exit code -6, cmd '/ws/install/beluga_amcl/lib/beluga_amcl/amcl_node --ros-args --log-level debug --ros-args -r __node:=amcl -p use_sim_time:=True --params-file /ws/install/beluga_example/share/beluga_example/config/params.yaml'].

I attach the log, after remove some noise like flatland or subscription (it is incredible the logs generated in debug mode)
log_beluga.log

@nahueespinosa
Copy link
Member

@olmerg The error might be related to ros2/rclcpp#1880. I'd say we don't need to implement the feature to save the last pose since nav2_amcl appears to have the same issue.

Implementing the service to reinitialize global localization would be enough for this PR. Please remove other changes.
Also note that we have discussed that this approach is not the correct way to do it during our weekly sync and here #157 (comment).

@hidmic
Copy link
Collaborator

hidmic commented May 3, 2023

@olmerg how far are we here? Need help?

@olmerg
Copy link
Collaborator Author

olmerg commented May 3, 2023

the functionality is working and I remove the part to save the state. to test the functionality

ros2 launch beluga_example example_launch.py

after some seconds in a new terminal

ros2 service call /reinitialize_global_localization std_srvs/srv/Empty

@nahueespinosa
Copy link
Member

@olmerg Could you rebase to current main and sign off your commits? GitHub seems to be confused about what belongs to your branch and what comes from main.

@hidmic
Copy link
Collaborator

hidmic commented May 8, 2023

Ping @olmerg

@olmerg olmerg force-pushed the olmerg/-reinitialize_global_localization branch from 0a51832 to 7e8ff05 Compare May 9, 2023 12:53
@olmerg olmerg requested review from hidmic and nahueespinosa and removed request for ivanpauno and nahueespinosa May 9, 2023 13:27
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
beluga_amcl/include/beluga_amcl/private/amcl_node.hpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
@olmerg olmerg requested a review from nahueespinosa May 9, 2023 14:33
Signed-off-by: Olmer Garcia-Bedoya <olmerg@ekumenlabs.com>
@olmerg olmerg requested review from nahueespinosa and removed request for nahueespinosa and hidmic May 10, 2023 14:20
Copy link
Member

@nahueespinosa nahueespinosa left a comment

Choose a reason for hiding this comment

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

@olmerg Looking good! Please fix the whitespaces.

beluga_amcl/include/beluga_amcl/private/amcl_node.hpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Outdated Show resolved Hide resolved
beluga_amcl/src/amcl_node.cpp Show resolved Hide resolved
Signed-off-by: Olmer Garcia-Bedoya <olmerg@ekumenlabs.com>
@olmerg olmerg requested a review from nahueespinosa May 10, 2023 15:19
Copy link
Member

@nahueespinosa nahueespinosa left a comment

Choose a reason for hiding this comment

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

LGTM!

@nahueespinosa nahueespinosa merged commit 9b09004 into main May 10, 2023
@nahueespinosa nahueespinosa deleted the olmerg/-reinitialize_global_localization branch May 10, 2023 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ros Related to ROS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Service to reinitialize global localization
4 participants