Avoid to include windows.h in realtime_helpers.hpp

[traversaro]

#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 the realtime_helpers.hpp header is included before rclcpp or ros_control (as several enums are called ERROR 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 is void* (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 your winnt.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 reason MIN, MAX and ERROR macros are still around), I think the lesser evil is definitely to using void* in place of HANDLE.

[traversaro]

fyi @SENAI-GilmarCorreia @saikishor @christophfroehlich

[saikishor]

@traversaro the Windows CI is complaining about the change

WNDPROC return value cannot be converted to LRESULT
TypeError: WPARAM is simple, so must be an int object (got NoneType)

Can you also fix the pre-commit changes?. Thank you

[traversaro]

I was indeed testing in a dirty env, let me fix that.

[traversaro]

CI seems happy now. Do you prefer that I squash the commits?

[GilmarCorreia]

I will test this modification. I recently formatted my PC, so it might take a while to get everything up and running smoothly.

Indeed, Windows brings more complications than benefits, and I believe the issues I reported on gazebo_ros2_control ( with my other profile @SENAI-GilmarCorreia) are related to some minor details in the CMakeLists.txt file that Windows cannot handle properly.

I am working hard on this and hope to contribute with new solutions this year.

[traversaro]

I will test this modification. I recently formatted my PC, so it might take a while to get everything up and running smoothly.

Indeed, Windows brings more complications than benefits, and I believe the issues I reported on gazebo_ros2_control ( with my other profile @SENAI-GilmarCorreia) are related to some minor details in the CMakeLists.txt file that Windows cannot handle properly.

I am working hard on this and hope to contribute with new solutions this year.

A bit OT, but if you are interested in running ROS 2 on Windows in robostack (https://robostack.github.io/index.html) we completed a long overdue rebuild of ROS Humble and Jazzy on Windows, you can check the available packages in https://robostack.github.io/humble.html and https://robostack.github.io/jazzy.html).

[saikishor]

@GilmarCorreia Do you think the changes proposed in this PR work for Windows? The CI atleast is happy. If you are then, we can get this merged. If you need some time to test it, we can probably wait

[saikishor]

LGTM and the CI is also happy. Let's see what @GilmarCorreia says

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.12%. Comparing base (d54140d) to head (918e7d5).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #255   +/-   ##
=======================================
  Coverage   70.12%   70.12%           
=======================================
  Files           8        8           
  Lines         492      492           
  Branches       84       84           
=======================================
  Hits          345      345           
  Misses         92       92           
  Partials       55       55           

Flag	Coverage Δ
unittests	70.12% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/realtime_helpers.cpp	39.79% <ø> (ø)

[GilmarCorreia]

Hahaha, I'm feeling important @saikishor, thank you! If the CI is happy, I'm happy too. If I run into any trouble, we can discuss it later in a new PR.

[GilmarCorreia]

I will test this modification. I recently formatted my PC, so it might take a while to get everything up and running smoothly.
Indeed, Windows brings more complications than benefits, and I believe the issues I reported on gazebo_ros2_control ( with my other profile @SENAI-GilmarCorreia) are related to some minor details in the CMakeLists.txt file that Windows cannot handle properly.
I am working hard on this and hope to contribute with new solutions this year.

A bit OT, but if you are interested in running ROS 2 on Windows in robostack (https://robostack.github.io/index.html) we completed a long overdue rebuild of ROS Humble and Jazzy on Windows, you can check the available packages in https://robostack.github.io/humble.html and https://robostack.github.io/jazzy.html).

Amazing work! I will test it.

[saikishor]

Hahaha, I'm feeling important @saikishor, thank you! If the CI is happy, I'm happy too. If I run into any trouble, we can discuss it later in a new PR.

You and @traversaro are known windows users to us. So, your feedback is really helpful. Thank you for approving it

[christophfroehlich]

Thanks, LGTM!

[christophfroehlich]

@Mergifyio backport humble

[mergify]

backport humble

✅ Backports have been created

• #256 Avoid to include windows.h in realtime_helpers.hpp (backport #255) has been created for branch humble