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

map_server: Initialise NodeHandle outside the MapServer constructor #1122

Merged
merged 1 commit into from
May 26, 2021

Conversation

bergercookie
Copy link
Contributor

@bergercookie bergercookie commented May 21, 2021

Currently if an exception is raised inside the MapServer execution, it's caught by the try-catch statement in main. However at that point the ros::NodeHandle instance is already destroyed since it's constructed inside MapServer::MapServer and that's initialised inside the try scope.

Because of that the ROS_ERROR statement in the catch doesn't print anything and the user isn't informed of the exception.

In this PR I'm initialising the NodeHandle outside MapServer and I'm passing a reference to it during construction.
a second NodeHandle outside MapServer in main

@@ -251,7 +252,7 @@ int main(int argc, char **argv)

try
{
MapServer ms(fname, res);
MapServer ms(private_nh, fname, res);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to pass in a special node handle? Isn't simply creating a node handle right after ros::init() above sufficient, such that we still have at least one NodeHandle in scope inside the catch()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it best of initializing and sharing a single NodeHandle instead, but sure, I could switch to that If you prefer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@DLu DLu merged commit 43832f5 into ros-planning:noetic-devel May 26, 2021
MatthijsBurgh pushed a commit to tue-robotics/navigation that referenced this pull request Aug 10, 2021
MatthijsBurgh pushed a commit to tue-robotics/navigation that referenced this pull request Sep 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants