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

Prevent robot_state_publisher from publishing on a timer #175

Merged
merged 3 commits into from
Oct 2, 2021

Conversation

wep21
Copy link

@wep21 wep21 commented Sep 23, 2021

Backport a part of #158 to galactic
Refer #158 (comment)

@wep21
Copy link
Author

wep21 commented Sep 23, 2021

@clalancette Is the behavior change of this PR still too big to merge into galactic?

@@ -95,7 +95,6 @@ class RobotStatePublisher : public rclcpp::Node
rclcpp::Publisher<std_msgs::msg::String>::SharedPtr description_pub_;
std::chrono::milliseconds publish_interval_ms_;
rclcpp::Subscription<sensor_msgs::msg::JointState>::SharedPtr joint_state_sub_;
rclcpp::TimerBase::SharedPtr timer_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing a class member breaks ABI, so this line can't be backported.

Copy link
Author

@wep21 wep21 Sep 23, 2021

Choose a reason for hiding this comment

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

@sloretz Fixed at 0919354

@@ -148,9 +148,7 @@ RobotStatePublisher::RobotStatePublisher(const rclcpp::NodeOptions & options)
&RobotStatePublisher::callbackJointState, this,
std::placeholders::_1));

// trigger to publish fixed joints
timer_ = this->create_wall_timer(
publish_interval_ms_, std::bind(&RobotStatePublisher::publishFixedTransforms, this));
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC if use_tf_static is False then the fixed transforms must be published regularly or they will not be received by late joining subscribers.

I think creating the timer only when use_tf_static is False would be a backportable change.

Copy link
Author

@wep21 wep21 Sep 23, 2021

Choose a reason for hiding this comment

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

@sloretz Fixed at 0919354

@wep21 wep21 requested a review from sloretz September 23, 2021 17:00
@wep21
Copy link
Author

wep21 commented Oct 1, 2021

@sloretz @clalancette friendly ping

@clalancette
Copy link
Contributor

clalancette commented Oct 1, 2021

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor

Failures on Windows CI are unrelated to this change. But since Windows is the most likely to have problems, I'm going to wait until Windows is fixed and re-run it before merging.

@clalancette clalancette merged commit 7dee973 into ros:galactic Oct 2, 2021
@wep21 wep21 deleted the fix/remove-timer branch October 3, 2021 06:10
@wep21
Copy link
Author

wep21 commented Nov 20, 2021

@clalancette Could you do bloom-release for galactic?

@clalancette
Copy link
Contributor

Done in ros/rosdistro#31336

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