-
Notifications
You must be signed in to change notification settings - Fork 9
[Feature] MJCF generation at runtime #18
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
[Feature] MJCF generation at runtime #18
Conversation
|
@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 🙌🏽 |
Grabacion.de.pantalla.desde.09-12-25.14.43.48.webmThis shows the model loading from the topic and generating the model on-the-fly from URDF |
Grabacion.de.pantalla.desde.09-12-25.14.41.22.webmIf executed without reutilizing the assets, then due to decomposition you see that it takes longer to start the simulation |
eholum-nasa
left a comment
There was a problem hiding this 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: |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
Co-authored-by: Erik Holum <erik.holum@nasa.gov>
@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 |
|
@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 |
|
@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? |
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! |
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 = [ |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
|
@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 |
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! |
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. |
|
@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? |
We are connected via slack channel of ros-controls or I also use Discord with same username as github |
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:
--publish_topic--scenearg to be able to switch between scene at runtimeraw_inputsandprocessed_inputs) directly in the URDF-a/--asset_dirargument to specify the directory to reuse already generated objects from a given asset folder, otherwise generate them.-s