search

ros-industrial / abb_driver

(old) ROS driver for ABB IRC5 / RW5 or RW6 controllers (Simple Message & RAPID)
http://wiki.ros.org/abb_driver
28 stars 17 forks source link

Use target_compile_defs with exported defs from industrial_core #12

Closed gavanderhoorn closed 3 years ago

gavanderhoorn commented 3 years ago

Since https://github.com/ros-industrial/industrial_core/pull/262, industrial_core exports its compiler definitions.

abb_driver depends on industrial_robot_client and simple_message, and should use those exported definitions, instead of hard-coding them in its own CMakeLists.txt.

The changes in this PR do that.

Note: this PR will fail CI until industrial_core sees a new release, as the CI configuration installs industrial_core using the released packages. As the most recent release of industrial_core does not include https://github.com/ros-industrial/industrial_core/pull/262, the variable referenced in this PR does not exist and the build will fail.

gavanderhoorn commented 3 years ago

Rebased to use CI.

gavanderhoorn commented 3 years ago

Actually, we might want to revert ros-industrial/industrial_core#262 in melodic-devel.

We unfortunately released melodic before that change, so we now have breaking builds -- also in packages not under our control.

We'd probably want/need to branch industrial_core and create a noetic-devel.

clalancette commented 3 years ago

Actually, we might want to revert ros-industrial/industrial_core#262 in melodic-devel.

All right, that's another option. Should I revert https://github.com/ros/rosdistro/pull/30043 for now, and then we can update it as needed later on? That will unblock the Melodic sync that I would like to start getting ready for.

gavanderhoorn commented 3 years ago

If you can wait 5 minutes I'll do a new release of industrial_core.

If not, you could revert.

clalancette commented 3 years ago

If you can wait 5 minutes I'll do a new release of industrial_core.

Waiting is fine, I just wasn't sure how much work it would be :).

gavanderhoorn commented 3 years ago

I've submitted https://github.com/ros/rosdistro/pull/30228 instead.

gavanderhoorn commented 3 years ago

I'm going to close this here for now, and retarget and re-open it later.

gavanderhoorn commented 3 years ago

I'm actually not going to revert ros-industrial/industrial_core#262. See https://github.com/ros-industrial/industrial_core/issues/274#issuecomment-880757047.

That doesn't change anything wrt the 0.7.2 release. We already reverted that in ros/rosdistro.

gavanderhoorn commented 3 years ago

Re-opening, as migrating to industrial_core_DEFINITIONS would still be better, even if industrial_core gets some bw-compatibility.