ros4hri / hri_fullbody

A ROS4HRI-conformant node to track and extract 2D and 3D skeletons of humans from RGB/RGB-D cameras
Apache License 2.0
14 stars 5 forks source link

add support for 32bit/m depth encoding #6

Closed Luca-Pozzi closed 9 months ago

Luca-Pozzi commented 9 months ago

This PR implements the changes described in #5 to add support for depth data encoded as 32bit floats.

The code checks for the encoding on the depth topic (if use_depth is true) by waiting 10s for a depth message. If the 10s timeout is reached, the encoding defaults to the original 16UC1. I am not a huge fan of rospy.wait_for_message but I find more reasonable to add a little overhead than re-reading the (same) encoding from each and every Image msg.

LorenzoFerriniCodes commented 9 months ago

Hi @Luca-Pozzi, thanks for your contribution!

In general, the idea of checking the message format and set a parameter to represent it is perfectly fine. However, why waiting a message for (maximum) 10 seconds when we will receive it and read it anyway later in the callback? I think the best option is to check in the callback whether the parameter has been set or not and setting it in the latter case.

Luca-Pozzi commented 9 months ago

Thank you for the feedback @LorenzoFerriniCodes!

The answer to your question, is that I have not thought about it 😅 Definitely a smarter implementation than mine.

I hope to implement the changes later this afternoon

Luca-Pozzi commented 9 months ago

Done, the check for depth encoding is now moved to the callback(s).

Thank you for the advice!