Skip to content

Commit

Permalink
MPPI: Do not set params success in dynamic params callback to allow c…
Browse files Browse the repository at this point in the history
…hanging params in other plugins (#4908)

* remove result success from dyn param handler

Signed-off-by: pepisg <[email protected]>

* remove reason handling

Signed-off-by: pepisg <[email protected]>

* update tests

Signed-off-by: pepisg <[email protected]>

---------

Signed-off-by: pepisg <[email protected]>
  • Loading branch information
pepisg authored Feb 10, 2025
1 parent f1cbaa4 commit 5c763e2
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 16 deletions.
6 changes: 0 additions & 6 deletions nav2_mppi_controller/src/parameters_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,6 @@ ParametersHandler::dynamicParamsCallback(
callback != get_param_callbacks_.end())
{
callback->second(param, result);
} else {
result.successful = false;
if (!result.reason.empty()) {
result.reason += "\n";
}
result.reason += "get_param_callback func for '" + param_name + "' not found.\n";
}
}

Expand Down
10 changes: 0 additions & 10 deletions nav2_mppi_controller/test/parameter_handler_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,6 @@ TEST(ParameterHandlerTest, DynamicAndStaticParametersNotDeclaredTest)
EXPECT_EQ(result.successful, true);
EXPECT_TRUE(result.reason.empty());

// Try to access some parameters that have not been declared
int p1 = 0, p2 = 0;
EXPECT_THROW(getParameter(p1, "not_declared", 8, ParameterType::Dynamic),
rclcpp::exceptions::InvalidParameterValueException);
EXPECT_THROW(getParameter(p2, "not_declared2", 9, ParameterType::Static),
rclcpp::exceptions::InvalidParameterValueException);

// Try to set some parameters that have not been declared via the service client
result_future = rec_param->set_parameters_atomically({
rclcpp::Parameter("static_int", 10),
Expand All @@ -292,7 +285,4 @@ TEST(ParameterHandlerTest, DynamicAndStaticParametersNotDeclaredTest)
// The ParameterNotDeclaredException handler in rclcpp/parameter_service.cpp
// overrides any other reasons and does not provide details to the service client.
EXPECT_EQ(result.reason, std::string("One or more parameters were not declared before setting"));

EXPECT_EQ(p1, 0);
EXPECT_EQ(p2, 0);
}

0 comments on commit 5c763e2

Please sign in to comment.