Skip to content

Conversation

Mayankm96
Copy link
Contributor

@Mayankm96 Mayankm96 commented Sep 1, 2025

Description

This PR fixes a bug where the platform_height parameter was incorrectly placed in the MeshPyramidStairsTerrainCfg class instead of the appropriate base configuration class for mesh terrain objects.

  • Removes the misplaced platform_height parameter from MeshPyramidStairsTerrainCfg
  • Adds the platform_height parameter to the correct location in the MeshRepeatedObjectsTerrainCfg class
  • Includes various formatting improvements with additional blank lines for consistency

Fixes #3162

Regression was introduced in MR #2695

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

@Mayankm96 Mayankm96 added the bug Something isn't working label Sep 1, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where the platform_height parameter was incorrectly placed in the MeshPyramidStairsTerrainCfg class instead of the appropriate base configuration class for mesh terrain objects.

  • Removes the misplaced platform_height parameter from MeshPyramidStairsTerrainCfg
  • Adds the platform_height parameter to the correct location in the MeshRepeatedObjectsTerrainCfg.ObjectCfg class
  • Includes various formatting improvements with additional blank lines for consistency

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
source/isaaclab/isaaclab/terrains/trimesh/mesh_terrains_cfg.py Moves platform_height from wrong class to correct base configuration and adds formatting improvements
source/isaaclab/isaaclab/terrains/height_field/hf_terrains_cfg.py Formatting improvements with additional blank lines and fixes a type annotation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

github-actions bot commented Sep 1, 2025

Test Results Summary

2 446 tests   2 037 ✅  2h 13m 34s ⏱️
   92 suites    408 💤
    1 files        1 ❌

For more details on these failures, see this check.

Results for commit 8970553.

@heleiduan
Copy link

Thanks for fixing this bug #3162! Could it be useful if ./isaaclab.sh -p source/isaaclab/test/terrains/check_mesh_subterrains.py is enabled during CI? This missing cfg was captured by runnning this test.

@Mayankm96
Copy link
Contributor Author

@heleiduan Good point. I was checking the same today why this wasn't caught in the CI. Will change the script to be a test instead :)

@Mayankm96 Mayankm96 requested a review from kellyguo11 September 2, 2025 04:32
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.

Thanks for the fix. That was a bad transfer from our end.

@Mayankm96
Copy link
Contributor Author

Merging this for now. Will make a unit test for it once I get more time.

@Mayankm96 Mayankm96 merged commit e57da49 into main Sep 4, 2025
8 of 10 checks passed
@Mayankm96 Mayankm96 deleted the fix/terrain-cfg branch September 4, 2025 15:15
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] Missing cfg attribute for MeshRepeatedObjectsTerrain

4 participants