From efde8ac0f6e264c9111788433e7e493e78fb59d9 Mon Sep 17 00:00:00 2001 From: Andreev Kirill Andreevich Date: Wed, 7 Oct 2026 01:09:10 +0300 Subject: [PATCH 1/3] Fix joint trajectory controller missing keys in setup assistant (#3434) Signed-off-by: Andreev Kirill Andreevich --- .../src/moveit_controllers_config.cpp | 18 ++++++++++ .../test/test_controllers.cpp | 35 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp b/moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp index 3b89ec6112..0b8c30efab 100644 --- a/moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp +++ b/moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp @@ -188,6 +188,20 @@ bool MoveItControllersConfig::GeneratedControllersConfig::writeYaml(YAML::Emitte { emitter << YAML::Key << "type" << YAML::Value << controller.type_; + if (controller.type_ == "FollowJointTrajectory") + { + auto action_ns_it = controller.parameters_.find("action_ns"); + if (action_ns_it == controller.parameters_.end() || action_ns_it->second.empty()) + { + emitter << YAML::Key << "action_ns" << YAML::Value << "follow_joint_trajectory"; + } + auto default_it = controller.parameters_.find("default"); + if (default_it == controller.parameters_.end() || default_it->second.empty()) + { + emitter << YAML::Key << "default" << YAML::Value << "true"; + } + } + // Write joints emitter << YAML::Key << "joints"; emitter << YAML::Value; @@ -202,6 +216,10 @@ bool MoveItControllersConfig::GeneratedControllersConfig::writeYaml(YAML::Emitte for (const auto& pair : controller.parameters_) { + if (pair.second.empty()) + { + continue; + } emitter << YAML::Key << pair.first; emitter << YAML::Value << pair.second; } diff --git a/moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp b/moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp index 64b6e114ff..3c79362ecc 100644 --- a/moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp +++ b/moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp @@ -175,6 +175,41 @@ TEST_F(ControllersTest, AddDefaultControllers) EXPECT_EQ(ros2_controllers_config->getControllers().size(), group_count); } + +TEST_F(ControllersTest, OutputMoveItControllersFix3314) +{ + config_data_->preloadWithFullConfig("moveit_resources_fanuc_moveit_config"); + auto moveit_controllers = config_data_->get("moveit_controllers"); + + std::vector& mcontrollers = moveit_controllers->getControllers(); + ASSERT_EQ(1u, mcontrollers.size()); + + // Set parameters to empty to simulate empty text boxes in UI + mcontrollers[0].parameters_["action_ns"] = ""; + mcontrollers[0].parameters_["default"] = ""; + + generateFiles("moveit_controllers"); + + YAML::Node generated = YAML::LoadFile(output_dir_ / "config/moveit_controllers.yaml"); + const YAML::Node& c_node = generated["moveit_simple_controller_manager"]["fanuc_controller"]; + ASSERT_TRUE(c_node["action_ns"]) << "action_ns missing"; + EXPECT_EQ(c_node["action_ns"].as(), "follow_joint_trajectory"); + ASSERT_TRUE(c_node["default"]) << "default missing"; + // The value is "true" in config string + EXPECT_EQ(c_node["default"].as(), "true"); + + // Now try with specific value to ensure no duplication/overwriting with default + mcontrollers[0].parameters_["action_ns"] = "custom_ns"; + mcontrollers[0].parameters_["default"] = "false"; + generateFiles("moveit_controllers"); + YAML::Node generated2 = YAML::LoadFile(output_dir_ / "config/moveit_controllers.yaml"); + const YAML::Node& c_node2 = generated2["moveit_simple_controller_manager"]["fanuc_controller"]; + ASSERT_TRUE(c_node2["action_ns"]); + EXPECT_EQ(c_node2["action_ns"].as(), "custom_ns"); + ASSERT_TRUE(c_node2["default"]); + EXPECT_EQ(c_node2["default"].as(), "false"); +} + int main(int argc, char** argv) { testing::InitGoogleTest(&argc, argv); From 49549cf8ab4ea8eb090c249a55f26cee2704b175 Mon Sep 17 00:00:00 2001 From: Andreev Kirill Andreevich Date: Wed, 7 Oct 2026 01:13:30 +0300 Subject: [PATCH 2/3] Fix logic for missing action_ns and default in setup assistant Signed-off-by: Andreev Kirill Andreevich --- .../src/moveit_controllers_config.cpp | 10 ++------ .../test/test_controllers.cpp | 23 ++++++++++++------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp b/moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp index 0b8c30efab..50efc0223b 100644 --- a/moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp +++ b/moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp @@ -190,13 +190,11 @@ bool MoveItControllersConfig::GeneratedControllersConfig::writeYaml(YAML::Emitte if (controller.type_ == "FollowJointTrajectory") { - auto action_ns_it = controller.parameters_.find("action_ns"); - if (action_ns_it == controller.parameters_.end() || action_ns_it->second.empty()) + if (controller.parameters_.find("action_ns") == controller.parameters_.end()) { emitter << YAML::Key << "action_ns" << YAML::Value << "follow_joint_trajectory"; } - auto default_it = controller.parameters_.find("default"); - if (default_it == controller.parameters_.end() || default_it->second.empty()) + if (controller.parameters_.find("default") == controller.parameters_.end()) { emitter << YAML::Key << "default" << YAML::Value << "true"; } @@ -216,10 +214,6 @@ bool MoveItControllersConfig::GeneratedControllersConfig::writeYaml(YAML::Emitte for (const auto& pair : controller.parameters_) { - if (pair.second.empty()) - { - continue; - } emitter << YAML::Key << pair.first; emitter << YAML::Value << pair.second; } diff --git a/moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp b/moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp index 3c79362ecc..37a86fd167 100644 --- a/moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp +++ b/moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp @@ -175,18 +175,17 @@ TEST_F(ControllersTest, AddDefaultControllers) EXPECT_EQ(ros2_controllers_config->getControllers().size(), group_count); } - TEST_F(ControllersTest, OutputMoveItControllersFix3314) { config_data_->preloadWithFullConfig("moveit_resources_fanuc_moveit_config"); auto moveit_controllers = config_data_->get("moveit_controllers"); - + std::vector& mcontrollers = moveit_controllers->getControllers(); ASSERT_EQ(1u, mcontrollers.size()); - - // Set parameters to empty to simulate empty text boxes in UI - mcontrollers[0].parameters_["action_ns"] = ""; - mcontrollers[0].parameters_["default"] = ""; + + // Erase parameters to simulate them being absent + mcontrollers[0].parameters_.erase("action_ns"); + mcontrollers[0].parameters_.erase("default"); generateFiles("moveit_controllers"); @@ -195,9 +194,17 @@ TEST_F(ControllersTest, OutputMoveItControllersFix3314) ASSERT_TRUE(c_node["action_ns"]) << "action_ns missing"; EXPECT_EQ(c_node["action_ns"].as(), "follow_joint_trajectory"); ASSERT_TRUE(c_node["default"]) << "default missing"; - // The value is "true" in config string EXPECT_EQ(c_node["default"].as(), "true"); - + + // Also check empty parameters + mcontrollers[0].parameters_["action_ns"] = ""; + mcontrollers[0].parameters_["default"] = ""; + generateFiles("moveit_controllers"); + YAML::Node generated_empty = YAML::LoadFile(output_dir_ / "config/moveit_controllers.yaml"); + const YAML::Node& c_node_empty = generated_empty["moveit_simple_controller_manager"]["fanuc_controller"]; + EXPECT_EQ(c_node_empty["action_ns"].as(), ""); + EXPECT_EQ(c_node_empty["default"].as(), ""); + // Now try with specific value to ensure no duplication/overwriting with default mcontrollers[0].parameters_["action_ns"] = "custom_ns"; mcontrollers[0].parameters_["default"] = "false"; From 5dab790138221158d5e7eefab9b242c732c473ce Mon Sep 17 00:00:00 2001 From: Andreev Kirill Andreevich Date: Wed, 7 Oct 2026 01:19:58 +0300 Subject: [PATCH 3/3] Omit empty non-action_ns parameters to fix bool parsing --- .../src/moveit_controllers_config.cpp | 4 ++++ .../moveit_setup_controllers/test/test_controllers.cpp | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp b/moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp index 50efc0223b..ac25e70443 100644 --- a/moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp +++ b/moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp @@ -214,6 +214,10 @@ bool MoveItControllersConfig::GeneratedControllersConfig::writeYaml(YAML::Emitte for (const auto& pair : controller.parameters_) { + if (pair.second.empty() && pair.first != "action_ns") + { + continue; + } emitter << YAML::Key << pair.first; emitter << YAML::Value << pair.second; } diff --git a/moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp b/moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp index 37a86fd167..f75974508c 100644 --- a/moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp +++ b/moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp @@ -196,14 +196,14 @@ TEST_F(ControllersTest, OutputMoveItControllersFix3314) ASSERT_TRUE(c_node["default"]) << "default missing"; EXPECT_EQ(c_node["default"].as(), "true"); - // Also check empty parameters + // Also check empty parameters: default should be omitted, action_ns should be kept as empty string mcontrollers[0].parameters_["action_ns"] = ""; mcontrollers[0].parameters_["default"] = ""; generateFiles("moveit_controllers"); YAML::Node generated_empty = YAML::LoadFile(output_dir_ / "config/moveit_controllers.yaml"); const YAML::Node& c_node_empty = generated_empty["moveit_simple_controller_manager"]["fanuc_controller"]; EXPECT_EQ(c_node_empty["action_ns"].as(), ""); - EXPECT_EQ(c_node_empty["default"].as(), ""); + EXPECT_FALSE(c_node_empty["default"]); // Now try with specific value to ensure no duplication/overwriting with default mcontrollers[0].parameters_["action_ns"] = "custom_ns";