From c4b23ea5a3c3f69dd01dee06b11c9a1cf87dd4f4 Mon Sep 17 00:00:00 2001
From: Johnny 5 <johnny5@picknik.ai>
Date: Tue, 1 Sep 2026 17:00:34 +0000
Subject: [PATCH 1/2] fix: preserve empty subtree model defaults

A SubTree model port declared with default="" was treated as mandatory, because the instantiation check tested the default's string form instead of whether a default was declared. Gate on the Any, as the model writer and XSD generator already do.

Backport of d665d411 from #29 (the 4.9.0 port) onto main so it can ship in MoveIt Pro 10.1.1. Fixes PickNikRobotics/moveit_pro#22069.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 src/xml_parsing.cpp     |  2 +-
 tests/gtest_subtree.cpp | 56 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp
index 15a045713..de0732142 100644
--- a/src/xml_parsing.cpp
+++ b/src/xml_parsing.cpp
@@ -789,7 +789,7 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
         if(it == port_remap.end() && !do_autoremap)
         {
           // remapping is not explicitly defined in the XML: use the model
-          if(port_info.defaultValueString().empty())
+          if(port_info.defaultValue().empty())
           {
             auto msg = StrCat("In the <TreeNodesModel> the <Subtree ID=\"", type_ID,
                               "\"> is defining a mandatory port called [", port_name,
diff --git a/tests/gtest_subtree.cpp b/tests/gtest_subtree.cpp
index 0bf48f8ae..7cf074413 100644
--- a/tests/gtest_subtree.cpp
+++ b/tests/gtest_subtree.cpp
@@ -732,6 +732,62 @@ TEST(SubTree, WhitespaceInSubtreeModel)
   FAIL() << "Exception was not thrown.";
 }
 
+TEST(SubTree, EmptyModelDefaultIsNotMandatory)
+{
+  struct TestCase
+  {
+    const char* name;
+    const char* port_model;
+    const char* invocation_attributes;
+    const char* expected_value;
+    bool should_build;
+  };
+
+  const TestCase test_cases[] = {
+    { "empty default, not remapped", R"(<inout_port name="note" default=""/>)", "", "",
+      true },
+    { "empty default, explicitly empty", R"(<inout_port name="note" default=""/>)",
+      R"( note="")", "", true },
+    { "non-empty default, not remapped",
+      R"(<inout_port name="note" default="something"/>)", "", "something", true },
+    { "no default, not remapped", R"(<inout_port name="note"/>)", "", "", false },
+  };
+
+  for(const auto& test_case : test_cases)
+  {
+    SCOPED_TRACE(test_case.name);
+    const auto xml_text = StrCat(
+        R"(<root main_tree_to_execute="MainTree" BTCPP_format="4">
+  <TreeNodesModel>
+    <SubTree ID="Child">)",
+        test_case.port_model,
+        R"(</SubTree>
+  </TreeNodesModel>
+  <BehaviorTree ID="MainTree">
+    <SubTree ID="Child")",
+        test_case.invocation_attributes,
+        R"(/>
+  </BehaviorTree>
+  <BehaviorTree ID="Child">
+    <ScriptCondition code="note==')",
+        test_case.expected_value,
+        R"('"/>
+  </BehaviorTree>
+</root>)");
+
+    BehaviorTreeFactory factory;
+    if(test_case.should_build)
+    {
+      auto tree = factory.createTreeFromText(xml_text);
+      EXPECT_EQ(tree.tickWhileRunning(), NodeStatus::SUCCESS);
+    }
+    else
+    {
+      EXPECT_THROW(factory.createTreeFromText(xml_text), RuntimeError);
+    }
+  }
+}
+
 class PrintToConsole : public BT::SyncActionNode
 {
 public:

From b61a3989bf35cdcc10c00694dcf2660902c0eb68 Mon Sep 17 00:00:00 2001
From: David Vadovszki <david.vadovszki@picknik.ai>
Date: Fri, 25 Sep 2026 15:21:28 -0600
Subject: [PATCH 2/2] test: name the failing case without SCOPED_TRACE

Under pixi's Windows job, the two tests that use SCOPED_TRACE crash with 0xc0000005: this one and Reactive.MissingOrEmptyGenericChildIdIsRejected, which #33 changes the same way. Stream the case name into each assertion instead, and wrap the build in ASSERT_NO_THROW so an unexpected throw still names its case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 tests/gtest_subtree.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/gtest_subtree.cpp b/tests/gtest_subtree.cpp
index 7cf074413..29f5b01a3 100644
--- a/tests/gtest_subtree.cpp
+++ b/tests/gtest_subtree.cpp
@@ -755,7 +755,6 @@ TEST(SubTree, EmptyModelDefaultIsNotMandatory)
 
   for(const auto& test_case : test_cases)
   {
-    SCOPED_TRACE(test_case.name);
     const auto xml_text = StrCat(
         R"(<root main_tree_to_execute="MainTree" BTCPP_format="4">
   <TreeNodesModel>
@@ -778,12 +777,13 @@ TEST(SubTree, EmptyModelDefaultIsNotMandatory)
     BehaviorTreeFactory factory;
     if(test_case.should_build)
     {
-      auto tree = factory.createTreeFromText(xml_text);
-      EXPECT_EQ(tree.tickWhileRunning(), NodeStatus::SUCCESS);
+      Tree tree;
+      ASSERT_NO_THROW(tree = factory.createTreeFromText(xml_text)) << test_case.name;
+      EXPECT_EQ(tree.tickWhileRunning(), NodeStatus::SUCCESS) << test_case.name;
     }
     else
     {
-      EXPECT_THROW(factory.createTreeFromText(xml_text), RuntimeError);
+      EXPECT_THROW(factory.createTreeFromText(xml_text), RuntimeError) << test_case.name;
     }
   }
 }