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

enable ros2 for win10 iot core #53

Closed
wants to merge 4 commits into from
Closed

Conversation

bfjelds
Copy link

@bfjelds bfjelds commented Jan 3, 2018

Most of these changes were made while trying to enable filters and laser_filters (using Node namespace, moving from boost to std, etc).

NO_ERROR is already defined for Windows, so I changed it here to NO_ERRORS.

I wasn't sure how best to handle time in ROS2, tried to get it right :).

enum class TF2Error : std::uint8_t {
NO_ERROR = 0,
NO_ERRORS = 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is a requirement but from my understanding, this should match the enums defined in the corresponding TF2Error ROS message

uint8 NO_ERROR = 0
.

For future references: This seems related to existing issues about windows defines: ros/geometry2#172 and ros2/rosidl#118.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I was blissfully unaware of that. Have pushed a fix to match the message and the enum.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! I'll leave it up to @tfoote to review the rest of this PR and comment on the best way to move forward regarding the enum names as he is the one that has the most context.

@mikaelarguedas mikaelarguedas mentioned this pull request Jan 22, 2018
3 tasks
@mikaelarguedas mikaelarguedas added the in review Waiting for review (Kanban column) label Jan 25, 2018
@tfoote tfoote added the more-information-needed Further information is required label Feb 22, 2018
@mikaelarguedas mikaelarguedas added in progress Actively being worked on (Kanban column) and removed in review Waiting for review (Kanban column) labels Mar 22, 2018
@cottsay
Copy link
Member

cottsay commented Feb 14, 2019

Hi @bfjelds, it looks like ther has been progress on some of the referenced PRs. Is this change still relevant? If so, what are the next steps?

@cottsay
Copy link
Member

cottsay commented Feb 21, 2019

Since this change doesn't appear to have much activity recently, I'm going to move it to the backlog for now. @bfjelds, if you find time to revisit this, please reach out.

@cottsay cottsay added backlog and removed in progress Actively being worked on (Kanban column) labels Feb 21, 2019
@clalancette
Copy link
Contributor

There hasn't been activity here in quite a while, and this now needs a rebase, so I'm going to close this out. Feel free to reopen or open a new PR if you want to continue. Thanks for the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog more-information-needed Further information is required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants