Avoid to include windows.h in realtime_helpers.hpp (backport #255) #256
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#193 added the inclusion of windows.h in the public header
realtime_helpers.hpp
.In my limited experience, it is quite important to try to only include windows.h in .cc/.cpp files, as
windows.h
define many problematic macros (MIN
,MAX
,ERROR
) that induce compilation errors when therealtime_helpers.hpp
header is included before rclcpp or ros_control (as several enums are calledERROR
in ros_control, see for an example of the confusing errors induced by windows.h ros-controls/gazebo_ros2_control#377 or #204 (comment), just to mention the most recent I found).For this reason, I think that in this specific case, even if it may seems not ideal, it is a good idea not to include windows.h, and just use directly the value of the
HANDLE
macro, that isvoid*
(see https://codemachine.com/downloads/win80/winnt.h or https://github.com/Alexpux/mingw-w64/blob/d0d7f784833bbb0b2d279310ddc6afb52fe47a46/mingw-w64-tools/widl/include/winnt.h#L557 for a reference, or if you are on a Windows machine just open yourwinnt.h
file). In general it is not a good idea to assume that a given macro will always have a value, but in this specific case, considering that Win32 API almost never breaks backward compatibility (and that is the reasonMIN
,MAX
andERROR
macros are still around), I think the lesser evil is definitely to usingvoid*
in place ofHANDLE
.This is an automatic backport of pull request #255 done by [Mergify](https://mergify.com).