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

Push Environments #88

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Push Environments #88

wants to merge 2 commits into from

Conversation

palanc
Copy link
Collaborator

@palanc palanc commented May 1, 2023

  1. Update existing push env to use cartesian action space
  2. Add new pushing envs
  3. All pushing envs use robotiq gripper. See additional PR here for gripper updates: Additional joint constraints robotiq_sim#1

Copy link
Owner

@vikashplus vikashplus left a comment

Choose a reason for hiding this comment

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

This seems like a larger refactor. See comments inline

@@ -1,7 +1,7 @@
{
# device1: sensors, actuators
'franka':{
'interface': {'type': 'franka', 'ip_address':'169.254.163.91'},
'interface': {'type': 'franka', 'ip_address':'172.16.0.1', 'gain_scale': 1.5},
Copy link
Owner

Choose a reason for hiding this comment

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

Scale can only be a max of 1.0

<include file="../../../../simhive/scene_sim/topfloor_scene.xml"/>
<include file="../../../../simhive/franka_sim/assets/assets.xml"/>
<include file="../../../../simhive/franka_sim/assets/gripper_assets.xml"/>
<include file="../../../../simhive/robotiq_sim/assets/assets.xml"/>
Copy link
Owner

Choose a reason for hiding this comment

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

Note that this is franka robot. Adding RobotiQ will cause confusion. We should create a new file for this and name it appropriately.

@@ -25,6 +30,27 @@ class PushBaseV0(env_base.MujocoEnv):

def __init__(self, model_path, obsd_model_path=None, seed=None, **kwargs):
Copy link
Owner

Choose a reason for hiding this comment

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

Currently it looks like, the original env is being replaced by this new IK based env. Right? I think the right way to implement this is to

  1. define a new act mode called IK
  2. Once the robot receives ctrl_desired in the step, it relies on the process-actuator call to do the translation from ee space to jnt space.
  3. I also think the action space definitions needs to be updated if its IK space

In summary, landing this in a generic way seems to require a rather larger refactor (possibly involving a refactor of the robot class). Let me know if should discuss this before going down this route.

@vikashplus vikashplus self-assigned this Sep 6, 2023
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