Rename enums to avoid collisions with MSVC compiler defines

[jacobperron]

Resolves ros2/build_farmer#237.

[jacobperron]

Debug:

• Linux
• Linux-aarch64
• macOS
• Windows

[cottsay]

CI looks great - thanks for the quick fix!

[dirk-thomas]

Doesn't this change API and require existing code to be updated (when ported to ROS 2)? Wouldn't it be another option to undefine the macros on Windows?

[jacobperron]

Doesn't this change API and require existing code to be updated (when ported to ROS 2)? Wouldn't it be another option to undefine the macros on Windows?

The API was already changed in #44. For example, instead of OK in ROS 1, it was changed to INFO (and now STATUS_INFO).

State state variables aren't exposed in ROS 1, were only exposed for testing purposes in ROS 2.

[jacobperron]

That being said, I'm also open to undefining the troublesome Windows macros if there's a better argument for that.

[dirk-thomas]

Sounds good to me then.

[david-vay]

These should probably be enum classes.

[jacobperron]

These should probably be enum classes.

Not a bad idea. But I don't think it resolves the clash we had with MSVC since it was caused by the preprocessor.

[david-vay]

Not a bad idea. But I don't think it resolves the clash we had with MSVC since it was caused by the preprocessor.

It should resolve that as well, since the names in an enum class are in their own namespace i.e.

  enum class Status : uint8_t
  {
    DEBUG = 0,
    INFO,
    WARN,
    ERROR
  };

  void f() {
    Status s1 = Status::INFO;
    Status s2 = INFO; // This will not compile
  }

[jacobperron]

🤔

I think it would still fail at the declaration:

  enum class Status : uint8_t
  {
    DEBUG = 0,  // This is clash with MSVC DEBUG macro
    INFO,
    WARN,
    ERROR  // This will also clash
  };

Just a guess, I haven't tried.

[dgossow]

you're right, that would be a problem