From ebd0060907ac9cfc30f86b7238135929a2eb5668 Mon Sep 17 00:00:00 2001 From: Jeremy Nimmer Date: Mon, 6 Jan 2025 14:44:20 -0800 Subject: [PATCH] Encapsulate std::cerr under a single point of control (#656) * Encapsulate std::cerr under a single point of control When integrating this library into a larger application, downstream code doesn't always want to spew onto cerr. To allow customization, we'll refactor to encapsulate the destination inside of a function. Future work can extent this with a mechanism to redirect it (e.g., to send it to spdlog instead of cerr). --------- Signed-off-by: Jeremy Nimmer --- BUILD.bazel | 19 +++++++ include/gz/math/PiecewiseScalarField3.hh | 17 +++--- include/gz/math/detail/Error.hh | 36 +++++++++++++ include/gz/math/graph/Graph.hh | 26 ++++++---- include/gz/math/graph/GraphAlgorithms.hh | 9 +++- src/Color.cc | 4 +- src/CoordinateVector3.cc | 42 +++++++++------ src/Error.cc | 24 +++++++++ src/Kmeans.cc | 26 ++++++---- src/SignalStats.cc | 22 ++++---- src/SphericalCoordinates.cc | 66 +++++++++++++++--------- 11 files changed, 209 insertions(+), 82 deletions(-) create mode 100644 include/gz/math/detail/Error.hh create mode 100644 src/Error.cc diff --git a/BUILD.bazel b/BUILD.bazel index 141dc2675..0dc3471a2 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -170,6 +170,7 @@ cc_library( includes = ["include"], deps = [ ":Config", + ":Error", ":Helpers", ":Vector3", ], @@ -217,6 +218,7 @@ cc_library( deps = [ ":Angle", ":Config", + ":Error", ":Helpers", ":Vector3", "@gz-utils//:ImplPtr", @@ -303,6 +305,17 @@ cc_library( ], ) +cc_library( + name = "Error", + srcs = ["src/Error.cc"], + hdrs = ["include/gz/math/detail/Error.hh"], + includes = ["include"], + deps = [ + ":Config", + ":Export", + ], +) + cc_test( name = "Ellipsoid_TEST", srcs = ["src/Ellipsoid_TEST.cc"], @@ -398,6 +411,7 @@ cc_library( includes = ["include"], deps = [ ":Config", + ":Error", ":Helpers", "@gz-utils//:NeverDestroyed", ], @@ -472,6 +486,7 @@ cc_library( includes = ["include"], deps = [ ":Config", + ":Error", ":Graph", ":Helpers", ], @@ -577,6 +592,7 @@ cc_library( includes = ["include"], deps = [ ":Config", + ":Error", ":Helpers", ":Rand", ":Vector3", @@ -869,6 +885,7 @@ cc_library( includes = ["include"], deps = [ ":Config", + ":Error", ":Region3", ":Vector3", ], @@ -1103,6 +1120,7 @@ cc_library( includes = ["include"], deps = [ ":Config", + ":Error", ":Helpers", "@gz-utils//:ImplPtr", ], @@ -1176,6 +1194,7 @@ cc_library( ":Angle", ":Config", ":CoordinateVector3", + ":Error", ":Helpers", ":Matrix3", ":Vector3", diff --git a/include/gz/math/PiecewiseScalarField3.hh b/include/gz/math/PiecewiseScalarField3.hh index e5e4cac3f..2bfe5b91a 100644 --- a/include/gz/math/PiecewiseScalarField3.hh +++ b/include/gz/math/PiecewiseScalarField3.hh @@ -18,14 +18,15 @@ #define GZ_MATH_PIECEWISE_SCALAR_FIELD3_HH_ #include -#include #include +#include #include #include #include #include #include +#include namespace gz::math { @@ -77,21 +78,23 @@ namespace gz::math { if (pieces[i].region.Empty()) { - std::cerr << "Region #" << i << " (" << pieces[i].region - << ") in piecewise scalar field definition is empty." - << std::endl; + std::ostringstream errStream; + errStream << "Region #" << i << " (" << pieces[i].region + << ") in piecewise scalar field definition is empty."; + detail::LogErrorMessage(errStream.str()); } for (size_t j = i + 1; j < pieces.size(); ++j) { if (pieces[i].region.Intersects(pieces[j].region)) { - std::cerr << "Detected overlap between regions in " + std::ostringstream errStream; + errStream << "Detected overlap between regions in " << "piecewise scalar field definition: " << "region #" << i << " (" << pieces[i].region << ") overlaps with region #" << j << " (" << pieces[j].region << "). Region #" << i - << " will take precedence when overlapping." - << std::endl; + << " will take precedence when overlapping."; + detail::LogErrorMessage(errStream.str()); } } } diff --git a/include/gz/math/detail/Error.hh b/include/gz/math/detail/Error.hh new file mode 100644 index 000000000..c3541180a --- /dev/null +++ b/include/gz/math/detail/Error.hh @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2024 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ +#ifndef GZ_MATH_DETAIL_ERROR_HH_ +#define GZ_MATH_DETAIL_ERROR_HH_ + +#include + +#include +#include + +namespace gz::math +{ + // Inline bracket to help doxygen filtering. + inline namespace GZ_MATH_VERSION_NAMESPACE { + namespace detail { + /// Prints the given error message to std::cerr, followed by a newline. + /// (In the future, we might provide a function to change the destination.) + void GZ_MATH_VISIBLE LogErrorMessage(const std::string& message); + } // namespace detail + } // namespace GZ_MATH_VERSION_NAMESPACE +} // namespace gz::math +#endif // GZ_MATH_DETAIL_ERROR_HH_ diff --git a/include/gz/math/graph/Graph.hh b/include/gz/math/graph/Graph.hh index 1469e9e50..3547498d3 100644 --- a/include/gz/math/graph/Graph.hh +++ b/include/gz/math/graph/Graph.hh @@ -18,14 +18,16 @@ #define GZ_MATH_GRAPH_GRAPH_HH_ #include -#include #include +#include #include +#include #include #include #include #include +#include "gz/math/detail/Error.hh" #include "gz/math/graph/Edge.hh" #include "gz/math/graph/Vertex.hh" @@ -113,8 +115,9 @@ namespace graph { if (!this->AddVertex(v.Name(), v.Data(), v.Id()).Valid()) { - std::cerr << "Invalid vertex with Id [" << v.Id() << "]. Ignoring." - << std::endl; + std::ostringstream errStream; + errStream << "Invalid vertex with Id [" << v.Id() << "]. Ignoring."; + detail::LogErrorMessage(errStream.str()); } } @@ -122,7 +125,7 @@ namespace graph for (auto const &e : _edges) { if (!this->AddEdge(e.vertices, e.data, e.weight).Valid()) - std::cerr << "Ignoring edge" << std::endl; + detail::LogErrorMessage("Ignoring edge"); } } @@ -146,8 +149,9 @@ namespace graph // No space for new Ids. if (id == kNullId) { - std::cerr << "[Graph::AddVertex()] The limit of vertices has been " - << "reached. Ignoring vertex." << std::endl; + detail::LogErrorMessage( + "[Graph::AddVertex()] The limit of vertices has been reached. " + "Ignoring vertex."); return NullVertex(); } } @@ -159,8 +163,9 @@ namespace graph // The Id already exists. if (!ret.second) { - std::cerr << "[Graph::AddVertex()] Repeated vertex [" << id << "]" - << std::endl; + std::ostringstream errStream; + errStream << "[Graph::AddVertex()] Repeated vertex [" << id << "]"; + detail::LogErrorMessage(errStream.str()); return NullVertex(); } @@ -215,8 +220,9 @@ namespace graph // No space for new Ids. if (id == kNullId) { - std::cerr << "[Graph::AddEdge()] The limit of edges has been reached. " - << "Ignoring edge." << std::endl; + detail::LogErrorMessage( + "[Graph::AddEdge()] The limit of edges has been reached. " + "Ignoring edge."); return NullEdge(); } diff --git a/include/gz/math/graph/GraphAlgorithms.hh b/include/gz/math/graph/GraphAlgorithms.hh index 9d3c77fec..72923a09e 100644 --- a/include/gz/math/graph/GraphAlgorithms.hh +++ b/include/gz/math/graph/GraphAlgorithms.hh @@ -26,6 +26,7 @@ #include #include +#include "gz/math/detail/Error.hh" #include "gz/math/graph/Graph.hh" #include "gz/math/Helpers.hh" @@ -217,7 +218,9 @@ namespace graph // Sanity check: The source vertex should exist. if (allVertices.find(_from) == allVertices.end()) { - std::cerr << "Vertex [" << _from << "] Not found" << std::endl; + std::ostringstream errStream; + errStream << "Vertex [" << _from << "] Not found"; + detail::LogErrorMessage(errStream.str()); return {}; } @@ -225,7 +228,9 @@ namespace graph if (_to != kNullId && allVertices.find(_to) == allVertices.end()) { - std::cerr << "Vertex [" << _from << "] Not found" << std::endl; + std::ostringstream errStream; + errStream << "Vertex [" << _from << "] Not found"; + detail::LogErrorMessage(errStream.str()); return {}; } diff --git a/src/Color.cc b/src/Color.cc index ba017ac7c..0759baa5c 100644 --- a/src/Color.cc +++ b/src/Color.cc @@ -16,9 +16,9 @@ */ #include #include -#include #include "gz/math/Color.hh" +#include "gz/math/detail/Error.hh" using namespace gz; using namespace math; @@ -574,6 +574,6 @@ void Color::Clamp() if (clamped) { // TODO(azeey) Use spdlog when we have it's available. - std::cerr << "Color values were clamped\n"; + detail::LogErrorMessage("Color values were clamped"); } } diff --git a/src/CoordinateVector3.cc b/src/CoordinateVector3.cc index dfc13f84d..1549c7ed6 100644 --- a/src/CoordinateVector3.cc +++ b/src/CoordinateVector3.cc @@ -19,10 +19,10 @@ #include "gz/math/CoordinateVector3.hh" #include "gz/math/Vector3.hh" #include "gz/math/Helpers.hh" +#include "gz/math/detail/Error.hh" #include "gz/utils/ImplPtr.hh" #include -#include #include #include @@ -220,14 +220,16 @@ CoordinateVector3 CoordinateVector3::operator+( { if (this->IsMetric()) { - std::cerr << "Spherical coordinates cannot be added to metric. " - "Returning NaN." << std::endl; + detail::LogErrorMessage( + "Spherical coordinates cannot be added to metric. " + "Returning NaN."); return Metric(NAN_D, NAN_D, NAN_D); } else { - std::cerr << "Metric coordinates cannot be added to spherical. " - "Returning NaN." << std::endl; + detail::LogErrorMessage( + "Metric coordinates cannot be added to spherical. " + "Returning NaN."); return Spherical(NAN_D, NAN_D, NAN_D); } } @@ -258,14 +260,16 @@ const CoordinateVector3& CoordinateVector3::operator+=( if (this->IsMetric()) { this->dataPtr->X() = this->dataPtr->Y() = NAN_D; - std::cerr << "Spherical coordinates cannot be added to metric. " - "Setting the result to NaN." << std::endl; + detail::LogErrorMessage( + "Spherical coordinates cannot be added to metric. " + "Setting the result to NaN."); } else { this->dataPtr->Lat() = this->dataPtr->Lon() = NAN_D; - std::cerr << "Metric coordinates cannot be added to spherical. " - "Setting the result to NaN." << std::endl; + detail::LogErrorMessage( + "Metric coordinates cannot be added to spherical. " + "Setting the result to NaN."); } return *this; } @@ -308,14 +312,16 @@ CoordinateVector3 CoordinateVector3::operator-( { if (this->IsMetric()) { - std::cerr << "Spherical coordinates cannot be subtracted from metric. " - "Returning NaN." << std::endl; + detail::LogErrorMessage( + "Spherical coordinates cannot be subtracted from metric. " + "Returning NaN."); return Metric(NAN_D, NAN_D, NAN_D); } else { - std::cerr << "Metric coordinates cannot be subtracted from spherical. " - "Returning NaN." << std::endl; + detail::LogErrorMessage( + "Metric coordinates cannot be subtracted from spherical. " + "Returning NaN."); return Spherical(NAN_D, NAN_D, NAN_D); } } @@ -346,14 +352,16 @@ const CoordinateVector3& CoordinateVector3::operator-=( if (this->IsMetric()) { this->dataPtr->X() = this->dataPtr->Y() = NAN_D; - std::cerr << "Spherical coordinates cannot be subtracted from metric. " - "Setting the result to NaN." << std::endl; + detail::LogErrorMessage( + "Spherical coordinates cannot be subtracted from metric. " + "Setting the result to NaN."); } else { this->dataPtr->Lat() = this->dataPtr->Lon() = NAN_D; - std::cerr << "Metric coordinates cannot be subtracted from spherical. " - "Setting the result to NaN." << std::endl; + detail::LogErrorMessage( + "Metric coordinates cannot be subtracted from spherical. " + "Setting the result to NaN."); } return *this; } diff --git a/src/Error.cc b/src/Error.cc new file mode 100644 index 000000000..057005fcb --- /dev/null +++ b/src/Error.cc @@ -0,0 +1,24 @@ +/* + * Copyright (C) 2024 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ +#include + +#include "gz/math/detail/Error.hh" + +void gz::math::detail::LogErrorMessage(const std::string& message) +{ + std::cerr << message << std::endl; +} diff --git a/src/Kmeans.cc b/src/Kmeans.cc index e8e3458ba..c6643f946 100644 --- a/src/Kmeans.cc +++ b/src/Kmeans.cc @@ -17,9 +17,10 @@ #include -#include +#include #include +#include #include "KmeansPrivate.hh" using namespace gz; @@ -43,8 +44,8 @@ bool Kmeans::Observations(const std::vector &_obs) { if (_obs.empty()) { - std::cerr << "Kmeans::SetObservations() error: Observations vector is empty" - << std::endl; + detail::LogErrorMessage( + "Kmeans::SetObservations() error: Observations vector is empty"); return false; } this->dataPtr->obs = _obs; @@ -56,8 +57,8 @@ bool Kmeans::AppendObservations(const std::vector &_obs) { if (_obs.empty()) { - std::cerr << "Kmeans::AppendObservations() error: input vector is empty" - << std::endl; + detail::LogErrorMessage( + "Kmeans::AppendObservations() error: input vector is empty"); return false; } this->dataPtr->obs.insert(this->dataPtr->obs.end(), _obs.begin(), _obs.end()); @@ -72,23 +73,26 @@ bool Kmeans::Cluster(int _k, // Sanity check. if (this->dataPtr->obs.empty()) { - std::cerr << "Kmeans error: The set of observations is empty" << std::endl; + detail::LogErrorMessage("Kmeans error: The set of observations is empty"); return false; } if (_k <= 0) { - std::cerr << "Kmeans error: The number of clusters has to" - << " be positive but its value is [" << _k << "]" - << std::endl; + std::ostringstream errStream; + errStream << "Kmeans error: The number of clusters has to" + << " be positive but its value is [" << _k << "]"; + detail::LogErrorMessage(errStream.str()); return false; } if (_k > static_cast(this->dataPtr->obs.size())) { - std::cerr << "Kmeans error: The number of clusters [" << _k << "] has to be" + std::ostringstream errStream; + errStream << "Kmeans error: The number of clusters [" << _k << "] has to be" << " lower or equal to the number of observations [" - << this->dataPtr->obs.size() << "]" << std::endl; + << this->dataPtr->obs.size() << "]"; + detail::LogErrorMessage(errStream.str()); return false; } diff --git a/src/SignalStats.cc b/src/SignalStats.cc index 4a4aa9e57..0605b6d26 100644 --- a/src/SignalStats.cc +++ b/src/SignalStats.cc @@ -15,8 +15,9 @@ * */ #include -#include +#include #include +#include using namespace gz; using namespace math; @@ -263,10 +264,11 @@ bool SignalStats::InsertStatistic(const std::string &_name) auto map = this->Map(); if (map.find(_name) != map.end()) { - std::cerr << "Unable to InsertStatistic [" + std::ostringstream errStream; + errStream << "Unable to InsertStatistic [" << _name - << "] since it has already been inserted." - << std::endl; + << "] since it has already been inserted."; + detail::LogErrorMessage(errStream.str()); return false; } } @@ -299,10 +301,11 @@ bool SignalStats::InsertStatistic(const std::string &_name) else { // Unrecognized name string - std::cerr << "Unable to InsertStatistic [" + std::ostringstream errStream; + errStream << "Unable to InsertStatistic [" << _name - << "] since it is an unrecognized name." - << std::endl; + << "] since it is an unrecognized name."; + detail::LogErrorMessage(errStream.str()); return false; } this->dataPtr->stats.push_back(stat); @@ -314,9 +317,8 @@ bool SignalStats::InsertStatistics(const std::string &_names) { if (_names.empty()) { - std::cerr << "Unable to InsertStatistics " - << "since no names were supplied." - << std::endl; + detail::LogErrorMessage( + "Unable to InsertStatistics since no names were supplied."); return false; } diff --git a/src/SphericalCoordinates.cc b/src/SphericalCoordinates.cc index f84a8a6b4..80460bf4e 100644 --- a/src/SphericalCoordinates.cc +++ b/src/SphericalCoordinates.cc @@ -14,11 +14,12 @@ * limitations under the License. * */ -#include +#include #include #include "gz/math/Matrix3.hh" #include "gz/math/SphericalCoordinates.hh" +#include "gz/math/detail/Error.hh" using namespace gz; using namespace math; @@ -118,8 +119,8 @@ SphericalCoordinates::SurfaceType SphericalCoordinates::Convert( else if ("CUSTOM_SURFACE" == _str) return CUSTOM_SURFACE; - std::cerr << "SurfaceType string not recognized, " - << "EARTH_WGS84 returned by default" << std::endl; + detail::LogErrorMessage( + "SurfaceType string not recognized, EARTH_WGS84 returned by default"); return EARTH_WGS84; } @@ -134,8 +135,8 @@ std::string SphericalCoordinates::Convert( else if (_type == CUSTOM_SURFACE) return "CUSTOM_SURFACE"; - std::cerr << "SurfaceType not recognized, " - << "EARTH_WGS84 returned by default" << std::endl; + detail::LogErrorMessage( + "SurfaceType not recognized, EARTH_WGS84 returned by default"); return "EARTH_WGS84"; } @@ -283,14 +284,16 @@ void SphericalCoordinates::SetSurface(const SurfaceType &_type) } case CUSTOM_SURFACE: { - std::cerr << "For custom surfaces, use SetSurface(type, radius," - "axisEquatorial, axisPolar)" << std::endl; + detail::LogErrorMessage( + "For custom surfaces, use SetSurface(type, radius," + "axisEquatorial, axisPolar)"); break; } default: { - std::cerr << "Unknown surface type[" - << this->dataPtr->surfaceType << "]\n"; + std::ostringstream errStream; + errStream << "Unknown surface type[" << this->dataPtr->surfaceType << "]"; + detail::LogErrorMessage(errStream.str()); break; } } @@ -306,8 +309,9 @@ void SphericalCoordinates::SetSurface( (_type != MOON_SCS) && (_type != CUSTOM_SURFACE)) { - std::cerr << "Unknown surface type[" - << _type << "]\n"; + std::ostringstream errStream; + errStream << "Unknown surface type[" << _type << "]"; + detail::LogErrorMessage(errStream.str()); return; } @@ -327,8 +331,8 @@ void SphericalCoordinates::SetSurface( } else { - std::cerr << "Invalid parameters found, defaulting to " - "Earth's parameters" << std::endl; + detail::LogErrorMessage( + "Invalid parameters found, defaulting to Earth's parameters"); this->dataPtr->ellA = g_EarthWGS84AxisEquatorial; this->dataPtr->ellB = g_EarthWGS84AxisPolar; @@ -569,8 +573,9 @@ std::optional PositionTransformTmp( if ((_in == SphericalCoordinates::SPHERICAL) != _pos.IsSpherical()) { - std::cerr << "Invalid input to PositionTransform. " - "The passed coordinate vector has wrong type.\n"; + detail::LogErrorMessage( + "Invalid input to PositionTransform. " + "The passed coordinate vector has wrong type."); return std::nullopt; } Vector3d tmp; @@ -642,7 +647,9 @@ std::optional PositionTransformTmp( break; default: { - std::cerr << "Invalid coordinate type[" << _in << "]\n"; + std::ostringstream errStream; + errStream << "Invalid coordinate type[" << _in << "]"; + detail::LogErrorMessage(errStream.str()); return std::nullopt; } } @@ -703,8 +710,12 @@ std::optional PositionTransformTmp( break; default: - std::cerr << "Unknown coordinate type[" << _out << "]\n"; - return std::nullopt; + { + std::ostringstream errStream; + errStream << "Unknown coordinate type[" << _out << "]"; + detail::LogErrorMessage(errStream.str()); + return std::nullopt; + } } return res; @@ -772,7 +783,8 @@ std::optional VelocityTransformTmp( _out == SphericalCoordinates::SPHERICAL || _vel.IsSpherical()) { - std::cerr << "Velocity cannot be expressed in spherical coordinates.\n"; + detail::LogErrorMessage( + "Velocity cannot be expressed in spherical coordinates."); return std::nullopt; } @@ -807,8 +819,12 @@ std::optional VelocityTransformTmp( case SphericalCoordinates::ECEF: break; default: - std::cerr << "Unknown coordinate type[" << _in << "]\n"; - return std::nullopt; + { + std::ostringstream errStream; + errStream << "Unknown coordinate type[" << _in << "]"; + detail::LogErrorMessage(errStream.str()); + return std::nullopt; + } } CoordinateVector3 res; @@ -840,8 +856,12 @@ std::optional VelocityTransformTmp( break; default: - std::cerr << "Unknown coordinate type[" << _out << "]\n"; - return std::nullopt; + { + std::ostringstream errStream; + errStream << "Unknown coordinate type[" << _out << "]"; + detail::LogErrorMessage(errStream.str()); + return std::nullopt; + } } return res;