Yaskawa-Global / motoros2

ROS 2 (rcl, rclc & micro-ROS) node for MotoPlus-compatible Yaskawa Motoman robot controllers
89 stars 14 forks source link

Use separate directory for external dependencies (platform lib, CMOS parameter lib, etc) #231

Closed gavanderhoorn closed 2 months ago

gavanderhoorn commented 3 months ago

As per title.

This clearly separates those dependencies from MotoROS2 proper and additionally, prevents GCC from treating them as part of MotoROS2 itself.

Conversely, it prevents GCC from treating MotoROS2 sources as 'external', and thus disabling many of its checks and warnings.

gavanderhoorn commented 3 months ago

I was unsure as to whether this would work, but a test build (I've only tested the YRC1 configuration) seems to suggest it does.

@ted-miller: as you have more experience with this, could you please take a look?

I'm mostly trying to avoid the compiler marking the main src (project) directory as system, which then makes it ignore issues in that directory (which we don't want, obviously).

ted-miller commented 3 months ago

I'm mostly trying to avoid the compiler marking the main src (project) directory as system, which then makes it ignore issues in that directory (which we don't want, obviously)

What type of issues have you seen the compiler ignore?

ted-miller commented 3 months ago

I'm mostly trying to avoid the compiler marking the main src (project) directory as system

I have verified that the main project folder is passed with -I and not isystem.

gavanderhoorn commented 3 months ago

I'm doing some strange / unorthodox things locally which caused the src dir to be passed with -isystem instead of regular -I.

For normal use it doesn't really affect anything, but for the case(s) I describe having the external dependencies in a separate directory helps to avoid the -isystem.

It also doesn't seem to affect regular builds at all, so seems like a benign, if somewhat vaguely motivated, change.

gavanderhoorn commented 2 months ago

@ted-miller: just making sure: ok with me merging this?

ted-miller commented 2 months ago

Yep. All good.