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

Multiple #defines when using boost and ur_kin.h #217

Closed
shaun-edwards opened this issue Sep 21, 2015 · 4 comments
Closed

Multiple #defines when using boost and ur_kin.h #217

shaun-edwards opened this issue Sep 21, 2015 · 4 comments

Comments

@shaun-edwards
Copy link
Member

Compiler errors result when the ur_kin.h file is included with boost header files. The error is caused by redefinition of #define macros here.

These macros should be prefixed with a namespace and/or check for previous definition and undef'ed.

@de-vri-es
Copy link
Contributor

There's no real reason to have them as defines, but those are some very dangerously generic names for macros. Undeffing them might have weird effects if boost code uses them (though boost should know better than to use macros with names like that).

The easiest "fix" is to move them to the source file instead of the header since they're not needed in the header anyway. Better yet they can be turned into const doubles in an anonymous namespace in the source file. It should have no noticeable effect in terms of optimization with any modern compiler.

Note that the current interface prevents linking an executable that can do inverse kinematics for more than one arm type. For my own use I changed the kinematics functions to accept a struct containing the kinematic parameters instead.

@de-vri-es
Copy link
Contributor

Actually, after double checking, there are guarantees about using const integral types as compile time constants, but not for doubles. Although I doubt it will make a difference in practise, that could be a reason to use #defines.

Of course, c++11 constexpr would be perfect here if that was allowed.

@gavanderhoorn
Copy link
Member

Nothing more than a +1 from me, this is a really nasty issue, just ran into it myself.

@fmessmer
Copy link
Contributor

fmessmer commented Oct 1, 2015

I (most likely) will be able to test this on Monday...

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

No branches or pull requests

4 participants