Ekumen-OS / andino

Open-source diff drive robot ROS 2 compatible
165 stars 45 forks source link

Jesus/#169 camera info yaml #231

Closed JesusSilvaUtrera closed 3 months ago

JesusSilvaUtrera commented 6 months ago

🦟 Bug fix

Closes #169

Summary

I have added a YAML file with the default camera intrinsics to load into the camera.launch.py file. I also included as a comment the link to the info of the camera, just in case we need more technical parameters of it.

The path to the YAML file is included as a parameter in the launch file, just in case it needs to be modified.

JesusSilvaUtrera commented 6 months ago

I have tried running it but as I don't possess an Andino at home, I haven't been able to fully test it, although no errors were found about the new YAML file.

JesusSilvaUtrera commented 6 months ago

@francocipollone I have been looking into this and I have some questions (maybe some are silly, sorry in advance about that jaja):

Plugin gazebo camera Camera info topic

Also, I assume those distortion coefficients are set to 0 by default, not because a calibration has been done, and I also assume that when the intrinsics are computed (via calibration or via copying from the internet) they should also be specified here, as well as in the YAML file.

ros2 run camera_calibration cameracalibrator --size 7x9 --square 0.02 --ros-args -r image:=/my_camera/image_raw -p camera:=/my_camera

We can discuss these topics in a meeting if you prefer.

Thanks in advance!

agalbachicar commented 6 months ago

First, I have seen that in the gazebo simulation the 'libgazebo_ros_camera.so' plugin is used, which is trivial, but I have seen a strange behavior, since the camera_link is provided as the frame but in the camera_info topic the frame is base_link, as you can see here:

Interesting finding indeed! A couple of references:

That is expected behavior given how it is built (see here and here ). Note the function SensorFrameID(). Given that the description does not have a frame_name tag, it'll return the unscoped parent link name.

agalbachicar commented 6 months ago

Also, I assume those distortion coefficients are set to 0 by default, not because a calibration has been done, and I also assume that when the intrinsics are computed (via calibration or via copying from the internet) they should also be specified here, as well as in the YAML file.

Correct. I think we should try to unify them in one place and source them everywhere else.

agalbachicar commented 6 months ago

Lastly, if you explain to me better which node is the responsible for publishing the CameraInfo message when it is being used with the real hardware, because from what I have seen in the docs of v4l2_camera this is only publishing the image, not the info, that is why in the YAML I only added parameters for that node, not the intrinsic parameters of the camera.

Here you can find where the camera info is published in the very same v4l2_camera_node.

However, the publisher is used only when using _intra_processcommunication. I honestly don't know why that default. The way we are launching the node makes it behave that way, see the default rclcpp::NodeOptions::use_intraprocess_comm value (false). Reference to our launchfile and reference to the node executable.

To the best of my knowledge, to get the camera_info topic to be published we should have our own executable that takes care of creating and spinning the node.

JesusSilvaUtrera commented 6 months ago

So, just to recap after reviewing all of @agalbachicar references:

We can discuss all of this and see which actions are better to take.

agalbachicar commented 5 months ago

Per F2F conversation with @JesusSilvaUtrera , we should be able to set also the intra_process_comms flag in the launch file preventing the creation of a custom binary to launch the node.

JesusSilvaUtrera commented 4 months ago

Sorry for the radio silence here. Let me know if it is ready for review. I notice there are a couple of conflicts.

Don't worry about this for now, it won't be ready until I have the camera and calibrate it (Javi has it right now, I will probably get it the next day we meet).

I will let you know when this is ready, thanks Franco!

JesusSilvaUtrera commented 4 months ago

Some notes here:

Let me know what do you think @francocipollone @jballoffet, thanks!

JesusSilvaUtrera commented 3 months ago

@francocipollone BTW, since Javi is OoO and I don't think he will review this, feel free to merge it once you don't have anything else to discuss (I can't do it because I don't have wrote permissions) . Thanks!