Sphere and initial object velocity#479
Conversation
viiik-inside
left a comment
viiik-inside
left a comment
There was a problem hiding this comment.
looks great!! Can you also add a test to see if the object moves with the given velocity if its possible.
isaaclab_arena/assets/object_base.py
Outdated
| self.object_cfg.init_state.rot = initial_pose.rotation_wxyz | ||
| self.event_cfg = self._init_event_cfg() | ||
|
|
||
| def set_initial_velocity(self, linear_velocity: tuple[float, float, float]) -> None: |
There was a problem hiding this comment.
maybe set_initial_linear_velocity is better
| linear_velocity: Linear velocity ``(vx, vy, vz)`` in the world frame. | ||
| """ | ||
| self.initial_velocity = linear_velocity | ||
| if self.object_cfg is not None and hasattr(self.object_cfg.init_state, "lin_vel"): |
There was a problem hiding this comment.
are these two conditions possible to be false?
There was a problem hiding this comment.
Yes, because self.object_cfg can exist but not have "lin_vel" attribure (e.g. the walls). Only articulated and rigid objects have it
isaaclab_arena/assets/object_base.py
Outdated
| "pose": initial_pose, | ||
| "asset_cfg": SceneEntityCfg(self.name), | ||
| } | ||
| if self.initial_velocity is not None: |
There was a problem hiding this comment.
can we anyway set the velocity to none. So we wont need this check. I see None is being handled in the event function
|
|
||
|
|
||
| @register_asset | ||
| class Sphere(LibraryObject): |
alexmillane
left a comment
alexmillane
left a comment
There was a problem hiding this comment.
Looks great.
I have a couple of small nits, and one larger comment that we'll probably have to sort out later.
| rigid_props=sim_utils.RigidBodyPropertiesCfg( | ||
| solver_position_iteration_count=16, | ||
| solver_velocity_iteration_count=1, | ||
| max_angular_velocity=1000.0, | ||
| max_linear_velocity=1000.0, | ||
| max_depenetration_velocity=5.0, | ||
| disable_gravity=False, | ||
| ), | ||
| mass_props=sim_utils.MassPropertiesCfg(mass=0.25), | ||
| ) |
There was a problem hiding this comment.
Is there some reason we need to change these RigidBodyPropertiesCfg. Could we leave them at the default values?
| def _generate_spawner_cfg(self) -> RigidObjectCfg: | ||
| object_cfg = RigidObjectCfg( | ||
| prim_path=self.prim_path, | ||
| spawn=self.spawner_cfg, | ||
| ) | ||
| object_cfg = self._add_initial_pose_to_cfg(object_cfg) | ||
| if self.initial_velocity is not None: | ||
| object_cfg.init_state.lin_vel = self.initial_velocity | ||
| return object_cfg | ||
|
|
||
| def _requires_reset_pose_event(self) -> bool: | ||
| return self.get_initial_pose() is not None and self.reset_pose |
There was a problem hiding this comment.
This is a bit messy. We don't want to define these functions per object in the library.
The problem you're trying to get around here is that the spawner object type only supports AssetBaseCfg and not RigidObjectCfg.
I think we need to solve this at a level higher in the heirachy though. Solving this per-specific library object is going to lead to a lot of duplication in the future.
I think we need to separate "does something provide a custom spawner?" from "what type of object is this?". Right now they are coupled together through ObjectType.
| no_gravity_cfg = sim_utils.SphereCfg( | ||
| radius=0.1, | ||
| visual_material=sim_utils.PreviewSurfaceCfg(diffuse_color=(0.8, 0.2, 0.2)), | ||
| collision_props=sim_utils.CollisionPropertiesCfg(), | ||
| rigid_props=sim_utils.RigidBodyPropertiesCfg( | ||
| solver_position_iteration_count=16, | ||
| solver_velocity_iteration_count=1, | ||
| max_angular_velocity=1000.0, | ||
| max_linear_velocity=1000.0, | ||
| max_depenetration_velocity=5.0, | ||
| disable_gravity=True, | ||
| ), | ||
| mass_props=sim_utils.MassPropertiesCfg(mass=0.25), | ||
| ) | ||
| sphere = asset_registry.get_asset_by_name("sphere")(spawner_cfg=no_gravity_cfg) |
There was a problem hiding this comment.
You shouldn't need to copy all these configs just to turn the gravity off. I think you should be able to access the particular property as an attribute.
sphere.spawner_cfg.rigid_props.disable_gravity=True
That should work (and if it doesn't we have a problem).
| displacement[0].item() < -OBJECT_VELOCITY_MIN_DISPLACEMENT # - x because initial velocity is in -x | ||
| ), f"Object did not move with given velocity: displacement in x was {displacement[0].item()}" |
There was a problem hiding this comment.
This seems like a bit of a loose test. Shouldn't we be able to bound this amount more tightly? Shouldn't the amount moved be roughly equal to the num_steps * time_per_step * velocity?
3 participants
Summary
New sphere in object library and ObjectBase has now optional initial velocity
Detailed description
Now it can be spawn spheres, and objects like the sphere can be initialized with linear velocity.