Fix deprecation warnings for upcoming ROS Galactic release #631
Conversation
If we are constructing from nanoseconds we must be explicit about the units. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
If a default value is not provided, then we must specify the parameter type. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
| @@ -604,7 +604,7 @@ void RosFilter<T>::integrateMeasurements(const rclcpp::Time & current_time) | |||
| const std::string first_measurement_topic = | |||
| first_measurement->topic_name_; | |||
| // revertTo may invalidate first_measurement | |||
| if (!revertTo(first_measurement_time - rclcpp::Duration(1))) { | |||
| if (!revertTo(first_measurement_time - rclcpp::Duration(1ns))) { | |||
There was a problem hiding this comment.
@ayrton04 should this be 1ns, 1s, ...?
There was a problem hiding this comment.
Thanks, I meant to ask the same question.
There was a problem hiding this comment.
If I understand the equivalent ROS 1 code, it is also subtracting 1 nanosecond:
robot_localization/src/ros_filter.cpp
Line 611 in a2da33b
There was a problem hiding this comment.
So the PRs involved here are
https://github.com/cra-ros-pkg/robot_localization/pull/266/files
and
https://github.com/cra-ros-pkg/robot_localization/pull/279/files
The only reason we subtract any time at all is to ensure that we go to a measurement (presumably the first) that is before first_measurement_time. But we could also just change this to use >=:
robot_localization/src/ros_filter.cpp
Lines 3322 to 3323 in 3326e80
If I had to guess (I didn't dig too far), the measurement times were likely stored as doubles originally, and the 1e-9 thing was just me being careful with floating point numbers. But ROS times have exact representations, so I think the 1e-9 could go, and we can change the revertTo reverse iteration to check for >= instead of just >.
There was a problem hiding this comment.
However, that change could be outside of scope for this. It was meant to be 1 nanosecond, yes.
|
@jacobperron I opened a |
|
Hold up, there's more warnings in other files I've overlooked. |
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Okay, I think I got them all. |
|
Otherwise LGTM, waiting for Tom's response on that one comment |
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
When there is a NoParameterOverrideProvided exception, the parameter is not declared so we don't need to undeclare it. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
|
@SteveMacenski, feel free to merge. I know you said LGTM, but your review still had comments requested, so I re-requested your review. |
Try-catches were added in #631 due to a new rclcpp feature enforcing static parameter. The behavior was later patched for this particular use-case in ros2/rclcpp#1673, so now we can avoid having to try-catch. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Try-catches were added in #631 due to a new rclcpp feature enforcing static parameter. The behavior was later patched for this particular use-case in ros2/rclcpp#1673, so now we can avoid having to try-catch. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Add missing message_filters dependency (#666) Headers from message_filters are included here: https://github.com/cra-ros-pkg/robot_localization/blob/67098c2341b5d1ccbcceb8eede60e79db74814a6/include/robot_localization/ros_robot_localization_listener.h\#L41-L42 Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Remove try-catch blocks around declare_parameter (#663) Try-catches were added in #631 due to a new rclcpp feature enforcing static parameter. The behavior was later patched for this particular use-case in ros2/rclcpp#1673, so now we can avoid having to try-catch. Signed-off-by: Jacob Perron <jacob@openrobotics.org> Co-authored-by: Jacob Perron <jacob@openrobotics.org>
* Remove try-catch blocks around declare_parameter (#663) Try-catches were added in #631 due to a new rclcpp feature enforcing static parameter. The behavior was later patched for this particular use-case in ros2/rclcpp#1673, so now we can avoid having to try-catch. Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Add missing message_filters dependency (#666) Headers from message_filters are included here: https://github.com/cra-ros-pkg/robot_localization/blob/67098c2341b5d1ccbcceb8eede60e79db74814a6/include/robot_localization/ros_robot_localization_listener.h\#L41-L42 Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Revert "Enable QoS overrides (#657)" This reverts commit 2816e92. Co-authored-by: Jacob Perron <jacob@openrobotics.org>
Building against the latest version of rclcpp produces some deprecation warnings. This PR fixes warnings related to
rclcpp::Durationconstruction (6b36efd) andrclcpp::Node::declare_parameter(7055b82).Related tickets upstream, deprecating usage:
The second commit (7055b82) related to declare_parameter is not compatible with Foxy, so I recommend creating a new
galactic-develbranch and we can retarget this PR.