Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Do not subscribe /clock from nodelet load#120

Merged
mjcarroll merged 1 commit intoros:noetic-develfrom
peci1:no-sim-time
Sep 25, 2024
Merged

Do not subscribe /clock from nodelet load#120
mjcarroll merged 1 commit intoros:noetic-develfrom
peci1:no-sim-time

Conversation

@peci1
Copy link
Copy Markdown
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
Copy Markdown
Contributor

sloretz commented Sep 4, 2024

@ros-pull-request-builder retest this please

@peci1
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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?

@sloretz sloretz requested a review from mjcarroll September 20, 2024 20:57
@mjcarroll mjcarroll merged commit f840a28 into ros:noetic-devel Sep 25, 2024
@peci1
Copy link
Copy Markdown
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?

@ros-discourse
Copy link
Copy Markdown

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/preparing-for-noetic-sync-2024-10-31/40332/5

@ros-discourse
Copy link
Copy Markdown

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/preparing-for-noetic-sync-2024-12-05/40942/2

@ros-discourse
Copy link
Copy Markdown

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/new-packages-for-noetic-2024-12-06/40995/2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants