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

Update Model parsing #2402

Merged
merged 45 commits into from
Sep 25, 2024
Merged

Conversation

MegMll
Copy link
Collaborator

@MegMll MegMll commented Sep 3, 2024

This pr is related to #2328.

After discussion, it was decided to let users decide of the root joint name, when there is one and to remove the fixed frame, that was added when a root joint was not provided.

Copy link
Contributor

@jcarpent jcarpent left a comment

Choose a reason for hiding this comment

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

Maybe, we can append the rootJointName variable just after the rootJoint.

include/pinocchio/parsers/mjcf.hpp Outdated Show resolved Hide resolved
include/pinocchio/parsers/mjcf.hpp Outdated Show resolved Hide resolved
include/pinocchio/parsers/mjcf/model.hxx Outdated Show resolved Hide resolved
include/pinocchio/parsers/mjcf/model.hxx Outdated Show resolved Hide resolved
include/pinocchio/parsers/sdf.hpp Outdated Show resolved Hide resolved
include/pinocchio/parsers/urdf.hpp Outdated Show resolved Hide resolved
include/pinocchio/parsers/urdf.hpp Outdated Show resolved Hide resolved
include/pinocchio/parsers/urdf/model.hxx Outdated Show resolved Hide resolved
include/pinocchio/parsers/urdf/model.hxx Outdated Show resolved Hide resolved
include/pinocchio/parsers/urdf/model.hxx Outdated Show resolved Hide resolved
@jcarpent jcarpent force-pushed the topic/urdf_root_joint branch from 7e41166 to 31636ef Compare September 4, 2024 13:33
@MegMll MegMll force-pushed the topic/urdf_root_joint branch from db577ae to 1c37592 Compare September 5, 2024 07:58
Copy link
Contributor

@jcarpent jcarpent left a comment

Choose a reason for hiding this comment

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

Currently, there is a break of API. You should keep the initial signature and create a new one. The original one should then call the new signature with default name being "root_joint".

@MegMll MegMll force-pushed the topic/urdf_root_joint branch from 1563a65 to c6671b5 Compare September 6, 2024 15:12
Copy link
Contributor

@jcarpent jcarpent left a comment

Choose a reason for hiding this comment

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

There is still one major issue regarding empty rootJointName argument.
@MegMll COuld you also uniformize the coding style?

Copy link
Contributor

@jcarpent jcarpent left a comment

Choose a reason for hiding this comment

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

There is still one major issue regarding empty rootJointName argument.
@MegMll COuld you also uniformize the coding style?

bindings/python/parsers/mjcf/model.cpp Outdated Show resolved Hide resolved
bindings/python/parsers/sdf/model.cpp Outdated Show resolved Hide resolved
bindings/python/parsers/sdf/model.cpp Outdated Show resolved Hide resolved
bindings/python/parsers/sdf/model.cpp Outdated Show resolved Hide resolved
bindings/python/parsers/urdf/model.cpp Outdated Show resolved Hide resolved
bindings/python/parsers/urdf/model.cpp Outdated Show resolved Hide resolved
bindings/python/parsers/urdf/model.cpp Outdated Show resolved Hide resolved
bindings/python/parsers/urdf/model.cpp Outdated Show resolved Hide resolved
bindings/python/pinocchio/robot_wrapper.py Outdated Show resolved Hide resolved
bindings/python/parsers/mjcf/model.cpp Outdated Show resolved Hide resolved
bindings/python/parsers/mjcf/model.cpp Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@MegMll MegMll force-pushed the topic/urdf_root_joint branch from c6671b5 to 4f64852 Compare September 10, 2024 08:51
Copy link
Contributor

@jorisv jorisv left a comment

Choose a reason for hiding this comment

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

Add some C++ and Python test and its good to me.

bindings/python/pinocchio/robot_wrapper.py Outdated Show resolved Hide resolved
bindings/python/pinocchio/robot_wrapper.py Outdated Show resolved Hide resolved
bindings/python/pinocchio/robot_wrapper.py Outdated Show resolved Hide resolved
bindings/python/pinocchio/robot_wrapper.py Outdated Show resolved Hide resolved
bindings/python/pinocchio/robot_wrapper.py Outdated Show resolved Hide resolved
bindings/python/pinocchio/robot_wrapper.py Outdated Show resolved Hide resolved
bindings/python/pinocchio/shortcuts.py Outdated Show resolved Hide resolved
bindings/python/pinocchio/shortcuts.py Outdated Show resolved Hide resolved
@MegMll MegMll force-pushed the topic/urdf_root_joint branch from 6195e86 to 4086e07 Compare September 23, 2024 10:23
jorisv
jorisv previously approved these changes Sep 23, 2024
include/pinocchio/parsers/mjcf/model.hxx Outdated Show resolved Hide resolved
bindings/python/pinocchio/shortcuts.py Show resolved Hide resolved
@jorisv jorisv enabled auto-merge September 24, 2024 12:39
@MegMll MegMll requested a review from jcarpent September 24, 2024 13:10
@jorisv jorisv merged commit e6c1399 into stack-of-tasks:devel Sep 25, 2024
20 checks passed
@MegMll MegMll deleted the topic/urdf_root_joint branch September 25, 2024 12:40
nim65s added a commit to nim65s/robotpkg that referenced this pull request Nov 8, 2024
    ## [3.3.0] - 2024-11-06

    ### Added

    - Default visualizer can be changed with `PINOCCHIO_VIEWER` environment variable stack-of-tasks/pinocchio#2419
    - Add more Python and C++ examples related to inverse kinematics with 3d tasks stack-of-tasks/pinocchio#2428
    - Add parsing of equality/connect tag for closed-loop chains for MJCF format stack-of-tasks/pinocchio#2413
    - Add compatibility with NumPy 2 `__array__` API stack-of-tasks/pinocchio#2436
    - Added argument to let users decide of root joint name when parsing models (urdf, mjcf, sdf) stack-of-tasks/pinocchio#2402
    - Allow use of `pathlib.Path | str` for paths in python bindings stack-of-tasks/pinocchio#2431
    - Add Pseudo inertia and Log-cholesky parametrization stack-of-tasks/pinocchio#2296
    - Add Pixi support stack-of-tasks/pinocchio#2459

    ### Fixed
    - Fix linkage of Boost.Serialization on Windows stack-of-tasks/pinocchio#2400
    - Fix mjcf parser appending of inertias at root joint stack-of-tasks/pinocchio#2403
    - Fix unit tests with GCC 13.3 stack-of-tasks/pinocchio#2416)
    - Fix class abstract error for Rviz viewer stack-of-tasks/pinocchio#2425
    - Fix compilation issue with MSCV and C++17 stack-of-tasks/pinocchio#2437
    - Fix `pinocchio-test-py-robot_wrapper` when building with SDF and collision support stack-of-tasks/pinocchio#2437
    - Fix crash when calling `Inertia::FromDynamicParameters` in Python with wrong vector size stack-of-tasks/pinocchio#2296
    - Fix `examples/cassie-simulation.py` and `examples/talos-simulation.py` stack-of-tasks/pinocchio#2443
    - Fix build with CppAd 2024 stack-of-tasks/pinocchio#2459
    - Fix `pinocchio-test-cpp-mjcf` unittest with Boost 1.86 stack-of-tasks/pinocchio#2459
    - Fix `pinocchio-test-cpp-constraint-variants` uninitialized values stack-of-tasks/pinocchio#2459
    - Fix mixing library symbols between Pinocchio scalar bindings stack-of-tasks/pinocchio#2459
    - Fix bug for get{Joint,Frame}JacobianTimeVariation stack-of-tasks/pinocchio#2466

    ### Changed

    - Modernize python code base with ruff stack-of-tasks/pinocchio#2418
    - Does not create a root_joint frame from parsed models (urdf, mjcf and sdf) when no root joint is provided stack-of-tasks/pinocchio#2402
nim65s added a commit to nim65s/robotpkg that referenced this pull request Nov 21, 2024
    ## [3.3.0] - 2024-11-06

    ### Added

    - Default visualizer can be changed with `PINOCCHIO_VIEWER` environment variable stack-of-tasks/pinocchio#2419
    - Add more Python and C++ examples related to inverse kinematics with 3d tasks stack-of-tasks/pinocchio#2428
    - Add parsing of equality/connect tag for closed-loop chains for MJCF format stack-of-tasks/pinocchio#2413
    - Add compatibility with NumPy 2 `__array__` API stack-of-tasks/pinocchio#2436
    - Added argument to let users decide of root joint name when parsing models (urdf, mjcf, sdf) stack-of-tasks/pinocchio#2402
    - Allow use of `pathlib.Path | str` for paths in python bindings stack-of-tasks/pinocchio#2431
    - Add Pseudo inertia and Log-cholesky parametrization stack-of-tasks/pinocchio#2296
    - Add Pixi support stack-of-tasks/pinocchio#2459

    ### Fixed
    - Fix linkage of Boost.Serialization on Windows stack-of-tasks/pinocchio#2400
    - Fix mjcf parser appending of inertias at root joint stack-of-tasks/pinocchio#2403
    - Fix unit tests with GCC 13.3 stack-of-tasks/pinocchio#2416)
    - Fix class abstract error for Rviz viewer stack-of-tasks/pinocchio#2425
    - Fix compilation issue with MSCV and C++17 stack-of-tasks/pinocchio#2437
    - Fix `pinocchio-test-py-robot_wrapper` when building with SDF and collision support stack-of-tasks/pinocchio#2437
    - Fix crash when calling `Inertia::FromDynamicParameters` in Python with wrong vector size stack-of-tasks/pinocchio#2296
    - Fix `examples/cassie-simulation.py` and `examples/talos-simulation.py` stack-of-tasks/pinocchio#2443
    - Fix build with CppAd 2024 stack-of-tasks/pinocchio#2459
    - Fix `pinocchio-test-cpp-mjcf` unittest with Boost 1.86 stack-of-tasks/pinocchio#2459
    - Fix `pinocchio-test-cpp-constraint-variants` uninitialized values stack-of-tasks/pinocchio#2459
    - Fix mixing library symbols between Pinocchio scalar bindings stack-of-tasks/pinocchio#2459
    - Fix bug for get{Joint,Frame}JacobianTimeVariation stack-of-tasks/pinocchio#2466

    ### Changed

    - Modernize python code base with ruff stack-of-tasks/pinocchio#2418
    - Does not create a root_joint frame from parsed models (urdf, mjcf and sdf) when no root joint is provided stack-of-tasks/pinocchio#2402
nim65s added a commit to nim65s/robotpkg that referenced this pull request Dec 6, 2024
    ## [3.3.0] - 2024-11-06

    ### Added

    - Default visualizer can be changed with `PINOCCHIO_VIEWER` environment variable stack-of-tasks/pinocchio#2419
    - Add more Python and C++ examples related to inverse kinematics with 3d tasks stack-of-tasks/pinocchio#2428
    - Add parsing of equality/connect tag for closed-loop chains for MJCF format stack-of-tasks/pinocchio#2413
    - Add compatibility with NumPy 2 `__array__` API stack-of-tasks/pinocchio#2436
    - Added argument to let users decide of root joint name when parsing models (urdf, mjcf, sdf) stack-of-tasks/pinocchio#2402
    - Allow use of `pathlib.Path | str` for paths in python bindings stack-of-tasks/pinocchio#2431
    - Add Pseudo inertia and Log-cholesky parametrization stack-of-tasks/pinocchio#2296
    - Add Pixi support stack-of-tasks/pinocchio#2459

    ### Fixed
    - Fix linkage of Boost.Serialization on Windows stack-of-tasks/pinocchio#2400
    - Fix mjcf parser appending of inertias at root joint stack-of-tasks/pinocchio#2403
    - Fix unit tests with GCC 13.3 stack-of-tasks/pinocchio#2416)
    - Fix class abstract error for Rviz viewer stack-of-tasks/pinocchio#2425
    - Fix compilation issue with MSCV and C++17 stack-of-tasks/pinocchio#2437
    - Fix `pinocchio-test-py-robot_wrapper` when building with SDF and collision support stack-of-tasks/pinocchio#2437
    - Fix crash when calling `Inertia::FromDynamicParameters` in Python with wrong vector size stack-of-tasks/pinocchio#2296
    - Fix `examples/cassie-simulation.py` and `examples/talos-simulation.py` stack-of-tasks/pinocchio#2443
    - Fix build with CppAd 2024 stack-of-tasks/pinocchio#2459
    - Fix `pinocchio-test-cpp-mjcf` unittest with Boost 1.86 stack-of-tasks/pinocchio#2459
    - Fix `pinocchio-test-cpp-constraint-variants` uninitialized values stack-of-tasks/pinocchio#2459
    - Fix mixing library symbols between Pinocchio scalar bindings stack-of-tasks/pinocchio#2459
    - Fix bug for get{Joint,Frame}JacobianTimeVariation stack-of-tasks/pinocchio#2466

    ### Changed

    - Modernize python code base with ruff stack-of-tasks/pinocchio#2418
    - Does not create a root_joint frame from parsed models (urdf, mjcf and sdf) when no root joint is provided stack-of-tasks/pinocchio#2402
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants