Replace fprintf with ROS 2 logging

[sloretz]

The Model class communicates error messages by calling fprintf()

urdf/urdf/src/model.cpp

Line 70 in 533de85

fprintf(stderr, "Could not open file [%s] for parsing.\n", filename.c_str());

This should instead use ROS 2's logging functionality. This might mean passing in a logger instance to the class.

See also:

#13 (review)
#13 (comment)

[guru-florida]

Should the parse do any writing to log files? As a suggestion, perhaps it could throw a parsing exception? The caller can then decide if the error should be logged or silent.

FYI I am resolving a similar issue over in ros2_controls URDF Transmissions parser (PR 182) and I figured I'd come over here to see how it was done and get a feel for precedence. Perhaps find an existing parsing exception class. @Karsten1987 suggested I remove the rclcpp dependency since I am only using it for logging in favor of rclutils logging directly. For now, I am going to switch the Transmission parsing class to throw runtime_error exception and there is also a simple convenience function that parses urdf Transmissions using that class that will catch the exception and send to rclutils logging (which rclcpp logging is based on). Eventually I'd like to integrate Transmissions parsing as part of this urdf module. Thoughts?

If you would like I can replace the fprintf calls with rclutils calls since I am doing the same thing over there. Unless you use other parts of rclcpp it reduces the dependency footprint.

[clalancette]

If you would like I can replace the fprintf calls with rclutils calls since I am doing the same thing over there. Unless you use other parts of rclcpp it reduces the dependency footprint.

As it stands, this package doesn't depend on either rclcpp or rcutils. I'd be in favor of using the RCUTILS macros, as adding a dependency on rclcpp just for logging seems too heavyweight. But I'd also like to hear @sloretz 's opinion on it.

[sloretz]

As it stands, this package doesn't depend on either rclcpp or rcutils. I'd be in favor of using the RCUTILS macros, as adding a dependency on rclcpp just for logging seems too heavyweight.

Agreed - I would use the rcutils API for this package. As far as I can tell that only requires including the macros and using them.

#include <rcutils/logging_macros.h>
// ...
        RCUTILS_LOG_WARN_NAMED(
          "my_package", "Blah Bla Blah [%s] failed", some_str.c_str());
// ...

FYI I am resolving a similar issue over in ros2_controls URDF Transmissions parser (PR 182) and I figured I'd come over here to see how it was done and get a feel for precedence. Perhaps find an existing parsing exception class.

@guru-florida You might find inspiration in SDFormat's Errors class. The APIs sdf::read[String|File|Xml]() Return a bool, but also offer Errors which has info in case it does fail.

[ahcorde]

Related PR #37