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

Fix part controller demo #546

Conversation

youliangtan
Copy link
Member

Summary

This addresses both of these issues related to the controller.

  • Demo scripts bugs #534

  • Bug when update to 1.5.0 #532

  • This PR added the previous v1.4 controller configs, so users can still initiate only a single controller (as opposed to composite), via the load_part_controller_config()` method.

  • Also there are some inconsistency of using body_parts in the internal implementation of handling controller_configs, I kept it consistent with the body_parts_controller_configs key. Still, I not a fan of dynamically changing the config dict in the internal method.

  • fixes the demo_control.py and tune_joints.py scripts.

What to Run

 python python robosuite/demos/demo_control.py
 python robosuite/scripts/tune_joints.py 

Also all test cases passed.

Signed-off-by: You Liang Tan <[email protected]>
Signed-off-by: You Liang Tan <[email protected]>
Signed-off-by: You Liang Tan <[email protected]>
Copy link
Member

@Abhiram824 Abhiram824 left a comment

Choose a reason for hiding this comment

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

Looks good overall. I think theres some extraneous code in the commit that can be removed though.

@kevin-thankyou-lin kevin-thankyou-lin mentioned this pull request Nov 5, 2024
robosuite/controllers/config/parts/default_baxter.json Outdated Show resolved Hide resolved
robosuite/controllers/config/parts/default_ur5e.json Outdated Show resolved Hide resolved
robosuite/demos/demo_control.py Outdated Show resolved Hide resolved
robosuite/demos/demo_control.py Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

do we need to change the key from body_parts to body_parts_controller_config I think if don't do this. This PR will have less changes

Copy link
Contributor

Choose a reason for hiding this comment

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

I think updating is fine for consistency?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think updating is fine for consistency?

@kevin-thankyou-lin kevin-thankyou-lin merged commit 0926cbe into ARISE-Initiative:master Nov 7, 2024
2 checks passed
@kevin-thankyou-lin
Copy link
Contributor

Resolves issue #534

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.

3 participants