GrmanRodriguez commented 2 years ago

This PR addresses several issues.

It addresses #2 by implementing tests using python's unittest library. The tests were performed using the following model:

The test model has spheres, cylinders and boxes, both aligned and not aligned, which allows for testing most configurations.

Each test can be seen in tests/tests.py.

6 is also addressed. The `LinkModifier` now also changes collision and inertia to match the visual if it finds them.

7 is addressed since the function that automatically set the origin is no longer part of the `LinkModifier`, but rather of `FixedOffsetModifier`. Origin modifications now only affect one degree of freedom at a time.

11 is addressed by adding the `FixedOffsetModifier`, which changes the length of a link and also adjusts the origin to keep the parent and child offsets. This allowed also to decouple `LinkModifier` and `JointModifier` such that any modification applied only affects the link/joint in question.

Simplifying Link and Joint modifiers means that bugs that come from needing unintuitive flags are removed (which addresses #10). [^1]

We added position as a possible modification, which refers to the position of the origin. To this end, we renamed the dimension parameter of the LinkModifier and JointModifier to axis. [^1]

We renamed the Side enum to be X, Y, Z instead of WIDTH, DEPTH, HEIGHT [^1]

We also updated the README to reflect the newest changes.

[^1]: is a syntax change and possibly breaks existing code @traversaro @CarlottaSartore

traversaro commented 2 years ago

is a syntax change and possibly breaks existing code @traversaro @CarlottaSartore ↩ ↩2 ↩3

Thanks! For the project that I am following, at the moment we only have code that uses v0.0.1, not current master, probably at some point in the future we will migrate to a new version but at the moment changes in the master branch will not affect us. fyi @fabiodinatale

AlexAntn commented 2 years ago

@traversaro I added some documentation, including some ASCII drawings to explain the formulas and how we derive them, also including comments directly next to the mathematical operations, taking into consideration your comments!

traversaro commented 2 years ago

Probabl we need to also update the corresponding __init__.py file as done in https://github.com/icub-tech-iit/urdf-modifiers/pull/16 ?

GrmanRodriguez commented 2 years ago

Probabl we need to also update the corresponding init.py file as done in https://github.com/icub-tech-iit/urdf-modifiers/pull/16 ?

Done in this commit

traversaro commented 2 years ago

Thanks @AlexAntn @GrmanRodriguez !

icub-tech-iit / urdf-modifiers

Decouple Link/Joint Modifiers, Improve Their Usability, Add FixedOffsetModifier and Add Tests #20

6 is also addressed. The `LinkModifier` now also changes collision and inertia to match the visual if it finds them.

7 is addressed since the function that automatically set the origin is no longer part of the `LinkModifier`, but rather of `FixedOffsetModifier`. Origin modifications now only affect one degree of freedom at a time.

11 is addressed by adding the `FixedOffsetModifier`, which changes the length of a link and also adjusts the origin to keep the parent and child offsets. This allowed also to decouple `LinkModifier` and `JointModifier` such that any modification applied only affects the link/joint in question.

icub-tech-iit / urdf-modifiers

Decouple Link/Joint Modifiers, Improve Their Usability, Add FixedOffsetModifier and Add Tests #20

6 is also addressed. The LinkModifier now also changes collision and inertia to match the visual if it finds them.

7 is addressed since the function that automatically set the origin is no longer part of the LinkModifier, but rather of FixedOffsetModifier. Origin modifications now only affect one degree of freedom at a time.

11 is addressed by adding the FixedOffsetModifier, which changes the length of a link and also adjusts the origin to keep the parent and child offsets. This allowed also to decouple LinkModifier and JointModifier such that any modification applied only affects the link/joint in question.

6 is also addressed. The `LinkModifier` now also changes collision and inertia to match the visual if it finds them.

7 is addressed since the function that automatically set the origin is no longer part of the `LinkModifier`, but rather of `FixedOffsetModifier`. Origin modifications now only affect one degree of freedom at a time.

11 is addressed by adding the `FixedOffsetModifier`, which changes the length of a link and also adjusts the origin to keep the parent and child offsets. This allowed also to decouple `LinkModifier` and `JointModifier` such that any modification applied only affects the link/joint in question.