URDFTools can take a Collection<InputStream> as the first parameters to allow for splitting the URDF up into multiple files #172

PotatoPeeler3000 · 2024-03-12T19:18:45Z

Initial attempt at getting the URDF to be split up, it loads properly. There are errors all over the place still

…. There are errors all over the place still

…'s with duplicate joints for testing

PotatoPeeler3000

Gonna look into what should be done about the name, don't like that we ignore the rest of the names after getting the first one

scs2-examples/src/test/java/us/ihmc/scs2/examples/urdf/ComposedURDFLoadingTest.java

scs2-examples/src/main/resources/urdf/limbA.urdf

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

scs2-session-logger/src/main/java/us/ihmc/scs2/session/tools/RobotModelLoader.java

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

scs2-examples/src/test/java/us/ihmc/scs2/examples/urdf/SimpleCrossFourBarRobotTest.java

scs2-session-logger/src/main/java/us/ihmc/scs2/session/tools/RobotModelLoader.java

…es to be more accurate. Removed method that isn't need anymore

…t components are prioritized

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

scs2-examples/src/test/java/us/ihmc/scs2/examples/urdf/SimpleCrossFourBarRobotTest.java

SylvainBertrand · 2024-04-05T15:17:47Z

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

-         if (!parserProperties.parseSensors)
-            urdfModel.getGazebos().removeIf(gazebo -> gazebo.getSensor() != null);
+      // If the model1 doesn't have any links that are in the model2, we define the model2 to be the parent
+      return mergeChildIntoParentModel(model2, model1);


I'd suggest inlining the mergeChildIntoParentModel method in here, as it now relies solving the parent-child relationship outside making it vulnerable to idiot bugs 😅
Above, I'd make 2 new local variables URDFModel parentModel and URDFModel childModel and resolve them, then after you should be able to inline the mergeChildIntoParentModel only once at the end of this method.

True, that was able to happen, just had to set the parent or child model to the correct model and after that has been done, at the end collect all the joints from the parent and child.

…'t be a parent and a child, so should only go into the correct if statement

SylvainBertrand

The main is keeping backward compatibility. The other comment about not instantiating the models is just subjective style.

SylvainBertrand · 2024-04-05T20:44:44Z

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

+      }
+
+      URDFModel parentModel = new URDFModel();
+      URDFModel childModel = new URDFModel();


No need to instantiate new models, just needed more references, these could be initialized to null

Done, yep true, should know that by now dang

SylvainBertrand · 2024-04-05T20:46:05Z

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

@@ -209,66 +247,256 @@ public static URDFModel loadURDFModel(InputStream inputStream, Collection<String
    *                            done.
    * @return the model.
    */
-   public static URDFModel loadURDFModel(InputStream inputStream,
+   public static URDFModel loadURDFModel(Collection<InputStream> inputStreams,
                                         Collection<String> resourceDirectories,
                                         ClassLoader resourceClassLoader,
                                         URDFParserProperties parserProperties) throws JAXBException


Let's add another factory for backward compatibility with the signature:
URDFModel loadURDFModel(InputStream inputStream, Collection<String> resourceDirectories, ClassLoader resourceClassLoader, URDFParserProperties parserProperties)

We have this... lets see if I can link it....

Couldn't link it but its on line 210

PotatoPeeler3000 · 2024-04-05T14:49:34Z

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

+      {
+         for (URDFJoint joint : model2.getJoints())
+         {
+            // Check if the model1 has any links that are referenced in model2


Its possible for a link from Model1 to be contained inside either the parent or child joint in model2, depending on which it is, that makes it either the child or parent

PotatoPeeler3000 · 2024-04-05T14:50:02Z

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

+      {
+         for (URDFJoint joint : model1.getJoints())
+         {
+            // Check if the model2 has any links that are referenced in model1


We have to check this for both models, if we get unlucky with the way the models are passed in and don't check both ways, the name may be wrong

PotatoPeeler3000 · 2024-04-05T20:30:00Z

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

-         if (!parserProperties.parseSensors)
-            urdfModel.getGazebos().removeIf(gazebo -> gazebo.getSensor() != null);
+      // If the model1 doesn't have any links that are in the model2, we define the model2 to be the parent
+      return mergeChildIntoParentModel(model2, model1);


True, that was able to happen, just had to set the parent or child model to the correct model and after that has been done, at the end collect all the joints from the parent and child.

PotatoPeeler3000 · 2024-04-05T20:50:03Z

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

+      }
+
+      URDFModel parentModel = new URDFModel();
+      URDFModel childModel = new URDFModel();


Done, yep true, should know that by now dang

PotatoPeeler3000 · 2024-04-05T20:51:21Z

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

@@ -209,66 +247,256 @@ public static URDFModel loadURDFModel(InputStream inputStream, Collection<String
    *                            done.
    * @return the model.
    */
-   public static URDFModel loadURDFModel(InputStream inputStream,
+   public static URDFModel loadURDFModel(Collection<InputStream> inputStreams,
                                         Collection<String> resourceDirectories,
                                         ClassLoader resourceClassLoader,
                                         URDFParserProperties parserProperties) throws JAXBException


We have this... lets see if I can link it....

PotatoPeeler3000 · 2024-04-05T20:52:06Z

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

+    *                            done.
+    * @return the model.
+    */
+   public static URDFModel loadURDFModel(InputStream inputStream, Collection<String> resourceDirectories, ClassLoader resourceClassLoader, URDFParserProperties parserProperties) throws JAXBException


This is what your looking for yes? @SylvainBertrand This is the factory you asked for, its just hidden in the mess haha

SylvainBertrand · 2024-04-05T21:05:39Z

scs2-definition/src/main/java/us/ihmc/scs2/definition/robot/urdf/URDFTools.java

+    *                            done.
+    * @return the model.
+    */
+   public static URDFModel loadURDFModel(InputStream inputStream, Collection<String> resourceDirectories, ClassLoader resourceClassLoader, URDFParserProperties parserProperties) throws JAXBException


Initial attempt at getting the URDF to be split up, it loads properly…

26bcbdf

…. There are errors all over the place still

PotatoPeeler3000 marked this pull request as draft March 12, 2024 19:19

PotatoPeeler3000 and others added 8 commits March 12, 2024 15:17

Added Gazebo models so the URDF is loaded properly

2dfe0e0

Fix breaking tests

ecdc44e

Fix merging all the gazebos

04ce97e

Added fix for handleing implicit joint definitions

0b4374f

Files cleanup because AI isn't the brightest

3559164

Added tests for loading multiple URDF's, created a couple simple URDF…

024de25

…'s with duplicate joints for testing

Prevent loading duplicate joints by checking if they already exist

2414126

Updated tests, added a few more, prints are more clear

8ea0cd2

PotatoPeeler3000 self-assigned this Mar 22, 2024

PotatoPeeler3000 added the enhancement New feature or request label Mar 22, 2024

PotatoPeeler3000 commented Mar 22, 2024

View reviewed changes

PotatoPeeler3000 marked this pull request as ready for review March 22, 2024 15:56

Nadia Operator and others added 3 commits March 23, 2024 10:34

More testing on real robot, still have lots of problems with logger

429fe06

Got working while logging, need to go through and combine commits

28958c7

Tested loading a real log without the composed robot

f8b5db4

PotatoPeeler3000 marked this pull request as draft March 25, 2024 20:39

More comments

c052427

PotatoPeeler3000 commented Mar 25, 2024

View reviewed changes

PotatoPeeler3000 mentioned this pull request Mar 26, 2024

Added method to pack the models as a byte array to be sent over the network and received on the client side ihmcrobotics/ihmc-robot-data-logger#6

Closed

PotatoPeeler3000 marked this pull request as ready for review March 26, 2024 14:46

SylvainBertrand requested changes Apr 1, 2024

View reviewed changes

PotatoPeeler3000 and others added 5 commits April 1, 2024 21:56

Addressed PR comments. Added more documentation, updated variable nam…

444d3a5

…es to be more accurate. Removed method that isn't need anymore

Added method to check if the model is a root model, added docs

7c369cb

Update tests to check that the correct name is gotten

4e70560

Change method to take in a parent and a child, and the name and paren…

0c1eb00

…t components are prioritized

Update RobotModelLoader.java to before PR

c2b182e

PotatoPeeler3000 commented Apr 2, 2024

View reviewed changes

PotatoPeeler3000 requested a review from SylvainBertrand April 2, 2024 20:34

PotatoPeeler3000 and others added 3 commits April 5, 2024 08:48

Save still needs work

f301869

Fix tests, add better doc for them

47e9a82

Fix comments

8f2c61b

SylvainBertrand requested changes Apr 5, 2024

View reviewed changes

PotatoPeeler3000 added 2 commits April 5, 2024 15:22

Inline method, add new constructor to allow for more options

f2b98ca

Don't need to worry about parent model being set because the URDF can…

223f9bd

…'t be a parent and a child, so should only go into the correct if statement

SylvainBertrand requested changes Apr 5, 2024

View reviewed changes

Don't initialize the variable, set to null just needed for reference

e818a89

PotatoPeeler3000 commented Apr 5, 2024

View reviewed changes

SylvainBertrand approved these changes Apr 5, 2024

View reviewed changes

PotatoPeeler3000 merged commit 61516ec into develop Apr 8, 2024
3 checks passed

PotatoPeeler3000 deleted the feature/combined-urdf branch April 8, 2024 13:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

URDFTools can take a Collection<InputStream> as the first parameters to allow for splitting the URDF up into multiple files #172

URDFTools can take a Collection<InputStream> as the first parameters to allow for splitting the URDF up into multiple files #172

PotatoPeeler3000 commented Mar 12, 2024

PotatoPeeler3000 left a comment

SylvainBertrand Apr 5, 2024

PotatoPeeler3000 Apr 5, 2024

SylvainBertrand left a comment

SylvainBertrand Apr 5, 2024

PotatoPeeler3000 Apr 5, 2024

SylvainBertrand Apr 5, 2024

PotatoPeeler3000 Apr 5, 2024

PotatoPeeler3000 Apr 5, 2024

PotatoPeeler3000 Apr 5, 2024

PotatoPeeler3000 Apr 5, 2024

PotatoPeeler3000 Apr 5, 2024

PotatoPeeler3000 Apr 5, 2024

PotatoPeeler3000 Apr 5, 2024

PotatoPeeler3000 Apr 5, 2024

SylvainBertrand Apr 5, 2024

SylvainBertrand Apr 5, 2024

URDFTools can take a Collection<InputStream> as the first parameters to allow for splitting the URDF up into multiple files #172

URDFTools can take a Collection<InputStream> as the first parameters to allow for splitting the URDF up into multiple files #172

Conversation

PotatoPeeler3000 commented Mar 12, 2024

PotatoPeeler3000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SylvainBertrand 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