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

Do not subscribe /clock from nodelet load #120

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

peci1
Copy link
Contributor

@peci1 peci1 commented Aug 16, 2023

Depends on ros/ros_comm#2342 .

See discussion in https://robotics.stackexchange.com/questions/96165/nodelet-load-process-does-message-deserialization .

When running in nodelet load mode, the node doesn't really do anything that would require sim time. It should only run the bond, one service call and nothing else. This PR makes sure that it is the case even if /use_sim_time is set to true.

@sloretz
Copy link
Contributor

sloretz commented Sep 4, 2024

@ros-pull-request-builder retest this please

@peci1
Copy link
Contributor Author

peci1 commented Sep 4, 2024

Hmm, maybe that was too fast and we actually need to wait for ros_comm release?

@peci1 peci1 closed this Sep 13, 2024
@peci1 peci1 reopened this Sep 13, 2024
@peci1 peci1 closed this Sep 14, 2024
@peci1 peci1 reopened this Sep 14, 2024
@peci1
Copy link
Contributor Author

peci1 commented Sep 14, 2024

The tests in this PR have passed after the required changes in ros_comm have been merged.

This is now ready for review.

@peci1
Copy link
Contributor Author

peci1 commented Sep 20, 2024

@sloretz it would be great to have this PR in the larger ros_comm test you called for. Could you please have a look at it?

Copy link
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

LGTM! @mjcarroll how does this look to you?

@mjcarroll mjcarroll merged commit f840a28 into ros:noetic-devel Sep 25, 2024
2 checks passed
@peci1
Copy link
Contributor Author

peci1 commented Sep 25, 2024

Thanks! Do you intend to make a release of nodelet_core in the period when ros_comm 1.17 is being tested?

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.

3 participants