AndrejOrsula / pymoveit2

Basic Python interface for MoveIt 2 built on top of ROS 2 actions and services
BSD 3-Clause "New" or "Revised" License
124 stars 44 forks source link

Allow Scaling Collision Meshes #62

Closed amalnanavati closed 2 months ago

amalnanavati commented 2 months ago

Description

This PR allows users to scale a collision mesh when adding it into the planning scene. Note that scaling occurs within the mesh's XYZ axes.

This PR also adds trimesh as a dependency. Although it is not a required dependency (it is only necessary when using meshes), I think it is beneficial to add so that all possible dependencies can be installed with a single rosdep call.

Testing

amalnanavati commented 2 months ago

I'm fine either way, because if we don't add it to pymoveit2's mandatory dependencies, then I'll just add it to the mandatory dependencies of my package that uses pymoveit2.

In general, I believe it is best to keep Python packages out of the manifest (they might not be handled properly like setup.py or pyproject.toml dependencies).

In my experience, for ROS projects it's easiest to manage all dependencies with rosdep. That way, a single command can do the installation across all packages in the workspace, which makes it easier for new team members to onboard. rosdep certainly has its limitations---like not being able to properly handle version numbers for Python dependencies, and using sudo for pip dependencies---but in my experience the benefits outweigh the drawbacks. In order to use a single rosdep command for all dependencies, all dependencies need to be specified in package.xml.

I haven't used ROS/ROS2 with a virtual environment, and I understand how that could get complicated. It seems like at least one person has been able to get rosdep working with venv.

The user also already gets an explanatory error if they are missing the package when calling the appropriate function.

I agree that this error is very helpful (and more than other open-source libraries do 😄 ).

amalnanavati commented 2 months ago

I went ahead and removed trimesh as a required dependency.