search

PR2 / pr2_robot

PR2-specific components that are used in bringing up a robot.
55 stars 54 forks source link

remove single quotes in const and enum description #252

Closed mikaelarguedas closed 7 years ago

mikaelarguedas commented 7 years ago

Any C++ file including CameraSynchronizerConfig.h fails to compile because of the single quotes. This will fix the failing builds of this package on Indigo (e.g. http://build.ros.org/view/Ibin_uT64/job/Ibin_uT64__pr2_camera_synchronizer__ubuntu_trusty_amd64__binary/)

tfoote commented 7 years ago

@TheDash This has been flagged as a potential regression for indigo on an upcoming sync. https://discourse.ros.org/t/indigo-sync-preparations-2017-04-22/1709/1

Will you have a chance to merge this and make a release soon?

jonbinney commented 7 years ago

@mikaelarguedas @tfoote since this seems like a problem other packages could easily run into, what about cleaning up the string when generating C++ code in dynamic_reconfigure instead? For starters, it could strip all single quotes. In the future it could escape them instead.

mikaelarguedas commented 7 years ago

@jonbinney Indeed I was surprised to see that this package was the only one on the entire set of released packages that was using these single quotes. I think that most people do compile the generated code and thus run into the issue before releasing. The goal of making the generation fail with an explicit error message was to make sure that people wouldn't be able to release not compiling code.

what about cleaning up the string when generating C++ code in dynamic_reconfigure instead?

It would be nice to cleanup the string beforehand indeed. I gave a shot at escaping them at generation time and didn't succeed first shot. I don't know when I'll have spare cycles to give it another try but I'm welcoming PRs :wink:

wjwwood commented 7 years ago

@TheDash can you please make a new release of this package into ROS Indigo?

We've been trying to get an Indigo package sync in for more than a week now and this is the only regression that is concerning us enough to block that. We're going to give you another day, but if we don't hear from you by then we're going to sync anyways which will make some of the PR2 packages uninstallable.

If you do not have time to release right now, at least let us know or ask us for help releasing it.

Thanks.

TheDash commented 7 years ago

Sorry guys. spam eater has been eating my PR2 emails.

On it.

k-okada commented 7 years ago

@TheDash kind reminder, this package still failig on buidfarm http://repositories.ros.org/status_page/ros_indigo_default.html?q=pr2_camera