-
Notifications
You must be signed in to change notification settings - Fork 3k
Removes usage of IsaacSim SimulationContext inside tests
#4046
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
base: main
Are you sure you want to change the base?
Removes usage of IsaacSim SimulationContext inside tests
#4046
Conversation
Greptile Summary
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Stage
participant CarbSettings
participant PhysX
participant USDContext
User->>Stage: attach_stage_to_usd_context(attaching_early)
Stage->>Stage: Check Isaac Sim version
Stage->>Stage: Check if stage is in memory
Stage->>PhysX: attach_stage(stage_id)
Stage->>CarbSettings: get("/physics/fabricUpdateTransformations")
CarbSettings-->>Stage: is_rendering_enabled
alt rendering enabled
Stage->>USDContext: attach_stage_with_callback(stage_id)
Stage->>PhysX: attach_stage(stage_id)
end
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 file reviewed, 6 comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
SimulationContext
|
@pascal-roth can you please fix the merge conflicts in here? |
|
@Mayankm96 everything resolved, ready to go |
SimulationContextSimulationContext inside tests
|
For some reason many tests are timing out. Going to run the job again before merging. |
|
the tests are timing out when checking if the Meaning something in the scene is preventing the destruction. This check has been commented out in the new PR So maybe lets merge that first and then these changes here should be work. What do you think @Mayankm96 |
|
@pascal-roth You can set this variable to True inside the tests for now: https://github.com/isaac-sim/IsaacLab/blob/main/source/isaaclab/isaaclab/sim/simulation_context.py#L257 The changes in #4324 will not land into |
|
@Mayankm96 should be good now with the tests (locally they pass) |
Description
Cleans up new util functions
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there