Skip to content

avoid stale parameter events in content filter tests.#3085

Open
fujitatomoya wants to merge 1 commit intorollingfrom
fujitatomoya/issues/3065
Open

avoid stale parameter events in content filter tests.#3085
fujitatomoya wants to merge 1 commit intorollingfrom
fujitatomoya/issues/3065

Conversation

@fujitatomoya
Copy link
Collaborator

Description

Fixes #3065

Is this user-facing behavior change?

No

Did you use Generative AI?

No

Additional Information

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya fujitatomoya self-assigned this Feb 27, 2026
Copy link
Collaborator Author

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

@Barry-Xu-2018 can you review this? i confirmed that this passes for both connextdds and fastdds.

@fujitatomoya
Copy link
Collaborator Author

Pulls: #3085
Gist: https://gist.githubusercontent.com/fujitatomoya/e5a4f3d4bd65e848efdb903c0ad05227/raw/251145d0223fb1dadd5da5db5c2179e2e3a5b57b/ros2.repos
BUILD args: --packages-up-to rclcpp
TEST args: --packages-select rclcpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18330

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

// To make this test deterministic, we use an event-level callback that only
// inspects changed_parameters for the assertion flags, rather than relying on
// parameter-level callbacks which cannot distinguish new vs changed.
auto event_handler = param_handler->add_parameter_event_callback(cb_changed_event);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The original purpose of the test was to check the impact of content filter for add_parameter_callback().
Therefore, it is recommended to use add_parameter_callback() for test.
What do you think about making this change?

  const std::string remote_node1_param_name = "param_node1";
  const std::string remote_node2_param_name = "param_node2";
  remote_node1->declare_parameter(remote_node1_param_name, 10);
  remote_node2->declare_parameter(remote_node2_param_name, "Default");
 
+   // Add an explanation similar to the one in above test.
+  param_handler.reset(); 
+  param_handler = std::make_shared<TestParameterEventHandler>(node);
+ 

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.

rclcpp.TestNode.ConfigureNodesFilterAndCheckAddParameterEventCallback failure

2 participants