-
Notifications
You must be signed in to change notification settings - Fork 15
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
Move definition logger to cpp to avoid "multiple definition" linker error #21
Move definition logger to cpp to avoid "multiple definition" linker error #21
Conversation
- define logger as extern in header - define logger in cpp - update CMakeLists
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #21 +/- ##
=========================================
Coverage ? 31.40%
=========================================
Files ? 3
Lines ? 207
Branches ? 134
=========================================
Hits ? 65
Misses ? 25
Partials ? 117
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable to me.
kinematics_interface/include/kinematics_interface/kinematics_interface.hpp
Outdated
Show resolved
Hide resolved
@Mergifyio backport humble |
✅ Backports have been created
|
(cherry picked from commit b7958c3)
Hi!
I ran into a little linking issue caused by the
kinematics_interface
package, more precisely by the definition ofrclcpp::Logger LOGGER
in kinematics_interface.hpp.This repos contains the minimal example to reproduce the error, but in a nutshell:
class_a.hpp
of ClassA includeskinematics_interface/kinematics_interface.hpp
;class_b.hpp
of ClassB includesclass_a.hpp
This setups leads to multiple definitions of
rclcpp::Logger LOGGER
and ultimatly to a linker error:To avoid this, could you split the code between header and cpp as proposed in the PR?
Note that in there, the
rclcpp::Logger
is declared as extern, but I guess the declaration of the logger in the header could actually be removed if not used by inherited interfaces (e.g., the KDL one redefine its own).Alternatively, if it really has to be a header-only package, maybe the variable could just be removed or hidden in a
get_logger()
function.What do you think?
Thanks!