Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

only use CLOCK_MONOTONIC if not on osx#1142

Merged
dirk-thomas merged 1 commit intoros:lunar-develfrom
ffurrer:fix/osx_compilation
Aug 14, 2017
Merged

only use CLOCK_MONOTONIC if not on osx#1142
dirk-thomas merged 1 commit intoros:lunar-develfrom
ffurrer:fix/osx_compilation

Conversation

@ffurrer
Copy link
Copy Markdown
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
Copy Markdown
Member

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

@flixr
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants