Skip to content
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

Adds sanity check for the term type inside the command manager #1315

Merged
merged 6 commits into from
Nov 1, 2024

Conversation

command-z-z
Copy link
Contributor

@command-z-z command-z-z commented Oct 28, 2024

Description

  • The purpose of this change is to enhance the robustness of the code by adding a sanity check to ensure that the command term is of a valid type before it is processed (Just like ActionTerm). This validation step is crucial for preventing unexpected errors and maintaining system stability, especially when dealing with dynamically configured command terms that may vary based on external inputs or configurations. By validating the type of the command term, we can provide early feedback to developers and users, helping to catch misconfigurations or type mismatches during the initial stages. This change aims to improve the overall reliability and maintainability of the codebase.

  • It also fixes iteration over group_cfg_items inside the observation manager.

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

The iteration over group_cfg.__dict__.items() was replaced with
group_cfg_items to ensure the correct items are iterated over. This
fixes potential issues with non-observation settings being processed
incorrectly.
@Mayankm96 Mayankm96 changed the title Adds sanity check if command term is valid type. Adds sanity check for the term type inside the command manager Oct 31, 2024
Signed-off-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com>
Signed-off-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com>
Signed-off-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com>
@Mayankm96 Mayankm96 merged commit c37c50e into isaac-sim:main Nov 1, 2024
3 checks passed
@command-z-z command-z-z deleted the manager branch November 1, 2024 13:12
iamdrfly pushed a commit to iamdrfly/IsaacLab that referenced this pull request Nov 21, 2024
…-sim#1315)

# Description

- The purpose of this change is to enhance the robustness of the code by
adding a sanity check to ensure that the command term is of a valid type
before it is processed (Just like ActionTerm). This validation step is
crucial for preventing unexpected errors and maintaining system
stability, especially when dealing with dynamically configured command
terms that may vary based on external inputs or configurations. By
validating the type of the command term, we can provide early feedback
to developers and users, helping to catch misconfigurations or type
mismatches during the initial stages. This change aims to improve the
overall reliability and maintainability of the codebase.

- It also fixes iteration over group_cfg_items inside the observation
manager.

## Type of change

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

## Checklist

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

---------

Signed-off-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com>
Co-authored-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants