anqixu / ueye_cam

A ROS nodelet and node that wraps the driver API for UEye cameras by IDS Imaging Development Systems GMBH.
Other
60 stars 102 forks source link

Drifting time offset between image timestamp and system time #82

Closed Chris-Bee closed 3 years ago

Chris-Bee commented 3 years ago

Hello everybody,

I'm using the UI-3270 LE-M-GL and driver/sdk version 4.93.00.

I'm facing an issue where the ROS message timestamp of /camera/image_raw is drifting and shows an increasing time offset between the system time and the timestamp of the ROS message header. I could narrow the issue down to the pixel clock. The issue occurs if we increase the pixel clock to 197 to allow image rates of 20Hz. The offset increases and decreases depending on the value of the pixel clock. We had an offset of 1 hour with a pixel clock value of 60 which seems to be similar to this issue https://github.com/anqixu/ueye_cam/issues/36#issue-141109154

The ROS node does not actively incorporate the pixel clock in any calculations, thus, I'm wondering if this is a ROS node or camera driver issue.

Chris-Bee commented 3 years ago

This line is actually causing the offset: https://github.com/anqixu/ueye_cam/blob/8a055ec4e31e37c3041aee46b2d7d457c803ce62/src/ueye_cam_nodelet.cpp#L987

Switching from no subscribers to having a subscriber multiple times and printing the difference between ros:time::now and the 'getImageTimestamp' showed that the error increases with time. For some reason always in multiples of full minutes (-119.996, -359.997) and it is read from the camera, thus I assume the internal time sync does not work correctly.

Setting the init time stamp to ros::time::now() solved the issue: init_ros_time_ = ros::Time::now();

anqixu commented 3 years ago

Okay, makes sense. Unfortunately I don't have a ueye camera during quarantine times, so I'm not able to test it. But, given that this relates to several issues from the past, I'll make the switch to using ros::Time::now(). Hopefully the worst case that could happen is that some existing usages might have a slight clock skew with this change.... but that's definitely better than clock drift.

Thanks for sleuthing.

anqixu commented 3 years ago

@Chris-Bee can you please pull this branch, build, and confirm that the change fixes things (and didn't break others)? Many thanks in advance!

Chris-Bee commented 3 years ago

Perfect, thank you. I will be able to perform the test on Friday.

anqixu commented 3 years ago

@Chris-Bee any updates? once confirmed, I'll make a release

Chris-Bee commented 3 years ago

Hello @anqixu my apologies for the delay. I tested the branch and it works as intended. Thanks again for adding the changes to the repo.

anqixu commented 3 years ago

thank you very much for confirming :)