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

Add Gazebo simulation plug-in for IMU and camera modules #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

2b-t
Copy link
Member

@2b-t 2b-t commented Oct 13, 2023

This pull request adds IMU and camera simulation modules for the Alphasense. In the process it was necessary to rotate the _optical_frame as it was not oriented correctly (see the images below). The old incorrect frame still exists as _frame:

Before After
Before After
Before After

There are still some frame inconsistencies that persist. The links do not have the same name as the ones published by the Alphasense driver. The Alphasense driver publishes camX_sensor_frame where X is an integer while the ones inside the URDF are called cam_front_left, cam_front_right, cam_side_right, ... Similarly for the IMU where the driver publishes imu_sensor_frame but inside the URDF the corresponding link is called alphasense_imu. I do not want to fix these inconsistencies as they might break somebody's URDF that build on them.

The current configuration neglects lens distortion. The reason for this is that the kalibr calibration values I have at hand only output distortion values for the equidistant distortion model (see here) while Gazebo only supports the radial tangential Brown–Conrady (see here). More information about this can be found on several issues on kalibr (e.g. #17 and #226). Somebody posted also a Matlab script for approximating the equidistant distortion model with the Brown-Conrady (RadTan) distortion model.

@mmattamala
Copy link

Thanks for making this as well @2b-t There are many things that are wrong in the alphasense driver (from Sevensense):

  • It publishes the images to the /alphasense_driver_ros/camX topic (a namespace), without respecting the ROS convention, which should be /alphasense_driver_ros/camX/image
  • It does not publish the CameraInfo message either
  • The driver publishes the optical_frame as X forward, Y left, Z up (which was what you corrected), while the other frame is called helper. However, this is also conceptually wrong since the optical frame should be Z forward, X right, Y down (as the plugin originally publishes), as this is the convention generally used for cameras.
    I think we can sit together on Monday and discuss this to see if we should emulate the alphasense driver or follow what's conceptually correct instead.

@mcamurri
Copy link
Contributor

I agree with @mmattamala. I designed the camera modules for this description and I followed the z-forward convention as it is the standard one for cameras.

The naming for the modules were chosen to clearly identify each camera on the frontier, and not to copy what the alphasense driver does.

A possible approach to solve might be the following:

  • You keep the camera_front with z-forward as it was before. That frame is directly linked to the IMU via the kalibr configs. The gazebo plugin publishes in this frame
  • You add a child frame with the same name and orientation as produced by the real driver, with appropriate rotations to use SevenSense's convention.

@2b-t
Keep in mind that a widely used package that depends on this repo and hesai_description is the Phasma description: https://github.com/Hilti-Research/phasma_description

We should either update their README to tell their users to point to a specific tag of hesai_description and alphasense_description, or add a submodule to it. In this way these two packages can be updated without the fear of breaking someone else's project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants