Updated TiledCamera references to Camera in downstream configs#5377
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
2 times, most recently
from
0cb296f to
da154d4
Compare
bdilinila
force-pushed
the
dev/bdilinila/OMPE-83462_update_downstream_Camera
branch
2 times, most recently
from
707fa21 to
82f9969
Compare
bdilinila
requested review from
Toni-SM,
hhansen-bdai and
xyao-nv
as code owners
bdilinila
force-pushed
the
dev/bdilinila/OMPE-83462_update_downstream_Camera
branch
from
82f9969 to
5092c12
Compare
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Follow-up Review
The new commit (6bc0dda) addresses one of the previous findings:
✅ Previous concern about README.md comment addressed — Line 290 now reads "Camera configuration (dimensions must match GELSIGHT_R15_CFG which provides the render_cfg)" which clarifies the relationship.
The remaining previous findings (function naming get_tiled_camera_cfg, class naming BaseTiledCameraCfg/WristTiledCameraCfg, subtitle strings, inner class naming convention) are minor style suggestions that don't block the PR.
Implementation Verdict
Ship it — clean mechanical refactor with consistent application. CI checks are pending but the changes are straightforward namespace updates.
bdilinila
force-pushed
the
dev/bdilinila/OMPE-83462_update_downstream_Camera
branch
from
6bc0dda to
c416c2e
Compare
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Follow-up Review
The new commit (c416c2e) adds the author's name to CONTRIBUTORS.md — a standard housekeeping change with no issues.
All previous findings from the earlier review remain as noted:
- ✅ README.md comment was already addressed in prior commit
- The remaining style suggestions (function/class naming with "tiled" prefix, subtitle strings) are minor and don't block the PR
Implementation Verdict
Ship it — the PR is a clean, consistent rename of TiledCamera → Camera references across the codebase. CI is mostly pending but pre-commit passed and the changes are mechanical namespace updates.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Follow-up Review
The new commit (4ef0d8d) adds minor documentation changes - updating a comment in visuotactile_sensor.py to clarify that camera dimensions must match the render config provided by GELSIGHT_R15_CFG. This is a helpful clarification and introduces no issues.
All previous findings remain addressed. The PR is a clean mechanical rename with no new concerns.
Implementation Verdict
Ship it
Resolves conflict in source/isaaclab/isaaclab/sensors/camera/camera.py: upstream isaac-sim#5377 reworded the Isaac Sim 4.5 segmentation-instancing warning ("Camera and TiledCamera" -> "Camera") inside the in-Camera HACK block, but this branch (commit "Separate render backend specific code from Camera") deliberately removed that block from Camera and relocated it to isaaclab_physx/renderers/isaac_rtx_renderer.py. Kept HEAD (block removed) and applied the equivalent rename in the relocated copy so the warning text matches upstream's intent. Also realigned the camera_cfg.py ".. deprecated:: 4.6.13" Sphinx markers to "4.6.22" to match the version in extension.toml and the CHANGELOG entry that announces this deprecation. Made-with: Cursor
4 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