Compiling ros/time.h in x64 Visual Studio project results in incorrect selection between sys/time.h and sys/timeb.h about roscpp_core HOT 5 CLOSED

The code in question:

The code base uses WIN32 in many places to check for Windows. That is afaik the recommended approach. I am not sure why that is not defined in your case.

Maybe @kejxu can comment on this?

from roscpp_core.

Thanks for sharing your experience @marcbertola, and redirection @dirk-thomas!

This is a good catch, VS compiler only defines _WIN32; however, we have never noticed build breaks resulting from this. Quick question for @marcbertola, how are you building the application? Are you building with catkin/cmake?

I dug in a bit into the code base, and exactly as you mentioned, WIN32 is defined in <windows.h>:

// minwindef.h
#ifndef WIN32
#define WIN32
#endif

whereas the VS compiler only specifies _WIN32. This is also the reason why our recent port have only been using _WIN32 (for example, 11db0ec)

from roscpp_core.

Hi @kejxu, thanks for looking into this.

I was writing a standalone program to characterize the throughput for a large PointCloud2 through ROS topics. It was a simple console program using standard, portable C++. It created a PointCloud2 message, populated it, and then sent it out repeatedly. I then used one of the standard ROS utilities (can't remember which one) to receive the messages and display the rate at which they were being received. To answer your question: I wasn't building ROS itself or using catkin.

Since I didn't actually need any of the Windows API to do this (just standard C++), I didn't include windows.h, and so WIN32 was not defined. My own research corroborates what you are saying about WIN32 vs _WIN32. When I did include it before including the ROS headers, WIN32 was defined and things started working.

Also, I haven't tried it, it's probably worth checking to see if if either variable is defined if you try compiling with GCC under Cygwin or MinGW. But then again, time.h might actually exist in those scenarios, so it might already be ok.

from roscpp_core.

Assuming that #110 checking for _WIN32 resolved this I am going ahead and close the ticket. Please feel free to continue commenting and the ticket can be reopened if necessary.

from roscpp_core.

I wasn't building ROS itself or using catkin.

Thanks! I wouldn't be able to make any assertive conclusions without further investigation, but I had very similar questions when I just started working with catkin. A few of these macros, WIN32 and _WINDOWS for example, I think are injected by catkin's cmake pipeline.

If time allows, I think it would be worthwhile to try building the application with catkin.

Since I didn't actually need any of the Windows API to do this (just standard C++), I didn't include windows.h, and so WIN32 was not defined. My own research corroborates what you are saying about WIN32 vs _WIN32. When I did include it before including the ROS headers, WIN32 was defined and things started working.

You're definitely doing the right thing not including windows.h =)

Also, I haven't tried it, it's probably worth checking to see if if either variable is defined if you try compiling with GCC under Cygwin or MinGW. But then again, time.h might actually exist in those scenarios, so it might already be ok.

This is a very interesting point. I have not had too much experience working with GCC on Windows, but based on what I have read about, _WIN32 might need to be injected from the command line during build time. However, at this point I think we are only targeting MSVC for the win32 platform. But still, this is still a very valid point, thanks for pointing it out!

from roscpp_core.