ros / roscpp_core

ros distribution sandbox
89 stars 116 forks source link

use std::this_thread::sleep_for instead of WaitableTimer #101

Closed kejxu closed 5 years ago

kejxu commented 5 years ago

use std::this_thread::sleep_for + std::chrono::nanoseconds (both supported since c++11) to replace WaitableTimer + WaitForSingleObject. While testing on Windows, WaitableTimer + WaitForSingleObject tend to have a bad accuracy, a lot of times shorter than expected (random range around the expected length); sleep_for, however, does not have this problem.

newly added libraries are only used inside Windows-specific code at this point, so they are guarded by the _WINDOWS macros (also adding in https://github.com/ros/roscpp_core/pull/100). Supposedly these 2 pull request would lead to merge conflict for each other, will merge and fix the conflict when it happens (when one of the changes gets merged =) ).

kejxu commented 5 years ago

previously this change was part of https://github.com/ros/roscpp_core/pull/100, and it caused failure in CI build for kinetic and lunar versions; however, not in melodic. the reason for this 2/3 fail is because ROS before melodic uses ubuntu 16.04 w/ g++5, -std=c++11 is not turned on by default so the newer libraries cannot be used. Starting from melodic, ubuntu 18.04 w/ g++7 are required, and -std=c++11 is turned on by default for g++7 so all the goodies that come with c++11 can be used

dirk-thomas commented 5 years ago

Thanks for the patch.