Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

only use CLOCK_MONOTONIC if not on osx #1142

Merged
merged 1 commit into from
Aug 14, 2017

Conversation

ffurrer
Copy link
Contributor

@ffurrer ffurrer commented Aug 14, 2017

I'm not sure if this is the cleanest solution, but it resolves the following compilation error:

/usr/local/include/boost/thread/pthread/condition_variable_fwd.hpp:42:13: error: use of undeclared identifier 'pthread_condattr_setclock'; did you mean 'pthread_condattr_setpshared'?
pthread_condattr_setclock(&attr, CLOCK_MONOTONIC);
^~~~~~~~~~~~~~~~~~~~~~~~~
pthread_condattr_setpshared

@dirk-thomas
Copy link
Member

@flixr since you have contributed this code part recently can you please comment on this patch.

@flixr
Copy link
Contributor

flixr commented Aug 14, 2017

Hm.. this actually seems to be a "bug" in boost, it doesn't handle the case that pthread_condattr_setclock is apparently not available under OSX...
This is a real shame, as the SteadyTimer then won't work (or rather use the system time instead of monotonic time).
But in the absence of a better solution, this seems ok as it will at least make it compile again...

@ffurrer what boost version are you using?

@dirk-thomas
Copy link
Member

Falling back to system clock when a steady clock is requested on OSX doesn't sounds like a good behavior. If there is no "simple" fix it might require a custom implementation for OSX to achieve feature parity.

@clalancette
Copy link
Contributor

FWIW, I just did some research on this topic for ROS2: ros2/rcutils#43 (comment) . We could use one of the solutions I pointed out there.

@dirk-thomas
Copy link
Member

I will merge this to fix the regression introduced by the switch to the steady time. Please create a follow up PR to support steady time on OS X (#1143).

@dirk-thomas dirk-thomas merged commit 83e19ce into ros:lunar-devel Aug 14, 2017
@flixr
Copy link
Contributor

flixr commented Aug 14, 2017

Actually it would make sense to also #ifdef out the specialization in steady_timer.cpp, so it uses the same implementation as for WallTime (with the time jump detection) on OSX.

@dirk-thomas
Copy link
Member

Actually it would make sense to also #ifdef out the specialization in steady_timer.cpp, so it uses the same implementation as for WallTime (with the time jump detection) on OSX.

I would like to see a PR to implement steady for OS X rather sooner than later. Therefore I don't think we need a patch for the hopefully very short time period in between.

@ffurrer
Copy link
Contributor Author

ffurrer commented Aug 15, 2017

@flixr I'm using 1.64:

/usr/local/Cellar/boost/1.64.0_1 (12,628 files, 395.7MB) *
Poured from bottle on 2017-04-30 at 08:38:38

@ffurrer ffurrer deleted the fix/osx_compilation branch August 15, 2017 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants