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.

Sign up for GitHub

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

Jump to bottom

Expands functionality of FrameTransformer to allow multi-body transforms #858

Merged

Dhoeller19 merged 20 commits into main from feature/add_external_body_to_frame_transformer

Oct 5, 2024

Collaborator

jsmith-bdai commented Aug 22, 2024 •

edited by Dhoeller19

Loading

Description

Update FrameTransformer to handle 2 new functionalities:

Target frames that aren't children of the source frame prim_path
Target frames that are based upon the source frame prim_path

These new changes mean that the frame names will most likely be different than the configured order - but this was always a possibility to the way the regex is parsed. To be safe, users need to use frame_names to determine indexing into FrameTransformerData.

Test cases have been added for both of these new functionalities - thanks @Mayankm96!

Also, the run script has been updated slightly as the previous indexing was off by 1.

Fixes #857 #294

Type of change

New feature (non-breaking change which adds functionality)

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

jsmith-bdai added 3 commits

August 22, 2024 16:15


          Update FrameTransformer to handle external frames

d480868


          Clean up comments

8284aaa


          Remove unneeded initializers

dfcdb17

jsmith-bdai requested review from Mayankm96 and jtigue-bdai

August 22, 2024 20:39

jsmith-bdai self-assigned this

jtigue-bdai approved these changes

View reviewed changes

Collaborator

jtigue-bdai left a comment

Looks good. Just would be good to warn that only Rigid Body Prims are eligible.

source/extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_transformer/frame_transformer.py Show resolved Hide resolved

source/extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_transformer/frame_transformer.py Outdated Show resolved Hide resolved

jsmith-bdai marked this pull request as ready for review

August 26, 2024 20:30

jsmith-bdai requested review from Dhoeller19 and kellyguo11 as code owners

August 26, 2024 20:30

pascal-roth added enhancement dev team labels

Mayankm96 reviewed

View reviewed changes

source/extensions/omni.isaac.lab/test/sensors/test_frame_transformer.py Outdated Show resolved Hide resolved

Mayankm96 reviewed

View reviewed changes

source/extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_transformer/frame_transformer.py Show resolved Hide resolved

Mayankm96 added 2 commits

August 30, 2024 12:20


          removes high atol and rtol

a9f61f8


          adds a simple failing test

5d8d6f3

Contributor

Mayankm96 commented Aug 30, 2024

I added a very simple test that queries frame offset to the source prim. I thought this was the initial question about the frame transformer bug. The issue persists. Could you take a look?

https://github.com/isaac-sim/IsaacLab/blob/feature/add_external_body_to_frame_transformer/source/extensions/omni.isaac.lab/test/sensors/test_frame_transformer.py#L402

Mayankm96 reviewed

View reviewed changes

source/extensions/omni.isaac.lab/test/sensors/test_frame_transformer.py Outdated Show resolved Hide resolved

jsmith-bdai and others added 5 commits

September 5, 2024 15:04


          Merge branch 'main' into feature/add_external_body_to_frame_transformer

cadd830


          Still working on adding same source and target

2463cc7


          Target frame being based upon source frame now working

e867e42


          Run pre-commit

67f9dbd


          Fixed to work when an env_ isn't present in prim path

15e5375

* Also updated docs

jsmith-bdai requested a review from pascal-roth as a code owner

September 6, 2024 18:40

jsmith-bdai commented

View reviewed changes

source/extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_transformer/frame_transformer.py Outdated Show resolved Hide resolved

jsmith-bdai commented

View reviewed changes

source/standalone/tutorials/04_sensors/run_frame_transformer.py Show resolved Hide resolved

jsmith-bdai requested review from jtigue-bdai and Mayankm96

September 6, 2024 18:45

jsmith-bdai changed the title ~~Feature/add external body to frame transformer~~ Expand funcitonality of FrameTransformer

jsmith-bdai changed the title ~~Expand funcitonality of FrameTransformer~~ Expand functionality of FrameTransformer


          Merge branch 'main' into feature/add_external_body_to_frame_transformer

5d3be27

jtigue-bdai reviewed

View reviewed changes

source/extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_transformer/frame_transformer.py Outdated Show resolved Hide resolved

source/extensions/omni.isaac.lab/test/sensors/test_frame_transformer.py Show resolved Hide resolved

jsmith-bdai commented

View reviewed changes

source/extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_transformer/frame_transformer.py Outdated Show resolved Hide resolved


          Update source/extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_…

baad0be

…transformer/frame_transformer.py

Signed-off-by: James Smith <142246516+jsmith-bdai@users.noreply.github.com>

Mayankm96 reviewed

View reviewed changes

source/extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_transformer/frame_transformer.py Show resolved Hide resolved

Mayankm96 reviewed

View reviewed changes

.../extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_transformer/frame_transformer_cfg.py Outdated Show resolved Hide resolved

Mayankm96 reviewed

View reviewed changes

.../extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_transformer/frame_transformer_cfg.py Outdated Show resolved Hide resolved

Mayankm96 reviewed

View reviewed changes

...extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_transformer/frame_transformer_data.py Show resolved Hide resolved

jsmith-bdai and others added 3 commits

September 17, 2024 08:29


          Update source/extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_…

dbe512a

…transformer/frame_transformer_cfg.py

Co-authored-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com>
Signed-off-by: James Smith <142246516+jsmith-bdai@users.noreply.github.com>


          Update source/extensions/omni.isaac.lab/omni/isaac/lab/sensors/frame_…

3b2eb9c

…transformer/frame_transformer_cfg.py

Co-authored-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com>
Signed-off-by: James Smith <142246516+jsmith-bdai@users.noreply.github.com>


          Merge branch 'main' into feature/add_external_body_to_frame_transformer

542e8bd

jsmith-bdai requested a review from Mayankm96

September 17, 2024 12:31

Mayankm96 changed the title ~~Expand functionality of FrameTransformer~~ Expands functionality of FrameTransformer to allow multi-body transforms


          Merge branch 'main' into feature/add_external_body_to_frame_transformer

93b20ba

Contributor

Mayankm96 commented Sep 19, 2024

@Dhoeller19 said he'd like to go over the logic and check it before we merge. Slating it for 1.3 release now

Collaborator Author

jsmith-bdai commented Sep 25, 2024

@Dhoeller19 @Mayankm96 Can we try to merge this soon? It's been up for some time now and I'd hate for it to get out of sync

Dhoeller19 approved these changes

View reviewed changes

Contributor

Mayankm96 commented Oct 4, 2024

Please update the changelog before merging @jsmith-bdai

Dhoeller19 added 4 commits

October 5, 2024 09:27


          Merge branch 'main' into feature/add_external_body_to_frame_transformer

bae9c60


          Update changelog

9b2c2fd


          Fix formatting

d63b9ae


          Update changelog message

cfbee93

Dhoeller19 merged commit 0ef582b into main

3 of 5 checks passed

Dhoeller19 deleted the feature/add_external_body_to_frame_transformer branch

October 5, 2024 07:47

iamdrfly pushed a commit to iamdrfly/IsaacLab that referenced this pull request


          Expands functionality of FrameTransformer to allow multi-body transfo…

fa68cac

…rms (isaac-sim#858)

# Description

Update FrameTransformer to handle 2 new functionalities:
* Target frames that aren't children of the source frame prim_path
* Target frames that are based upon the source frame prim_path

These new changes mean that the frame names will most likely be
different than the configured order - but this was always a possibility
to the way the regex is parsed. To be safe, users need to use
`frame_names` to determine indexing into `FrameTransformerData`.

Test cases have been added for both of these new functionalities -
thanks @Mayankm96!

Also, the run script has been updated slightly as the previous indexing
was off by 1.


Fixes isaac-sim#857 isaac-sim#294

## Type of change

- New feature (non-breaking change which adds functionality)

## 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
- [x] 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jtigue-bdai jtigue-bdai approved these changes

Dhoeller19 Dhoeller19 approved these changes

kellyguo11 Awaiting requested review from kellyguo11 kellyguo11 is a code owner

pascal-roth Awaiting requested review from pascal-roth pascal-roth is a code owner

Mayankm96 Awaiting requested review from Mayankm96 Mayankm96 is a code owner

Labels

dev team enhancement