Skip to content

Fix GelSight Mini render data path - #6777

Merged
kellyguo11 merged 1 commit into
isaac-sim:developfrom
kellyguo11:kellyguo11/fix-gelsight-mini-background-path
Jul 29, 2026
Merged

Fix GelSight Mini render data path#6777
kellyguo11 merged 1 commit into
isaac-sim:developfrom
kellyguo11:kellyguo11/fix-gelsight-mini-background-path

Conversation

@kellyguo11

Copy link
Copy Markdown
Contributor

Description

Point GELSIGHT_MINI_CFG at the published gelsight_r15_data render-data
directory. The previous gs_mini_data background and calibration URLs return
404, causing GelSight Mini renderer initialization to fail with
FileNotFoundError.

This change preserves the existing public configuration symbol and makes its
background resolve to:

IsaacLab/TacSL/gelsight_r15_data/bg.jpg

No new dependencies are required.

Fixes #6182

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

Not applicable.

Validation

  • Confirmed the regression test fails with the old gs_mini_data/bg.jpg path.
  • isaaclab.bat -p -m pytest source/isaaclab_assets/test/test_gelsight.py -q
    (1 passed; one unrelated existing deprecation warning)
  • isaaclab.bat -p tools/changelog/cli.py check develop
  • isaaclab.bat -f

Checklist

  • I have read and understood the contribution guidelines.
  • I have run the pre-commit checks with isaaclab.bat -f.
  • Documentation changes are not required for this configuration correction.
  • My changes generate no new warnings.
  • I have added a regression test that proves the fix is effective.
  • I have added a changelog fragment for isaaclab_assets.
  • My name already exists in CONTRIBUTORS.md.

Point the predefined configuration at the available R1.5 render data
so background and calibration downloads no longer return 404.
@github-actions github-actions Bot added bug Something isn't working asset New asset feature or request labels Jul 29, 2026
@kellyguo11
kellyguo11 marked this pull request as ready for review July 29, 2026 15:42
@kellyguo11
kellyguo11 requested a review from a team July 29, 2026 15:42

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isaac Lab Review Bot

No material issues were identified in the reviewed diff.

  • Design and architecture: No material concerns.
  • API: No material concerns.
  • Implementation: No material concerns.

Ship it. No actionable findings were identified in the reviewed diff.

Conservative automated review; human maintainers own approval decisions.

Comment on lines +13 to +15
assert f"{GELSIGHT_MINI_CFG.sensor_data_dir_name}/{GELSIGHT_MINI_CFG.background_path}" == (
"gelsight_r15_data/bg.jpg"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Test only checks path text

This assertion only restates the configured path string, so it remains green when the referenced resource cannot be resolved or used during GelSight renderer initialization. Exercise resource resolution or renderer initialization to cover the reported regression.

Knowledge Base Used: Isaac Lab Assets and Contrib Extensions

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@greptile-apps

greptile-apps Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Corrects the GelSight Mini render-data directory and adds regression coverage.

  • Reuses the published gelsight_r15_data directory in GELSIGHT_MINI_CFG.
  • Adds a test asserting the resulting background-relative path.
  • Adds an isaaclab_assets changelog fragment.

Confidence Score: 4/5

The PR appears safe to merge, with the non-blocking caveat that its regression test does not verify the resource-resolution failure it is intended to prevent.

The configuration now references the intended published directory, but the test can pass without proving that the background is retrievable and usable by the renderer.

Files Needing Attention: source/isaaclab_assets/test/test_gelsight.py

Important Files Changed

Filename Overview
source/isaaclab_assets/isaaclab_assets/sensors/gelsight.py Updates the GelSight Mini configuration to reference the published render-data directory while preserving its public symbol and other parameters.
source/isaaclab_assets/test/test_gelsight.py Adds path-level regression coverage, but does not exercise resource resolution or renderer initialization.
source/isaaclab_assets/changelog.d/kellyguo11-fix-gelsight-mini-background-path.rst Documents the corrected GelSight Mini render-data configuration.

Reviews (1): Last reviewed commit: "Fix GelSight Mini render data path" | Re-trigger Greptile

@kellyguo11
kellyguo11 merged commit cec3ce4 into isaac-sim:develop Jul 29, 2026
96 of 98 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

asset New asset feature or request bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant