Adds RTXLidar #2308
Adds RTXLidar #2308
Conversation
|
Hi @jtigue-bdai, thanks for the great work on this! I pulled in your branch to work on adding MDP support for the RTX Lidar. Had two questions:
Thanks! |
|
Hey @samirchowd thanks for that suggestion. "sensor.data.output['data']" is gonna look pretty weird. Yeah I think point_cloud is a good choice. I can make the point cloud change. As for the mdp terms I'm not sure you can make a pr to this branch since I moved it out of the IsaacSim/isaaclab into a private fork. If you can go for it. Depending on how much you were thinking of adding, one thing I could do is make an empty diff in the mdp files you were planning to work on and then you can add them to this as a suggestion and I can add you to the contributors list. If there are a lot of additions then it would be easier as a PR separately. |
|
To limit the scope of the PR, I can raise it in a separate PR once this is merged in that case. I think it'll keep your development and testing simpler. |
jtigue-bdai
requested review from
Mayankm96,
jsmith-bdai,
pascal-roth,
kellyguo11 and
hhansen-bdai
as code owners
|
Hey @jtigue-bdai, any chance of this getting reviewed soon? Would be more than happy to help out any way 😄 Thanks! |
Signed-off-by: James Tigue <[email protected]>
| if not carb_settings_iface.get("/isaaclab/cameras_enabled"): | ||
| raise RuntimeError( | ||
| "A camera was spawned without the --enable_cameras flag. Please use --enable_cameras to enable" | ||
| " rendering." | ||
| ) |
There was a problem hiding this comment.
shouldn't that not be RTXLiDAR in this case?
| lidar_prim = stage.GetPrimAtPath(lidar_prim_path) | ||
| # Check if prim is a camera | ||
| if not lidar_prim.IsA(UsdGeom.Camera): | ||
| raise RuntimeError(f"Prim at path '{lidar_prim_path}' is not a Camera.") |
There was a problem hiding this comment.
That error might be hard to interpret, maybe we add something extra for the LiDAR?
| raise RuntimeError(f"Prim at path '{lidar_prim_path}' is not a Camera.") | |
| raise RuntimeError(f"Prim at path '{lidar_prim_path}' is not a Camera which is the base prim class for the RTXLiDAR") |
| for name in self.cfg.optional_data_types: | ||
| if name == "azimuth": | ||
| init_params["outputAzimuth"] = True | ||
| elif name == "elevation": | ||
| init_params["outputElevation"] = True | ||
| elif name == "normal": | ||
| init_params["outputNormal"] = True | ||
| elif name == "velocity": | ||
| init_params["outputVelocity"] = True | ||
| elif name == "beamId": | ||
| init_params["outputBeamId"] = True | ||
| elif name == "emitterId": | ||
| init_params["outputEmitterId"] = True | ||
| elif name == "materialId": | ||
| init_params["outputMaterialId"] = True | ||
| elif name == "objectId": | ||
| init_params["outputObjectId"] = True |
There was a problem hiding this comment.
codewise this feels a bit ugly, could we not directly have the config param as a dict:
cfg.optional_data_types = {"outputAzimuth": False, "outputElevation": False, ...}
and then in the notes link to the description which are available. In that way, we are more robust against code changes on the RTX implementation side
| init_params = { | ||
| "outputAzimuth": False, | ||
| "outputElevation": False, | ||
| "outputNormal": False, | ||
| "outputVelocity": False, | ||
| "outputBeamId": False, | ||
| "outputEmitterId": False, | ||
| "outputMaterialId": False, | ||
| "outputObjectId": False, | ||
| "outputTimestamp": True, # always turn on timestamp field | ||
| } |
There was a problem hiding this comment.
then this would be the default of the config (see the comment below)
| # process data for different segmentation types | ||
| # Note: Replicator returns raw buffers of dtype int32 for segmentation types | ||
| # so we need to convert them to uint8 4 channel images for colorized types |
There was a problem hiding this comment.
why is that comment here?
| def _process_annotator_output(self, name: str, output: Any) -> tuple[torch.tensor, dict | None]: | ||
| """Process the annotator output. | ||
|
|
||
| This function is called after the data has been collected from all the cameras. |
There was a problem hiding this comment.
| This function is called after the data has been collected from all the cameras. | |
| This function is called after the data has been collected from all the RTXLiDARs. |
| Please refer to the :class:'RtxLidar' and :class:'RtxLidarData' for a list and description of available data types. | ||
| """ | ||
| data_frame: Literal["world", "sensor"] = "world" | ||
| """The frame to represent the output.data. |
There was a problem hiding this comment.
| """The frame to represent the output.data. | |
| """The frame to represent the :attr:`RtxLidar.data` in. |
Can we use this parameters for sphinx indexing? Please check again
| spawn: LidarCfg = MISSING | ||
| """Spawn configuration for the asset. | ||
|
|
||
| If None, then the prim is not spawned by the asset. Instead, it is assumed that the | ||
| asset is already present in the scene. | ||
| """ |
There was a problem hiding this comment.
given the docstring, it would be allowed to be set it to None, so the default should be None, otherwise the configclass check errors out
| spawn: LidarCfg = MISSING | |
| """Spawn configuration for the asset. | |
| If None, then the prim is not spawned by the asset. Instead, it is assumed that the | |
| asset is already present in the scene. | |
| """ | |
| spawn: LidarCfg | None = None | |
| """Spawn configuration for the asset. | |
| If None, then the prim is not spawned by the asset. Instead, it is assumed that the | |
| asset is already present in the scene. | |
| """ |
| RTX_LIDAR_INFO_FIELDS = { | ||
| "numChannels": int, | ||
| "numEchos": int, | ||
| "numReturnsPerScan": int, | ||
| "renderProductPath": str, | ||
| "ticksPerScan": int, | ||
| "transform": torch.Tensor, | ||
| } |
There was a problem hiding this comment.
brainstorming here but would it make sense to have this directly as the default value for the info param below?
Description
This PR creates a sensor based on the RTX Lidar:
Fixes ##865
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there