Skip to content

Conversation

@Ace314159
Copy link
Contributor

There were 2 main things I had to change to get this working on windows

  1. To fix the build issues, I changed a couple things. Although most of the classes were exported using the visibility control, it was missing for ResourceManager and GenericSystem in hardware_interface. Also, TestController::set_command_interface_configuration wasn't exported either.
  2. There were also issues when loading a controller in controller_manager. In release mode, asserts are disabled, so the try_lock wasn't called, resulting in unlock being called too many times, which is undefined behavior. This seems to be fine on other platforms, but on Windows, this resulted in the lock not being considered freed and caused a hang when other functions tried to use that lock. Also it looked like the debug message for configuring a controller was wrong, so I changed that as well.

@Karsten1987
Copy link
Contributor

This looks good to me. If @v-lopez doesn't have any complaints about the lock behavior, I'd merge this.

@bmagyar bmagyar requested a review from v-lopez June 25, 2021 07:08
@Ace314159 Ace314159 mentioned this pull request Jul 1, 2021
6 tasks
Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

thanks!

@Karsten1987
Copy link
Contributor

@bmagyar @v-lopez is this good to merge?

@bmagyar
Copy link
Member

bmagyar commented Jul 2, 2021

Yup, just forgot to come back after kicking off the CI

@bmagyar bmagyar merged commit 6328200 into ros-controls:master Jul 2, 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.

4 participants