search

ros-drivers / openni2_camera

ROS wrapper for openni 2.0
http://wiki.ros.org/openni2_camera
BSD 3-Clause "New" or "Revised" License
56 stars 95 forks source link

Properly implement color/depth frame time synchronization #78

Closed roehling closed 3 years ago

roehling commented 6 years ago

Frame synchronization, while available as an configurable option, has never been implemented properly. The PR ensures that matching frames have matching time stamps. This makes it possible to use the ExactTime policy to pair off the images later on.

jack-oquin commented 6 years ago

Seems like a useful improvement. I don't know why some of the Travis CI tests failed.

roehling commented 6 years ago

Apparently, the CI test checks for ABI compatibility, which is broken by two more private variables. Maybe it helps to move them to the end of the class definition? Not sure how to proceed.

130s commented 6 years ago

Thanks for the PR and looking into CI failure @roehling. Sounds like ABI check did the right thing but may or may not be in a bit too strict way? I still want to confirm why ABI check failed with other maintainers of CI tool before moving on.

130s commented 6 years ago

Sorry for delay. I guess ABI incompatibility should not be easily taken unfortunately. I'm ok to split a melodic branch (and maintain multiple branches).

roehling commented 6 years ago

Before we do that, maybe there is a way to avoid the incompatibility? The Travis CI log mentions a full report (presumably with more details) as HTML document, but it's not part of the CI output.

130s commented 5 years ago

@roehling I think we should not break ABI compatibility, not just because I know some businesses that use this driver for the production systems, but in general it's disturbing (sorry if I sound pedantic).

As a maintainer I suggest and offer 2 options: