Updated TiledCamera references to Camera in downstream configs#5377
Updated TiledCamera references to Camera in downstream configs#5377bdilinila wants to merge 9 commits intoisaac-sim:developfrom
Conversation
github-actions
Bot
added
documentation
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR is a continuation of #5162, performing a straightforward renaming of TiledCamera → Camera and TiledCameraCfg → CameraCfg across downstream configuration files, scripts, and documentation. The changes are purely mechanical namespace updates with no logic changes. The renaming appears consistent and complete across all 30 files touched.
Architecture Impact
Self-contained. This is a downstream cleanup following the unification of TiledCamera and Camera classes in the parent PR #5162. The changes only update references in:
- Task environment configurations (cartpole, shadow_hand, dexsuite, stack, pick_place)
- Demo and benchmark scripts
- Environment base classes (comment updates only)
- Contrib sensor implementations (visuotactile sensor)
- Test files
No API contracts are changed — the Camera and CameraCfg classes already exist and support all the functionality previously provided by TiledCamera/TiledCameraCfg.
Implementation Verdict
Ship it — this is a clean mechanical refactor with consistent application across the codebase.
Test Coverage
The PR updates test files appropriately:
test_visuotactile_sensor.py— updatedTiledCameraCfg→CameraCfgimports and usagestest_shadow_hand_vision_presets.py— updated docstring references
No new tests are needed as this is a renaming exercise. The existing tests will validate the renamed references work correctly.
CI Status
No CI checks available yet — recommend waiting for CI to pass before merging.
Findings
🔵 Improvement: source/isaaclab_tasks/isaaclab_tasks/direct/cartpole/cartpole_camera_presets_env_cfg.py:30-51 — Inconsistent naming convention
The inner class CartpoleCameraCfg is defined but then used to create preset instances. While functional, consider whether this class should have a leading underscore to indicate it's internal (like _ShadowHandBaseCameraCfg in shadow_hand_vision_env_cfg.py), for consistency across the codebase.
🔵 Improvement: scripts/demos/sensors/cameras.py:244,258 — Subtitle strings could be more descriptive
The subtitles were changed from "TiledCamera" to "Camera", which is correct but loses information about which camera implementation is being compared. Since both are now Camera, the subtitles could indicate the instance name (e.g., "camera" vs "tiled_camera") to maintain distinction in the visualization output:
subtitles=["camera", "tiled_camera"], # matches scene entity names🔵 Improvement: source/isaaclab_tasks/isaaclab_tasks/direct/cartpole_showcase/cartpole_camera/cartpole_camera_env_cfg.py:12 — Function name could be updated
The function get_tiled_camera_cfg still contains "tiled" in its name despite now returning a CameraCfg. For consistency, consider renaming to get_camera_cfg:
def get_camera_cfg(data_type: str, width: int = 100, height: int = 100) -> CameraCfg:🔵 Improvement: source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/dexsuite/config/kuka_allegro/camera_cfg.py:55,95 — Class name could be clearer
The preset classes are named BaseCameraCfg and WristCameraCfg, but these names might be confused with the actual CameraCfg class. The original names BaseTiledCameraCfg and WristTiledCameraCfg distinguished them as preset containers. Consider names like BaseCameraPresetCfg and WristCameraPresetCfg to maintain clarity.
🔵 Improvement: source/isaaclab_contrib/docs/README.md:288 — Example code comment could be more precise
The comment on line 288 says "Camera configuration (must match render_cfg dimensions)" but doesn't explain what render_cfg is. A brief note about GELSIGHT_R15_CFG providing the render configuration would improve documentation clarity.
bdilinila
requested review from
Mayankm96,
jtigue-bdai,
kellyguo11,
ooctipus and
pascal-roth
as code owners
bdilinila
changed the title
Greptile SummaryThis PR continues the migration from PR #5162, replacing all remaining Confidence Score: 5/5Safe to merge; all remaining findings are P2 style suggestions about stale 'tiled' naming that do not affect runtime behavior. All changes are correct mechanical renames. No logic changes, no missing migrations that would cause runtime errors. Two leftover 'tiled' names in function identifiers and a variable attribute are style-only issues. scripts/benchmarks/benchmark_cameras.py (function names still use 'tiled'), source/isaaclab_tasks/isaaclab_tasks/direct/cartpole/cartpole_camera_env.py (self._tiled_camera variable) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[TiledCamera / TiledCameraCfg] -->|Deprecated| B[Camera / CameraCfg]
subgraph PR5162["PR #5162 (previous)"]
C[Core sensor class merged]
end
subgraph PR5377["This PR — downstream config migration"]
D[Env configs\ncartpole, shadow_hand,\ndexsuite, stack, nutpour]
E[Demo scripts\ncameras.py, tacsl_sensor.py]
F[Benchmark scripts\nbenchmark_cameras.py]
G[Contrib sensors\nVisuoTactileSensor]
H[Env base classes\ndirect_rl_env, marl_env,\nmanager_based_env comments]
end
PR5162 --> PR5377
B --> D
B --> E
B --> F
B --> G
B --> H
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
LGTM question: Will this PR update the docs, if not we can make a note to update them in a later PR. |
|
|
||
| @configclass | ||
| class MultiDataTypeCartpoleTiledCameraCfg(PresetCfg): | ||
| class MultiDataTypeCartpoleCameraCfg(PresetCfg): |
There was a problem hiding this comment.
Public symbol rename without a deprecation alias, any downstream import of MultiDataTypeCartpoleTiledCameraCfg will break silently on next pull.
maybe we should call this out as a breaking change
There was a problem hiding this comment.
Perhaps we should only rename references to TiledCamera and not the actual environment configs? Let's discuss on Monday.
|
|
||
|
|
||
| @configclass | ||
| class ShadowHandVisionTiledCameraCfg(PresetCfg): |
There was a problem hiding this comment.
nvsekkin
left a comment
nvsekkin
left a comment
There was a problem hiding this comment.
same comment for the following changes:
MultiDataTypeCartpoleTiledCameraCfg → MultiDataTypeCartpoleCameraCfg
ShadowHandVisionTiledCameraCfg → ShadowHandVisionCameraCfg
BaseTiledCameraCfg → BaseCameraPresetCfg
WristTiledCameraCfg → WristCameraPresetCfg
we may want to add them as deprecated in case anyone is using them
| @configclass | ||
| class BaseTiledCameraCfg(PresetCfg): | ||
| """Tiled camera configurations""" | ||
| class BaseCameraPresetCfg(PresetCfg): |
There was a problem hiding this comment.
| @configclass | ||
| class WristTiledCameraCfg(PresetCfg): | ||
| """Tiled camera configurations""" | ||
| class WristCameraPresetCfg(PresetCfg): |
There was a problem hiding this comment.
|
|
||
| @configclass | ||
| class MultiDataTypeCartpoleTiledCameraCfg(PresetCfg): | ||
| class MultiDataTypeCartpoleCameraCfg(PresetCfg): |
There was a problem hiding this comment.
Perhaps we should only rename references to TiledCamera and not the actual environment configs? Let's discuss on Monday.
| from isaaclab_tasks.direct.cartpole.cartpole_camera_env_cfg import CartpoleRGBCameraEnvCfg as CartpoleCameraEnvCfg | ||
|
|
||
|
|
||
| def get_tiled_camera_cfg(data_type: str, width: int = 100, height: int = 100) -> TiledCameraCfg: |
There was a problem hiding this comment.
@nvsekkin - this was a suggestion from the reviewer bot; although, I am not sure whether this is a good move unless we add deprecation alias like your other message
There was a problem hiding this comment.
Disregard - I am reverting changes to methods and classes to avoid any breaking changes
bdilinila
force-pushed
the
dev/bdilinila/OMPE-83462_update_downstream_Camera
branch
from
87c8428 to
b44b9a0
Compare
bdilinila
force-pushed
the
dev/bdilinila/OMPE-83462_update_downstream_Camera
branch
from
0cb296f to
da154d4
Compare
…ithub.com:bdilinila/IsaacLab into dev/bdilinila/OMPE-83462_update_downstream_Camera
3 participants
This pull request is a continuation of #5162. This changes references from TiledCamera to Camera in downstream configs.
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there