Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for Windows #2595

Merged
merged 6 commits into from
Oct 14, 2021
Merged

Fixes for Windows #2595

merged 6 commits into from
Oct 14, 2021

Conversation

Ace314159
Copy link
Contributor

Basic Info

Info Please fill out this column
Ticket(s) this addresses N/A
Primary OS tested on Windows
Robotic platform tested on Gazebo and Rviz simulation of turtlebot

Description of contribution in a few bullet points

  • Fixes build and some runtime errors on Windows

Description of documentation updates required from your changes

N/A


Future work that may be required in bullet points

None, Nav2 seems to work completely on Windows

For Maintainers:

  • Check that any new parameters added are updated in navigation.ros.org
  • Check that any significant change is added to the migration guide
  • Check that any new functions have Doxygen added
  • Check that any new features have test coverage
  • Check that any new plugins is added to the plugins page
  • If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

Replace sleep with Sleep on Windows
Undef NO_ERROR from windows headers
* Define _USE_MATH_DEFINES for M_PI
* Replace uint with unsigned int
* Don't add compiler flags that aren't supported on MSVC
* Use vector instead of VLA when building with MSVC
@SteveMacenski
Copy link
Member

All new development needs to be targeted to main though so it can be incorporated into new distributions. I'm OK with also merging into Foxy, but we need a main PR to merge foxy.

@Ace314159 Ace314159 mentioned this pull request Oct 14, 2021
6 tasks
@SteveMacenski
Copy link
Member

I also question if this PR builds if main doesn't but CI on non-main branches are not working right now. I'd test though before being able to merge so my guess is whatever change you need to make in your other PR needs to be made here too

@Ace314159
Copy link
Contributor Author

This branch should build. I have been using it for a while now. I have a successful CI build using my branch from a week ago here

@SteveMacenski SteveMacenski merged commit 4d7a856 into ros-navigation:foxy-devel Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants