urdf_model: don't use or define M_PI#38
Conversation
|
i wonder though what happens to existing user code. Let's say people include this |
|
That's true, but I don't think it's entirely our problem. We could wait for a change in major version number if we wanted to be conservative. |
|
@clalancette @sloretz any thoughts? |
There was a problem hiding this comment.
Looks good to me.
If not breaking any users who are using the math definitions is important the macro definition could be left in.
The change to use const double pi_2 should be merged regardless. If I understand correctly the current code is broken on windows if a user includes <cmath> before including urdf_model/pose.h. The _USE_MATH_DEFINES macro would not be set when it is evaluated in <cmath>.
clalancette
left a comment
There was a problem hiding this comment.
I agree with @sloretz assessment. Users really shouldn't be depending on urdf_model/pose.h to bring in M_PI for them anyway, so I think it is fine to remove _USE_MATH_DEFINES.
Just use a local variable instead. This is an alternative to #34.
cc @Karsten1987