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

useRosTime in settings/client.xml doesn't work #10

Closed
worldpotato opened this issue Dec 4, 2020 · 2 comments · Fixed by #11
Closed

useRosTime in settings/client.xml doesn't work #10

worldpotato opened this issue Dec 4, 2020 · 2 comments · Fixed by #11
Assignees
Labels

Comments

@worldpotato
Copy link
Contributor

The problem:

The ros node doesn't use the ros time even if the value for useRosTime is set to true. The default value is true, see:

<useRosTime>true</useRosTime>

How it should work:

The ros node should use the ros time if it is set in the settings.

possible solution

I think the option should be renamed to use-ros-time because it is read with as that name value. See:

("use-ros-time", po::bool_switch(&ros_node_settings.use_ros_time),

@rossctaylor rossctaylor self-assigned this Dec 14, 2020
@rossctaylor
Copy link
Contributor

Thanks for pointing this out. It looks like the problem is the parent node name for the setting. The code is expecting Settings.RosNode.useRosTime:

use_ros_time = settings.get("Settings.RosNode.useRosTime", use_ros_time);

While the default settings uses Settings.ClientRos.useRosTime:

I will get this fixed soon. In the meantime, you can change ClientRos to RosNode in the settings file or use the command line option (--use-ros-time) to override.

@worldpotato
Copy link
Contributor Author

Yes, figured it out on the weekend, too. But our lidar (M8) got damaged and I had no chance to test it properly.

@rossctaylor rossctaylor linked a pull request Dec 17, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants