Skip to content

Fixes missing ray_cast_drift in RayCasterCamera #2901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 11, 2025

Conversation

pascal-roth
Copy link
Collaborator

@pascal-roth pascal-roth commented Jul 11, 2025

Description

Fixes missing ray_cast_drift in RayCasterCamera

Fixes #2891

Type of change

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

Checklist

  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@pascal-roth pascal-roth added the bug Something isn't working label Jul 11, 2025
@pascal-roth pascal-roth self-assigned this Jul 11, 2025
@pascal-roth
Copy link
Collaborator Author

@jsmith-bdai and @ooctipus would be great to get some quick eyes on that as #2556 breaks the raycaster camera

Copy link
Collaborator

@jtigue-bdai jtigue-bdai left a comment

Choose a reason for hiding this comment

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

LGTM sorry about the break.

@ooctipus
Copy link
Contributor

ooctipus commented Jul 11, 2025

The changes looks good to me, thank you Pascal for catching that.
On the second thought, it seems like we are getting quite some bugs recently because of user omits env_ids may being slice or sequence, or (maybe) None. After merge in this, maybe we should consider adding test for env_ids type for our mdps. Or if dealing with so many cases for env_ids may be too unwieldy for customer(even ourself), maybe we should consider not exposing None or slice, but give then Sequence only. Happy to discuss more on this @jtigue-bdai @Mayankm96

@kellyguo11
Copy link
Contributor

thanks for catching the fix! it does seem like the ID flexibilities have been hitting us in a couple of places recently, we should consider the design for Newton as well and see what makes sense to support.

kellyguo11 and others added 2 commits July 11, 2025 11:42
Signed-off-by: Kelly Guo <kellyg@nvidia.com>
Signed-off-by: Kelly Guo <kellyguo123@hotmail.com>
@pascal-roth
Copy link
Collaborator Author

I agree @kellyguo11 and @ooctipus, we should introduce tests for the cases.

@kellyguo11 kellyguo11 merged commit e49ab0a into isaac-sim:main Jul 11, 2025
4 of 5 checks passed
@pascal-roth pascal-roth deleted the fix/drift-ray-caster-cam branch July 14, 2025 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report] 'RayCasterCamera' object has no attribute 'ray_cast_drift'
4 participants