Implement Goto Node, Extract Leaf Node from Action Node, CheckPoint Node is a Leaf #622

calvertdw · 2025-01-28T22:46:19Z

No description provided.

calvertdw · 2025-01-31T22:19:47Z

...-high-level-behaviors/src/libgdx/java/us/ihmc/rdx/ui/behavior/actions/RDXCheckPointNode.java

 import us.ihmc.tools.io.WorkspaceResourceDirectory;

-public class RDXCheckPointNode extends RDXActionNode<CheckPointNodeState, CheckPointNodeDefinition>
+public class RDXCheckPointNode extends RDXLeafNode<CheckPointNodeState, CheckPointNodeDefinition>


Checkpoint extends leaf (I wish checkpoint was spelled as one word, but whatever)

calvertdw · 2025-01-31T22:20:16Z

...vel-behaviors/src/libgdx/java/us/ihmc/rdx/ui/behavior/actions/RDXChestOrientationAction.java

@@ -159,7 +159,7 @@ public void render3DPanelImGuiOverlays()
   {
      if (isMouseHovering)
      {
-         tooltip.render("%s Action\nIndex: %d\nName: %s".formatted(getActionTypeTitle(), state.getActionIndex(), definition.getName()));
+         tooltip.render("%s Action\nIndex: %d\nName: %s".formatted(getLeafTypeTitle(), state.getLeafIndex(), definition.getName()));


Action -> Leaf almost everywhere

calvertdw · 2025-01-31T22:20:36Z

ihmc-high-level-behaviors/src/libgdx/java/us/ihmc/rdx/ui/behavior/logic/RDXGotoNode.java

 import us.ihmc.tools.io.WorkspaceResourceDirectory;

-public class RDXGotoNode extends RDXBehaviorTreeNode<GotoNodeState, GotoNodeDefinition>
+public class RDXGotoNode extends RDXLeafNode<GotoNodeState, GotoNodeDefinition>


Goto node is a leaf

calvertdw · 2025-01-31T22:21:53Z

ihmc-high-level-behaviors/src/libgdx/java/us/ihmc/rdx/ui/behavior/sequence/RDXActionNode.java


 /**
 * The UI representation of a robot behavior action. It provides a base
 * template for implementing an interactable action.
 */
 public abstract class RDXActionNode<S extends ActionNodeState<D>,
                                    D extends ActionNodeDefinition>
-      extends RDXBehaviorTreeNode<S, D>
+      extends RDXLeafNode<S, D>


Actions are leaves

calvertdw · 2025-01-31T22:22:22Z

...aviors/src/libgdx/java/us/ihmc/rdx/ui/behavior/sequence/RDXActionProgressWidgetsManager.java

@@ -17,7 +17,7 @@ public class RDXActionProgressWidgetsManager
 {
   private final ImGuiUniqueLabelMap labels = new ImGuiUniqueLabelMap(getClass());
   private final ImGuiLabelledWidgetAligner widgetAligner = new ImGuiLabelledWidgetAligner();
-   private final SortedSet<RDXActionNode<?, ?>> sortedActionNodesToRender = new TreeSet<>(Comparator.comparingInt(node -> node.getState().getActionIndex()));
+   private final SortedSet<RDXActionNode<?, ?>> sortedActionNodesToRender = new TreeSet<>(Comparator.comparingInt(node -> node.getState().getLeafIndex()));


Progress widgets still only render actions, because leaves don't take time

calvertdw · 2025-01-31T22:23:14Z

...gh-level-behaviors/src/libgdx/java/us/ihmc/rdx/ui/behavior/tree/RDXBehaviorTreeRootNode.java

Root node (sequence) deals primarily with leaves, not actions

calvertdw · 2025-01-31T22:24:31Z

ihmc-high-level-behaviors/src/main/java/us/ihmc/behaviors/ai2r/AI2RNodeExecutor.java

AI2R needs to be tested. I don't know how to do it. @LuigiPenco93?

we'd need a python example similar to what Dex has done for the scene graph

calvertdw · 2025-01-31T22:25:47Z

...vel-behaviors/src/main/java/us/ihmc/behaviors/behaviorTree/BehaviorTreeRootNodeExecutor.java

               {
-                  LogTools.warn("Actions failed, fallback actions are next for execution.");
+                  LogTools.warn("Leaves failed, fallback leaves are next for execution.");


calvertdw · 2025-01-31T22:26:28Z

...ehaviors/src/main/java/us/ihmc/behaviors/behaviorTree/ros2/ROS2BehaviorTreeMessageTools.java

-            subscriptionNode.setBehaviorTreeNodeStateMessage(gotoNodeStateMessage.getState());
-            subscriptionNode.setBehaviorTreeNodeDefinitionMessage(gotoNodeStateMessage.getDefinition().getDefinition());
+            subscriptionNode.setBehaviorTreeNodeStateMessage(gotoNodeStateMessage.getState().getState());
+            subscriptionNode.setBehaviorTreeNodeDefinitionMessage(gotoNodeStateMessage.getDefinition().getDefinition().getDefinition());


I actually don't know why these lines are here

calvertdw · 2025-01-31T22:27:03Z

ihmc-high-level-behaviors/src/main/java/us/ihmc/behaviors/logic/GotoNodeDefinition.java

+   private final CRDTBidirectionalBoolean gotoNext;
+   private final CRDTBidirectionalLong gotoNodeID;
+   /** We use this to save the goto node name to file instead of the number for human readability. */
+   private String gotoNodeName = GOTO_NEXT;


Very similar implementation to executeAfter

calvertdw · 2025-01-31T22:28:01Z

ihmc-high-level-behaviors/src/main/java/us/ihmc/behaviors/logic/GotoNodeState.java

+   @Override
+   public void validateFields(List<LeafNodeState<?>> leafNodes)
+   {
+      super.validateFields(leafNodes);


This validation step also validates the executeAfter fields

calvertdw · 2025-01-31T22:29:42Z

...interfaces/src/main/messages/ihmc_interfaces/behavior_msgs/msg/GotoNodeDefinitionMessage.msg

+bool goto_next
+
+# The ID of the node to goto
+uint32 goto_node_id


Goto node message fields

TomaszTB

Looks good. Left a couple of thoughts

TomaszTB · 2025-02-03T20:29:26Z

ihmc-high-level-behaviors/src/libgdx/java/us/ihmc/rdx/ui/behavior/sequence/RDXLeafNode.java

+   private final S state;
+   private final D definition;


A whole bunch of classes that extend the RDXBehaviorTreeNode and BehaviorTreeNodeExecutor also store the state and definition, which they get from the super class... What about making the state and definition fields protected? Not in this PR though. Just a thought.

That's actually a great idea, I'll add a JIRA ticket

TomaszTB · 2025-02-03T20:37:16Z

...vel-behaviors/src/main/java/us/ihmc/behaviors/behaviorTree/BehaviorTreeRootNodeExecutor.java

 //         int j = i + 1;
-//         for (; j < state.getActionChildren().size()
-//              && state.getActionChildren().get(j).calculateExecuteAfterActionIndex(state.getActionChildren()) < i; j++);
+//         for (; j < state.getLeafChildren().size()
+//              && state.getLeafChildren().get(j).calculateExecuteAfterLeafIndex(state.getLeafChildren()) < i; j++);


Is there a reason to keep this commented code?

ds58 · 2025-02-03T23:22:31Z

ihmc-high-level-behaviors/src/main/java/us/ihmc/behaviors/sequence/LeafNodeState.java

+      this.leafIndex = leafIndex;
+   }
+
+   public int getLeafIndex()


Needs javadoc, I wouldn't have any clue what a leaf index is from this

ds58 · 2025-02-03T23:22:46Z

ihmc-high-level-behaviors/src/main/java/us/ihmc/behaviors/sequence/LeafNodeState.java

+      return isExecuting.getValue();
+   }
+
+   public int calculateExecuteAfterLeafIndex()


Needs javadoc, what does this mean and when do you call it?

Added doc and changed to getExecuteAfterLeafIndex. I named it calculate to warn that it's not a basic getter, but I don't want to cache the calculation

ds58 · 2025-02-03T23:22:52Z

ihmc-high-level-behaviors/src/main/java/us/ihmc/behaviors/sequence/LeafNodeState.java

+      return leafIndex - 1; // previous
+   }
+
+   public LeafNodeState<?> findExecuteAfterLeaf()


Needs javadoc, what does this mean and when do you call it?

Added doc and changed to getExecuteAfterLeaf. I named it find to warn that it's not a basic getter, but I don't want to cache the calculation

ds58 · 2025-02-03T23:23:54Z

ihmc-high-level-behaviors/src/main/java/us/ihmc/behaviors/sequence/LeafNodeExecutor.java

+      this.state = state;
+   }
+
+   public String getCantExecuteMessage()


getCantExecuteMessage() -> getExecuteFailureMessage() or something

This is a message to print when the canExecute pre-condition is false (i.e. the action is not valid to even attempt execution) I've added doc.

ds58 · 2025-02-03T23:25:48Z

ihmc-high-level-behaviors/src/libgdx/java/us/ihmc/rdx/ui/behavior/sequence/RDXLeafNode.java

+   private final ImGuiFlashingColors isExecutingFlashingColor = new ImGuiFlashingColors(0.1, ImGuiTools.PURPLE, ImGuiTools.DARK_PURPLE);
+   private final ImGuiHollowArrowRenderer hollowArrowRenderer = new ImGuiHollowArrowRenderer();
+   private final ImGuiFlashingText flashingDescriptionColor = new ImGuiFlashingText(ImGuiTools.RED);
+   private boolean wasFailed = false;


Can you add a comment about what wasFailed represents? e.g. Used in update() for sending notifications

ds58 · 2025-02-03T23:28:30Z

...-level-behaviors/src/main/java/us/ihmc/behaviors/behaviorTree/BehaviorTreeRootNodeState.java

@@ -92,24 +97,24 @@ public void fromMessage(BehaviorTreeRootNodeStateMessage message)
   }

   @Nullable
-   public <T extends ActionNodeState<?>> T findNextPreviousAction(Class<T> actionClass, int queryIndex, @Nullable RobotSide side)
+   public <T extends LeafNodeState<?>> T findNextPreviousLeaf(Class<T> leafClass, int queryIndex, @Nullable RobotSide side)


Out of scope for this PR but I still really do not like the name findNextPreviousLeaf

…d implement most of goto node.

calvertdw force-pushed the feature/goto-node branch 2 times, most recently from d65f7d1 to 25c8bc9 Compare January 31, 2025 21:22

calvertdw changed the title ~~Implement behavior goto node~~ Implement Goto Node, Extract Leaf Node from Action Node, CheckPoint Node is a Leaf Jan 31, 2025

calvertdw commented Jan 31, 2025

View reviewed changes

calvertdw marked this pull request as ready for review January 31, 2025 22:29

calvertdw requested review from ds58, TomaszTB and LuigiPenco93 January 31, 2025 22:30

calvertdw commented Jan 31, 2025

View reviewed changes

calvertdw force-pushed the feature/goto-node branch from 2a9af87 to 83069a8 Compare February 3, 2025 18:34

TomaszTB approved these changes Feb 3, 2025

View reviewed changes

ds58 requested changes Feb 3, 2025

View reviewed changes

calvertdw force-pushed the feature/goto-node branch from 8eeaf8e to a3d3d16 Compare February 4, 2025 16:25

ds58 approved these changes Feb 4, 2025

View reviewed changes

calvertdw and others added 10 commits February 5, 2025 17:09

Get started on goto node.

4294019

Implement some of goto node state. TODO: Implement combo box.

f9ddaaf

Extract leaf node from action node in ROS 2 messages.

3c0c64a

Make checkpoint extend leaf instead of action in ROS 2 messages.

66f8da0

Extract leaf node as common part of action, checkpoint, goto nodes an…

2c7a6f3

…d implement most of goto node.

Complete goto node draft implementation.

af4e756

action -> leaf where applicable

c0d1d28

Fix up goto node.

95f7854

Remove commented code.

6fc920d

Add documentation, rename a few methods.

ab7c8e8

calvertdw force-pushed the feature/goto-node branch from a3d3d16 to ab7c8e8 Compare February 5, 2025 23:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Goto Node, Extract Leaf Node from Action Node, CheckPoint Node is a Leaf #622

Implement Goto Node, Extract Leaf Node from Action Node, CheckPoint Node is a Leaf #622

calvertdw commented Jan 28, 2025

calvertdw Jan 31, 2025

calvertdw Jan 31, 2025

calvertdw Jan 31, 2025

calvertdw Jan 31, 2025

calvertdw Jan 31, 2025

calvertdw Jan 31, 2025

calvertdw Jan 31, 2025

LuigiPenco93 Feb 3, 2025 •

edited

Loading

calvertdw Jan 31, 2025

calvertdw Jan 31, 2025

calvertdw Jan 31, 2025

calvertdw Jan 31, 2025

calvertdw Jan 31, 2025

TomaszTB left a comment

TomaszTB Feb 3, 2025

calvertdw Feb 3, 2025

TomaszTB Feb 3, 2025

calvertdw Feb 3, 2025

ds58 Feb 3, 2025

calvertdw Feb 4, 2025

ds58 Feb 3, 2025

calvertdw Feb 4, 2025

ds58 Feb 3, 2025

calvertdw Feb 4, 2025

ds58 Feb 3, 2025

calvertdw Feb 4, 2025

ds58 Feb 3, 2025

calvertdw Feb 4, 2025

ds58 Feb 3, 2025

Implement Goto Node, Extract Leaf Node from Action Node, CheckPoint Node is a Leaf #622

Are you sure you want to change the base?

Implement Goto Node, Extract Leaf Node from Action Node, CheckPoint Node is a Leaf #622

Conversation

calvertdw commented Jan 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LuigiPenco93 Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomaszTB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LuigiPenco93 Feb 3, 2025 •

edited

Loading