Skip to content
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

Fixes to SMURFA io and conversion #373

Merged
merged 16 commits into from
Mar 11, 2025

Conversation

m0rsch1
Copy link
Contributor

@m0rsch1 m0rsch1 commented Feb 6, 2025

Description

The complete IO handling and processing of Arrangements is currently not working correctly.
A SMURFA needs to also export its entities to be reimportable and therefore processable by e.g.
phobos/scripts/convert.py

Related Issue

#367

Motivation and Context

I am currently working on creating a SMURFA from a database backend.
Because the SMURFA handling is not working, the inner parts and assemblies are not correctly converted to a top-level model (aka SMURF)

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

…her func

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
…te of self.model

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
…n given a dict with x,y,z keys and values

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
@m0rsch1
Copy link
Contributor Author

m0rsch1 commented Feb 6, 2025

@hwiedPro @AlpenAalAlex I have created a draft pull request. Please have a look if u like.

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
This function does not properly assemble a consistent URDF/SMURF from an
Arrangement!

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
…or proper export

Also, interface renaming has to be done according to root_entity only

This runs through but my example produces a weird URDF (which makes MARS crash)

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
…ctory conversion

(which is a SMURFA to SMURF conversion actually)

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
@m0rsch1
Copy link
Contributor Author

m0rsch1 commented Feb 13, 2025

@hwiedPro @AlpenAalAlex so far the smurfa generation and conversion to smurf works pipeline wise. However the transformations i produce or which are produced seem to be broken somehow. I also have Entities in there which get modified by exchange_root(). In what state are Arrangement::assemble() and Robot::exchange_root()? Are these well-tested or have to be treated as being experimental?

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
To be tested

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
@m0rsch1
Copy link
Contributor Author

m0rsch1 commented Feb 18, 2025

Status update: Robot::exchange_root() was not producing correct results.
This has been fixed now.

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
…nsformed

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
@m0rsch1 m0rsch1 marked this pull request as ready for review February 20, 2025 08:35
@AlpenAalAlex
Copy link
Collaborator

Thank you so much. I will take a look

@AlpenAalAlex
Copy link
Collaborator

In what state are Arrangement::assemble() and Robot::exchange_root()? Are these well-tested or have to be treated as being experimental?

Experimental I assume

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
robot = world.assemble()
else:
robot = Robot(inputfile=args.input)

# Check results
if not world and not not robot:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "not not" intended? I think for robots multiple.py:201 has to be

anchor=anchor,
world=self

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was not intended. Fixed it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "not not" intended? I think for robots multiple.py:201 has to be

anchor=anchor,
world=self

I don't know if that is correct. Haven't touched this.

@AlpenAalAlex AlpenAalAlex merged commit b835223 into dfki-ric:pre_v2.1.0 Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants