Skip to content

Conversation

@Ortisa
Copy link
Contributor

@Ortisa Ortisa commented Dec 4, 2025

This PR allows to generate MJCF at runtime and to be able to publish that on a separate topic.

The following features are added into this PR:

  • Generate MJCF and publish it over a topic using argument --publish_topic
  • Added --scene arg to be able to switch between scene at runtime
  • Support for defining input tags (raw_inputs and processed_inputs) directly in the URDF
  • Added -a / --asset_dir argument to specify the directory to reuse already generated objects from a given asset folder, otherwise generate them.
  • Option to also choose whether to save the model with argument -s

@saikishor
Copy link
Member

@eholum-nasa @ndunkelb-nasa when you get some time, please take a look at this PR. We will polish things by tomorrow, but good to have some initial insights 🙌🏽

@saikishor saikishor requested a review from eholum-nasa December 5, 2025 20:05
@Ortisa
Copy link
Contributor Author

Ortisa commented Dec 9, 2025

Grabacion.de.pantalla.desde.09-12-25.14.43.48.webm

This shows the model loading from the topic and generating the model on-the-fly from URDF

@Ortisa
Copy link
Contributor Author

Ortisa commented Dec 9, 2025

Grabacion.de.pantalla.desde.09-12-25.14.41.22.webm

If executed without reutilizing the assets, then due to decomposition you see that it takes longer to start the simulation

Copy link
Collaborator

@eholum-nasa eholum-nasa left a comment

Choose a reason for hiding this comment

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

FWIW I merged the trimesh changes into this branch just to check it out in jazzy any it worked! There are still functional things to worry about, like mesh colorings that bpy would just handle and trimesh doesn't seem to support. So a lot of grey in our converted MJCFs... But the changes in this particular PR seem very nice.

Added a few thoughts to the args from trying it out.

# Part inputs data
raw_inputs, processed_inputs = parse_inputs_xml(parsed_args.mujoco_inputs)
output_filepath = parsed_args.output
if not os.path.isabs(parsed_args.output) and parsed_args.publish_topic:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I understand this conditional? Why the check on if the topic is published?

Copy link
Member

@saikishor saikishor Dec 10, 2025

Choose a reason for hiding this comment

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

I'll be frank, we saw that there was a minor bug on the assets directory if you use both --publish-topic and --output tags and this is just a way to fix it. We didn't spend too much time there. I can also make them mutually exclusive if needed

saikishor and others added 2 commits December 10, 2025 19:40
Co-authored-by: Erik Holum <erik.holum@nasa.gov>
@saikishor
Copy link
Member

FWIW I merged the trimesh changes into this branch just to check it out in jazzy any it worked! There are still functional things to worry about, like mesh colorings that bpy would just handle and trimesh doesn't seem to support. So a lot of grey in our converted MJCFs... But the changes in this particular PR seem very nice.

@eholum-nasa I don't see the commits on the branch

@ndunkelb-nasa
Copy link
Collaborator

FWIW I merged the trimesh changes into this branch just to check it out in jazzy any it worked! There are still functional things to worry about, like mesh colorings that bpy would just handle and trimesh doesn't seem to support. So a lot of grey in our converted MJCFs... But the changes in this particular PR seem very nice.

@eholum-nasa I don't see the commits on the branch

Maybe @eholum-nasa missed the reference to this branch? Just wanted to put out there that I added some extra commits after @eholum-nasa made this comment that fixed mesh colorings (specifically from going from stl -> obj) and adding in handling daes that have textures that come from png or jpgs.

@saikishor
Copy link
Member

@ndunkelb-nasa this is great. Do you want to merge it into this PR? or shall we do it in a different PR?. Let me know. Depending on it, I can merge it right now

@saikishor
Copy link
Member

@ndunkelb-nasa Apart from that, I would like to hear from you what you think about the features in this PR?. Do you think it is interesting?

@ndunkelb-nasa
Copy link
Collaborator

@ndunkelb-nasa this is great. Do you want to merge it into this PR? or shall we do it in a different PR?. Let me know. Depending on it, I can merge it right now

I think I am happy to keep it separate bc there is already a bunch of focused discussion in this one. But I think after this goes in, we should certainly go ahead and do a PR for that as well. I would like someone to check my work as well!

@ndunkelb-nasa
Copy link
Collaborator

@ndunkelb-nasa Apart from that, I would like to hear from you what you think about the features in this PR?. Do you think it is interesting?

Absolutely! I think this is exactly the right path and makes it more baked into the ROS workflow instead of having to have a whole new path. The main thing I am struggling with is to figure out what exactly I need to run with something like the test robot to have a reproducible example... Would you be able to add a launch arg or something in there so that it is easy to test these features? I have tried a couple of things, but because things are just topic based now, there isn't a lot of errors that get thrown, sometimes things just crash if I haven't configured everything properly (which I clearly haven't).


robot_description = {"robot_description": ParameterValue(value=robot_description_content, value_type=str)}

converter_command = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

So what I thought I understood from the new code was that I could replace this with something like

    converter_process = Node(
        package="mujoco_ros2_simulation",
        executable="make_mjcf_from_robot_description.py",
        output="screen",
        arguments=[
            "--publish_topic",
            "mujoco_robot_description",
            "--scene",
            PathJoinSubstitution(
                [
                    FindPackageShare("mujoco_ros2_simulation"),
                    "test_resources",
                    "scene.xml",
                ]
            ),
        ],
    )

and I could remove the mujoco_model parameter from the urdf, and it should just work. However, when I do that, my ros2 control node crashes, but I do see the mujoco_robot_description_topic with the mujoco info.

What am I missing here?

Copy link
Member

Choose a reason for hiding this comment

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

I believe yes, something like that should already get it to work. Please give me a couple of minutes to check

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think im getting a failure on this line

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, I cannot test it rolling with bpy

[make_mjcf_from_robot_description-5] Traceback (most recent call last):
[make_mjcf_from_robot_description-5]   File "/home/user/ros2_control_ws/install/mujoco_ros2_simulation/share/mujoco_ros2_simulation/scripts/make_mjcf_from_robot_description.py", line 21, in <module>
[make_mjcf_from_robot_description-5]     import bpy
[make_mjcf_from_robot_description-5] ModuleNotFoundError: No module named 'bpy'
[ERROR] [make_mjcf_from_robot_description-5]: process has died [pid 8272, exit code 1, cmd 'python3 /home/user/ros2_control_ws/install/mujoco_ros2_simulation/share/mujoco_ros2_simulation/scripts/make_mjcf_from_robot_description.py'].

Shall we merge your changes into this as well? that should make things easier

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah maybe I just don't have the inputs there

Copy link
Member

Choose a reason for hiding this comment

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

Yes, your URDF description needs to have the URDF inputs and all

Copy link
Collaborator

@ndunkelb-nasa ndunkelb-nasa Dec 17, 2025

Choose a reason for hiding this comment

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

alright, I actually figured it out. It was missing the inputs and the typical scene like we have there needs to get rid of this line so it doesn't try to reference a file that no longer exists

I still think this PR could benefit from a working example of this workflow, because I think this is actually what everyone should be using instead of generating separate mjcf files.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, for now the idea is not to have any includes in the scene file. Just to not to overcomplicate things

@saikishor
Copy link
Member

@ndunkelb-nasa If there is any other thing, let me know. I'll be happy to address them

@ndunkelb-nasa
Copy link
Collaborator

@ndunkelb-nasa If there is any other thing, let me know. I'll be happy to address them

I think the last thing that would be useful would be to add a launch arg in test_robot.launch.py that shows an example of this. I think it would be good to show users how to use this workflow, because I think it should probably be the default way to use these drivers.

@saikishor
Copy link
Member

@ndunkelb-nasa If there is any other thing, let me know. I'll be happy to address them

I think the last thing that would be useful would be to add a launch arg in test_robot.launch.py that shows an example of this. I think it would be good to show users how to use this workflow, because I think it should probably be the default way to use these drivers.

Yess, I was about to do that, but the CI would fail due to the recently added tests. I will instead add that in the new PR of trimesh. Is that okay with you?

@ndunkelb-nasa
Copy link
Collaborator

@ndunkelb-nasa If there is any other thing, let me know. I'll be happy to address them

I think the last thing that would be useful would be to add a launch arg in test_robot.launch.py that shows an example of this. I think it would be good to show users how to use this workflow, because I think it should probably be the default way to use these drivers.

Yess, I was about to do that, but the CI would fail due to the recently added tests. I will instead add that in the new PR of trimesh. Is that okay with you?

Good with me! Seems like its time for us to update all of our workflows to use this as the default!

@saikishor
Copy link
Member

Good with me! Seems like its time for us to update all of our workflows to use this as the default!

I'm glad. Shall we wait for @eholum-nasa's approval too? or we proceed with merge?

@ndunkelb-nasa
Copy link
Collaborator

Good with me! Seems like its time for us to update all of our workflows to use this as the default!

I'm glad. Shall we wait for @eholum-nasa's approval too? or we proceed with merge?

I imagine he is good to go, but I think he will probably see this early tomorrow morning, so I think we might as well wait for him.

@saikishor
Copy link
Member

@ndunkelb-nasa, what's the best media to reach you for some important discussions?

@ndunkelb-nasa
Copy link
Collaborator

@ndunkelb-nasa, what's the best media to reach you for some important discussions?

I'm not sure I would have a good way at the moment. What would be normal for y'all? Discord?

@saikishor
Copy link
Member

I'm not sure I would have a good way at the moment. What would be normal for y'all? Discord?

We are connected via slack channel of ros-controls or I also use Discord with same username as github

@saikishor saikishor enabled auto-merge (squash) December 18, 2025 16:12
@saikishor saikishor disabled auto-merge December 18, 2025 16:13
@saikishor saikishor merged commit 3c19c9b into ros-controls:main Dec 18, 2025
5 checks passed
@saikishor saikishor deleted the feature_mjcf_generator_runtime branch December 18, 2025 16:13
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.

4 participants