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

Fix compile erros when using RoboStack (conda) #453

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nberliner
Copy link

The RoboStack maintainers helped to fix compile errors when building the UR drivers using a conda environment. conda is, in my humble opinion, very useful and the suggested changes might be helpful for others.

For more information, please see RoboStack/ros-noetic#155 and RoboStack/ros-noetic#157 .

@fmauch
Copy link
Collaborator

fmauch commented Aug 16, 2021

This seems unusual at first.

The console_bridge issues should probably be gone with UniversalRobots/Universal_Robots_Client_Library#74 merged.

As for the -D__STDC_FORMAT_MACROS definition: Why is this necessary?

@nberliner
Copy link
Author

Hi, thanks for having a look. To be honest, I don't know what -D__STDC_FORMAT_MACROS does or why it is necessary. It was suggested to me by the maintainer of RoboStack here.

Maybe you could chime in on RoboStack/ros-noetic#157 to check where this fits best?

@gavanderhoorn
Copy link
Contributor

@fmauch wrote:

As for the -D__STDC_FORMAT_MACROS definition: Why is this necessary?

From How to printf uint64_t? Fails with: “spurious trailing ‘%’ in format”:

The ISO C99 standard specifies that these macros must only be defined if explicitly requested.

#define __STDC_FORMAT_MACROS
#include <inttypes.h>

... now PRIu64 will work

@fmauch
Copy link
Collaborator

fmauch commented Aug 16, 2021

Thanks @gavanderhoorn

From my point of view none of the changes here should then be necessary with the latest versions.

@nberliner could you try building the driver without the changes with the latest library version (master or boost branch) and the latest master of this driver?

See the README about building the library from source, as well.

@nberliner
Copy link
Author

I've tried to build the driver as well as ur_client_library from source. Unfortunately, building the ur_client_library within the RoboStack conda environment seems to be rather complicated (see here) and catkin_make_isolated failed. I probably need to wait until the RoboStack maintainers include the updates.

Please feel free to close this PR, it's definitely beyond my expertise.

Thanks for looking into it :)

@github-actions
Copy link

github-actions bot commented Apr 1, 2023

This PR hasn't made any progress for quite some time and will be closed soon. Please comment if it is still relevant.

@github-actions github-actions bot added the Stale label Apr 1, 2023
@fmauch
Copy link
Collaborator

fmauch commented Apr 11, 2023

I should have a look at this again, before closing this.

@fmauch fmauch removed the Stale label Apr 11, 2023
@github-actions
Copy link

This PR hasn't made any progress for quite some time and will be closed soon. Please comment if it is still relevant.

@github-actions github-actions bot added the Stale label Jul 11, 2023
Copy link
Collaborator

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

Sorry for not having answered for so long. I am still unsure whether I would like to add something that "only" makes it build on an unsupported build system.

The D__STDC_FORMAT_MACROS thing seems to be something that should be fixed inside the conda builds as suggested in RoboStack/ros-noetic#157.

If we could change the console_bridge dependency to a ordinary catkin dependency to rosconsole and not explicitly link against that but let ${catkin_LIBRARIES} handle this, that would be great. Unfortunately, this is out of my scope to test.

find_package(catkin REQUIRED COMPONENTS
roscpp
ur_robot_driver
)
find_package(Eigen3 REQUIRED)
find_package(yaml-cpp REQUIRED)
find_package(ur_client_library REQUIRED)
find_package(console_bridge REQUIRED)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems to make more sense to rely on rosconsole since we use ros/console.h inside the log handler.

@github-actions github-actions bot removed the Stale label Jul 12, 2023
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.

3 participants