-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Bt warning fix #5594
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
base: main
Are you sure you want to change the base?
Bt warning fix #5594
Conversation
2be7daf
to
c48f8ee
Compare
Codecov Report❌ Patch coverage is
... and 9 files with indirect coverage changes 🚀 New features to boost your workflow:
|
nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp
Outdated
Show resolved
Hide resolved
nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp
Outdated
Show resolved
Hide resolved
nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp
Outdated
Show resolved
Hide resolved
nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp
Outdated
Show resolved
Hide resolved
nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp
Outdated
Show resolved
Hide resolved
nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull in main / rebase in order for CI to pass
nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp
Outdated
Show resolved
Hide resolved
nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Jad El Hajj <[email protected]>
Signed-off-by: Jad El Hajj <[email protected]>
Signed-off-by: Jad El Hajj <[email protected]>
Signed-off-by: Jad El Hajj <[email protected]>
Signed-off-by: Jad El Hajj <[email protected]>
Signed-off-by: Jad El Hajj <[email protected]>
Signed-off-by: Jad El Hajj <[email protected]>
Signed-off-by: Jad El Hajj <[email protected]>
…_impl.hpp Signed-off-by: Steve Macenski <[email protected]>
Signed-off-by: Jad El Hajj <[email protected]>
Signed-off-by: Jad El Hajj <[email protected]>
Signed-off-by: Jad El Hajj <[email protected]>
ef5192b
to
4a62ece
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now. Did you test this again that it works for the cases we had issue with before?
current_bt_id.c_str(), entry.path().string().c_str()); | ||
if (registered_ids.count(id)) { | ||
RCLCPP_WARN(logger_, "Skipping conflicting BT file %s (duplicate ID %s)", | ||
entry.path().c_str(), id.c_str()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indent error.
nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp
Outdated
Show resolved
Hide resolved
nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp
Outdated
Show resolved
Hide resolved
…_impl.hpp Signed-off-by: Steve Macenski <[email protected]>
…_impl.hpp Signed-off-by: Steve Macenski <[email protected]>
Basic Info
Description of contribution in a few bullet points
A fix PR making sure that BTCPP warning would never be logged. The final logic will be:
Description of how this change was tested
PS: codecov might complain about not covered code in
bt_action_server_impl.hpp
, but I added unit test for all 4 conditions. So the mock loadBehaviorTree intest_behavior_tree_node.cpp
will be covering the new changes.For Maintainers:
backport-*
.