Skip to content

fix: Node level lazy parameter fix#852

Open
TannerGilbert wants to merge 1 commit intogazebosim:ros2from
TannerGilbert:feat/lazy_parameter_fix
Open

fix: Node level lazy parameter fix#852
TannerGilbert wants to merge 1 commit intogazebosim:ros2from
TannerGilbert:feat/lazy_parameter_fix

Conversation

@TannerGilbert
Copy link

🦟 Bug fix

Fixes #751

Summary

Fix the lazy ROS parameter on the parameter_bridge and RosGzBridge nodes so it actually works as a global default for all bridge topics.

What was broken:

  • parameter_bridge.cpp declared and read the lazy parameter but never applied it to bridge configs — it was a dead variable
  • RosGzBridge (used for YAML config and parameter-based bridges) never declared a lazy parameter at all, so YAML-loaded bridges always defaulted to lazy: false

What this PR does:

  • Changes BridgeConfig::is_lazy from bool to std::optional so we can distinguish "not set" from "explicitly set to false"
  • The node-level lazy parameter now serves as the default for all bridges that don't explicitly set lazy per-entry in YAML
  • Per-entry lazy: true or lazy: false in YAML overrides the node-level default
  • Applies lazy to CLI-specified bridges in parameter_bridge as well

Behaviour:

Node-level lazy Per-entry YAML lazy Resolved value
false (default) not set false
false (default) true true
false (default) false false
true not set true
true true true
true false false

Checklist

  • Signed all commits for DCO
  • Added a screen capture or video to the PR description that demonstrates the fix (as needed)
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • Updated Bazel files (if adding new files). Created an issue otherwise.
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Generated-by" to your commits. (See this policy for more info.)

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.

Signed-off-by: Tanner, Gilbert <gilbertta@edu.aau.at>
EXPECT_EQ(
"Could not parse config: file empty [test/config/empty.yaml]",
g_last_log_event.message);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
}
// Verify that BridgeHandle::IsLazy() falls back to kDefaultLazy when
// BridgeConfig::is_lazy is std::nullopt, and returns the explicit value
// when it is set.
TEST(BridgeHandleIsLazy, NulloptFallsBackToDefault)
{
ros_gz_bridge::BridgeConfig config;
config.is_lazy = std::nullopt;
// IsLazy() must return the hard-coded default when no value is set.
EXPECT_EQ(ros_gz_bridge::kDefaultLazy, config.is_lazy.value_or(ros_gz_bridge::kDefaultLazy));
}
TEST(BridgeHandleIsLazy, ExplicitTrueIsRespected)
{
ros_gz_bridge::BridgeConfig config;
config.is_lazy = true;
EXPECT_TRUE(config.is_lazy.value_or(ros_gz_bridge::kDefaultLazy));
}
TEST(BridgeHandleIsLazy, ExplicitFalseIsRespected)
{
ros_gz_bridge::BridgeConfig config;
config.is_lazy = false;
EXPECT_FALSE(config.is_lazy.value_or(ros_gz_bridge::kDefaultLazy));
}

Comment on lines 240 to 242
void RosGzBridge::add_bridge(const BridgeConfig & config)
{
bool gz_to_ros = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
void RosGzBridge::add_bridge(const BridgeConfig & input_config)
{
// Resolve the laziness: if the caller left is_lazy as nullopt, inherit the
// node-level "lazy" parameter so that the effective value is always explicit.
BridgeConfig config = input_config;
if (!config.is_lazy.has_value()) {
bool node_lazy = kDefaultLazy;
this->get_parameter("lazy", node_lazy);
config.is_lazy = node_lazy;
}
bool gz_to_ros = false;

@github-project-automation github-project-automation bot moved this from Inbox to In review in Core development Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

ros_gz_bridge: The global lazy parameter is not applied

2 participants