From 00a3dc6b6ed597f0c49439ba2e80b3e46f0bfae3 Mon Sep 17 00:00:00 2001 From: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> Date: Thu, 27 Jan 2022 13:43:13 -0700 Subject: [PATCH 01/13] add USD-specific error codes Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> --- include/sdf/Error.hh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/sdf/Error.hh b/include/sdf/Error.hh index fb920ec72..2758ec3b5 100644 --- a/include/sdf/Error.hh +++ b/include/sdf/Error.hh @@ -147,6 +147,18 @@ namespace sdf /// \brief Merge include is unspported for the type of entity being /// included. MERGE_INCLUDE_UNSUPPORTED, + + /// \brief Parsing a SDF object to a USD object failed. + /// This error type is specific to the USD component. + USD_SDF_TO_USD_PARSING_ERROR, + + /// \brief The pxr::SdfPath does not point to a valid USD prim. + /// This error type is specific to the USD component. + USD_INVALID_PRIM_PATH, + + /// \brief A pxr API was not able to be applied to a USD prim. + /// This error type is specific to the USD component. + USD_FAILED_PRIM_API_APPLY, }; class SDFORMAT_VISIBLE Error From 0d3c3f7842ddc9c0b1b010f77a5729ea2f79534f Mon Sep 17 00:00:00 2001 From: Teo Koon Peng Date: Wed, 9 Feb 2022 16:25:28 +0800 Subject: [PATCH 02/13] sdf::usd::Error Signed-off-by: Teo Koon Peng --- include/sdf/Error.hh | 12 -- usd/include/sdf/usd/Error.hh | 187 ++++++++++++++++++++++++ usd/include/sdf/usd/sdf_parser/Light.hh | 3 +- usd/include/sdf/usd/sdf_parser/World.hh | 3 +- usd/src/CMakeLists.txt | 1 + usd/src/Error.cc | 166 +++++++++++++++++++++ usd/src/sdf_parser/Light.cc | 4 +- usd/src/sdf_parser/World.cc | 4 +- 8 files changed, 362 insertions(+), 18 deletions(-) create mode 100644 usd/include/sdf/usd/Error.hh create mode 100644 usd/src/Error.cc diff --git a/include/sdf/Error.hh b/include/sdf/Error.hh index 2758ec3b5..fb920ec72 100644 --- a/include/sdf/Error.hh +++ b/include/sdf/Error.hh @@ -147,18 +147,6 @@ namespace sdf /// \brief Merge include is unspported for the type of entity being /// included. MERGE_INCLUDE_UNSUPPORTED, - - /// \brief Parsing a SDF object to a USD object failed. - /// This error type is specific to the USD component. - USD_SDF_TO_USD_PARSING_ERROR, - - /// \brief The pxr::SdfPath does not point to a valid USD prim. - /// This error type is specific to the USD component. - USD_INVALID_PRIM_PATH, - - /// \brief A pxr API was not able to be applied to a USD prim. - /// This error type is specific to the USD component. - USD_FAILED_PRIM_API_APPLY, }; class SDFORMAT_VISIBLE Error diff --git a/usd/include/sdf/usd/Error.hh b/usd/include/sdf/usd/Error.hh new file mode 100644 index 000000000..25e4d96eb --- /dev/null +++ b/usd/include/sdf/usd/Error.hh @@ -0,0 +1,187 @@ +/* + * Copyright 2022 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 SDF_USD_ERROR_HH_ +#define SDF_USD_ERROR_HH_ + +#include +#include +#include +#include + +#include +#include +#include + +#ifdef _WIN32 +// Disable warning C4251 which is triggered by +// std::string +#pragma warning(push) +#pragma warning(disable : 4251) +#endif + +namespace sdf +{ +// Inline bracket to help doxygen filtering. +inline namespace SDF_VERSION_NAMESPACE +{ +namespace usd +{ +// + +/// \enum ErrorCode +/// \brief Set of error codes. Usually one or more errors are returned in +/// an Errors vector. The collection of Errors should be take as a whole, +/// where an error toward the beginning of the vector can inform errors +/// toward the end of the vector. +/// \sa Errors +enum class ErrorCode +{ + // \brief No error + NONE = 0, + + /// \brief A wrapped SDF error. + SDF_ERROR, + + /// \brief Parsing a SDF object to a USD object failed. + /// This error type is specific to the USD component. + SDF_TO_USD_PARSING_ERROR, + + /// \brief The pxr::SdfPath does not point to a valid USD prim. + /// This error type is specific to the USD component. + INVALID_PRIM_PATH, + + /// \brief A pxr API was not able to be applied to a USD prim. + /// This error type is specific to the USD component. + FAILED_PRIM_API_APPLY, +}; + +class IGNITION_SDFORMAT_VISIBLE Error +{ + /// \brief Default constructor + public: + Error(); + + /// \brief Constructor. + /// \param[in] _code The error code. + /// \param[in] _message A description of the error. + /// \sa ErrorCode. + public: + Error(const ErrorCode _code, const std::string &_message); + + /// \brief Constructor. + /// \param[in] _code The error code. + /// \param[in] _message A description of the error. + /// \param[in] _filePath The file path that is related to this error. + /// \sa ErrorCode. + public: + Error(const ErrorCode _code, const std::string &_message, + const std::string &_filePath); + + /// \brief Constructor. + /// \param[in] _code The error code. + /// \param[in] _message A description of the error. + /// \param[in] _filePath The file path that is related to this error. + /// \param[in] _lineNumber The line number in the provided file path where + /// this error was raised. + /// \sa ErrorCode. + public: + Error(const ErrorCode _code, const std::string &_message, + const std::string &_filePath, int _lineNumber); + + /// \brief Constructor. + /// \param[in] _sdf_error Wrap a sdf error. + /// \sa ErrorCode. + public: + Error(const sdf::Error& _sdf_error); + + /// \brief Get the error code. + /// \return An error code. + /// \sa ErrorCode. + public: + ErrorCode Code() const; + + /// \brief Get the error message, which is a description of the error. + /// \return Error message. + public: + std::string Message() const; + + /// \brief Get the file path associated with this error. + /// \return Returns the path of the file that this error is related to, + /// nullopt otherwise. + public: + std::optional FilePath() const; + + /// \brief Sets the file path that is associated with this error. + /// \param[in] _filePath The file path that is related to this error. (e.g. + /// /tmp/test_file.sdf) + public: + void SetFilePath(const std::string &_filePath); + + /// \brief Get the line number associated with this error. + /// \return Returns the line number. nullopt otherwise. + public: + std::optional LineNumber() const; + + /// \brief Sets the line number that is associated with this error. + /// \param[in] _lineNumber The line number that is related to this error. + public: + void SetLineNumber(int _lineNumber); + + /// \brief Get the underlying sdf error. + /// \return The underlying sdf error. + public: + std::optional SdfError() const; + + /// \brief Safe bool conversion. + /// \return True if this Error's Code() != NONE. In otherwords, this is + /// true when there is an error. + public: + explicit operator bool() const; + + /// \brief Compare this Error to a boolean value. + /// \return True if the boolean evaluation of this Error equals _value. + /// If _value == false, then true is returned when this Error's Code() is + /// equal to NONE and false is returned otherwise. If _value == true, + /// then true is returned when this Error's Code() is not equal to NONE + /// and false is returned otherwise. + /// \sa explicit operator bool() const + public: + bool operator==(const bool _value) const; + + /// \brief Output operator for an error. + /// \param[in,out] _out The output stream. + /// \param[in] _err The error to output. + /// \return Reference to the given output stream + public: + friend IGNITION_SDFORMAT_VISIBLE std::ostream &operator<<( + std::ostream &_out, const sdf::usd::Error &_err); + + /// \brief Private data pointer. + IGN_UTILS_IMPL_PTR(dataPtr) +}; + +using Errors = std::vector; + +} // namespace usd +} // namespace SDF_VERSION_NAMESPACE +} // namespace sdf + +#ifdef _WIN32 +#pragma warning(pop) +#endif + +#endif diff --git a/usd/include/sdf/usd/sdf_parser/Light.hh b/usd/include/sdf/usd/sdf_parser/Light.hh index fdb260238..0996ada6b 100644 --- a/usd/include/sdf/usd/sdf_parser/Light.hh +++ b/usd/include/sdf/usd/sdf_parser/Light.hh @@ -32,6 +32,7 @@ #include "sdf/config.hh" #include "sdf/system_util.hh" #include "sdf/Light.hh" +#include "../Error.hh" namespace sdf { @@ -48,7 +49,7 @@ namespace sdf /// be a valid USD path. /// \return Errors, which is a vector of Error objects. Each Error includes /// an error code and message. An empty vector indicates no error. - sdf::Errors SDFORMAT_VISIBLE ParseSdfLight(const sdf::Light &_light, + sdf::usd::Errors SDFORMAT_VISIBLE ParseSdfLight(const sdf::Light &_light, pxr::UsdStageRefPtr &_stage, const std::string &_path); } } diff --git a/usd/include/sdf/usd/sdf_parser/World.hh b/usd/include/sdf/usd/sdf_parser/World.hh index f5c7d8ccd..8dcb11ae4 100644 --- a/usd/include/sdf/usd/sdf_parser/World.hh +++ b/usd/include/sdf/usd/sdf_parser/World.hh @@ -32,6 +32,7 @@ #include "sdf/config.hh" #include "sdf/system_util.hh" #include "sdf/World.hh" +#include "../Error.hh" namespace sdf { @@ -48,7 +49,7 @@ namespace sdf /// a valid USD path. /// \return Errors, which is a vector of Error objects. Each Error includes /// an error code and message. An empty vector indicates no error. - sdf::Errors SDFORMAT_VISIBLE ParseSdfWorld(const sdf::World &_world, + sdf::usd::Errors SDFORMAT_VISIBLE ParseSdfWorld(const sdf::World &_world, pxr::UsdStageRefPtr &_stage, const std::string &_path); } } diff --git a/usd/src/CMakeLists.txt b/usd/src/CMakeLists.txt index 1dc30bbab..76a5d32e2 100644 --- a/usd/src/CMakeLists.txt +++ b/usd/src/CMakeLists.txt @@ -1,4 +1,5 @@ set(sources + Error.cc sdf_parser/Light.cc sdf_parser/World.cc ) diff --git a/usd/src/Error.cc b/usd/src/Error.cc new file mode 100644 index 000000000..0f8e2cd09 --- /dev/null +++ b/usd/src/Error.cc @@ -0,0 +1,166 @@ +/* + * Copyright 2022 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 "sdf/usd/Error.hh" + +#include + +using namespace sdf::usd; + +class sdf::usd::Error::Implementation +{ + /// \brief The error code value. + public: + ErrorCode code = ErrorCode::NONE; + + /// \brief Description of the error. + public: + std::string message = ""; + + /// \brief File path where the error was raised. + public: + std::optional filePath = std::nullopt; + + /// \brief Line number in the file path where the error was raised. + public: + std::optional lineNumber = std::nullopt; + + public: + std::optional sdf_error = std::nullopt; +}; + +///////////////////////////////////////////////// +Error::Error() : dataPtr(ignition::utils::MakeImpl()) {} + +///////////////////////////////////////////////// +Error::Error(const ErrorCode _code, const std::string &_message) + : dataPtr(ignition::utils::MakeImpl()) +{ + this->dataPtr->code = _code; + this->dataPtr->message = _message; +} + +///////////////////////////////////////////////// +Error::Error(const ErrorCode _code, const std::string &_message, + const std::string &_filePath) + : dataPtr(ignition::utils::MakeImpl()) +{ + this->dataPtr->code = _code; + this->dataPtr->message = _message; + this->dataPtr->filePath = _filePath; +} + +///////////////////////////////////////////////// +Error::Error(const ErrorCode _code, const std::string &_message, + const std::string &_filePath, int _lineNumber) + : dataPtr(ignition::utils::MakeImpl()) +{ + this->dataPtr->code = _code; + this->dataPtr->message = _message; + this->dataPtr->filePath = _filePath; + this->dataPtr->lineNumber = _lineNumber; +} + +///////////////////////////////////////////////// +Error::Error(const sdf::Error &_sdf_error) + : dataPtr(ignition::utils::MakeImpl()) +{ + this->dataPtr->code = ErrorCode::SDF_ERROR; + this->dataPtr->sdf_error = _sdf_error; +} + +///////////////////////////////////////////////// +ErrorCode Error::Code() const { return this->dataPtr->code; } + +///////////////////////////////////////////////// +std::string Error::Message() const { return this->dataPtr->message; } + +///////////////////////////////////////////////// +std::optional Error::FilePath() const +{ + return this->dataPtr->filePath; +} + +///////////////////////////////////////////////// +void Error::SetFilePath(const std::string &_filePath) +{ + this->dataPtr->filePath = _filePath; +} + +///////////////////////////////////////////////// +std::optional Error::LineNumber() const +{ + return this->dataPtr->lineNumber; +} + +///////////////////////////////////////////////// +void Error::SetLineNumber(int _lineNumber) +{ + this->dataPtr->lineNumber = _lineNumber; +} + +///////////////////////////////////////////////// +std::optional Error::SdfError() const +{ + return this->dataPtr->sdf_error; +} + +///////////////////////////////////////////////// +Error::operator bool() const { return this->dataPtr->code != ErrorCode::NONE; } + +///////////////////////////////////////////////// +bool Error::operator==(const bool _value) const +{ + return ((this->dataPtr->code != ErrorCode::NONE) && _value) || + ((this->dataPtr->code == ErrorCode::NONE) && !_value); +} + +namespace sdf +{ +// Inline bracket to help doxygen filtering. +inline namespace SDF_VERSION_NAMESPACE +{ +namespace usd +{ + +///////////////////////////////////////////////// +std::ostream &operator<<(std::ostream &_out, const sdf::usd::Error &_err) +{ + if (_err.dataPtr->code == ErrorCode::SDF_ERROR) + { + _out << _err.dataPtr->sdf_error.value(); + return _out; + } + + std::string pathInfo = ""; + + if (_err.FilePath().has_value()) pathInfo += ":" + _err.FilePath().value(); + + if (_err.LineNumber().has_value()) + pathInfo += ":L" + std::to_string(_err.LineNumber().value()); + + if (!pathInfo.empty()) pathInfo = "[" + pathInfo + "]: "; + + _out << "Error Code " + << static_cast::type>( + _err.Code()) + << ": " << pathInfo << "Msg: " << _err.Message(); + return _out; +} +} // namespace usd +} // namespace SDF_VERSION_NAMESPACE +} // namespace sdf diff --git a/usd/src/sdf_parser/Light.cc b/usd/src/sdf_parser/Light.cc index b3a068cb6..b9eb1b8b0 100644 --- a/usd/src/sdf_parser/Light.cc +++ b/usd/src/sdf_parser/Light.cc @@ -42,11 +42,11 @@ inline namespace SDF_VERSION_NAMESPACE { // namespace usd { - sdf::Errors ParseSdfLight(const sdf::Light &_light, + sdf::usd::Errors ParseSdfLight(const sdf::Light &_light, pxr::UsdStageRefPtr &_stage, const std::string &_path) { const pxr::SdfPath sdfLightPath(_path); - sdf::Errors errors; + sdf::usd::Errors errors; switch (_light.Type()) { case sdf::LightType::POINT: diff --git a/usd/src/sdf_parser/World.cc b/usd/src/sdf_parser/World.cc index 390e3cefd..6be4724a8 100644 --- a/usd/src/sdf_parser/World.cc +++ b/usd/src/sdf_parser/World.cc @@ -43,10 +43,10 @@ inline namespace SDF_VERSION_NAMESPACE { // namespace usd { - sdf::Errors ParseSdfWorld(const sdf::World &_world, pxr::UsdStageRefPtr &_stage, + sdf::usd::Errors ParseSdfWorld(const sdf::World &_world, pxr::UsdStageRefPtr &_stage, const std::string &_path) { - sdf::Errors errors; + sdf::usd::Errors errors; _stage->SetMetadata(pxr::UsdGeomTokens->upAxis, pxr::UsdGeomTokens->z); _stage->SetEndTimeCode(100); _stage->SetMetadata(pxr::TfToken("metersPerUnit"), 1.0); From 8e2a32c0fe31a49f581949367899b885210e3293 Mon Sep 17 00:00:00 2001 From: Teo Koon Peng Date: Thu, 10 Feb 2022 09:58:30 +0800 Subject: [PATCH 03/13] rename to UsdError to avoid confusion Signed-off-by: Teo Koon Peng --- usd/include/sdf/usd/{Error.hh => UsdError.hh} | 20 ++++---- usd/include/sdf/usd/sdf_parser/Light.hh | 4 +- usd/include/sdf/usd/sdf_parser/World.hh | 4 +- usd/src/CMakeLists.txt | 2 +- usd/src/{Error.cc => UsdError.cc} | 46 +++++++++---------- usd/src/sdf_parser/Light.cc | 9 ++-- usd/src/sdf_parser/World.cc | 4 +- 7 files changed, 45 insertions(+), 44 deletions(-) rename usd/include/sdf/usd/{Error.hh => UsdError.hh} (91%) rename usd/src/{Error.cc => UsdError.cc} (71%) diff --git a/usd/include/sdf/usd/Error.hh b/usd/include/sdf/usd/UsdError.hh similarity index 91% rename from usd/include/sdf/usd/Error.hh rename to usd/include/sdf/usd/UsdError.hh index 25e4d96eb..25dae607a 100644 --- a/usd/include/sdf/usd/Error.hh +++ b/usd/include/sdf/usd/UsdError.hh @@ -48,7 +48,7 @@ namespace usd /// where an error toward the beginning of the vector can inform errors /// toward the end of the vector. /// \sa Errors -enum class ErrorCode +enum class UsdErrorCode { // \brief No error NONE = 0, @@ -69,18 +69,18 @@ enum class ErrorCode FAILED_PRIM_API_APPLY, }; -class IGNITION_SDFORMAT_VISIBLE Error +class IGNITION_SDFORMAT_VISIBLE UsdError { /// \brief Default constructor public: - Error(); + UsdError(); /// \brief Constructor. /// \param[in] _code The error code. /// \param[in] _message A description of the error. /// \sa ErrorCode. public: - Error(const ErrorCode _code, const std::string &_message); + UsdError(const UsdErrorCode _code, const std::string &_message); /// \brief Constructor. /// \param[in] _code The error code. @@ -88,7 +88,7 @@ class IGNITION_SDFORMAT_VISIBLE Error /// \param[in] _filePath The file path that is related to this error. /// \sa ErrorCode. public: - Error(const ErrorCode _code, const std::string &_message, + UsdError(const UsdErrorCode _code, const std::string &_message, const std::string &_filePath); /// \brief Constructor. @@ -99,20 +99,20 @@ class IGNITION_SDFORMAT_VISIBLE Error /// this error was raised. /// \sa ErrorCode. public: - Error(const ErrorCode _code, const std::string &_message, + UsdError(const UsdErrorCode _code, const std::string &_message, const std::string &_filePath, int _lineNumber); /// \brief Constructor. /// \param[in] _sdf_error Wrap a sdf error. /// \sa ErrorCode. public: - Error(const sdf::Error& _sdf_error); + explicit UsdError(const sdf::Error& _sdf_error); /// \brief Get the error code. /// \return An error code. /// \sa ErrorCode. public: - ErrorCode Code() const; + UsdErrorCode Code() const; /// \brief Get the error message, which is a description of the error. /// \return Error message. @@ -168,13 +168,13 @@ class IGNITION_SDFORMAT_VISIBLE Error /// \return Reference to the given output stream public: friend IGNITION_SDFORMAT_VISIBLE std::ostream &operator<<( - std::ostream &_out, const sdf::usd::Error &_err); + std::ostream &_out, const sdf::usd::UsdError &_err); /// \brief Private data pointer. IGN_UTILS_IMPL_PTR(dataPtr) }; -using Errors = std::vector; +using UsdErrors = std::vector; } // namespace usd } // namespace SDF_VERSION_NAMESPACE diff --git a/usd/include/sdf/usd/sdf_parser/Light.hh b/usd/include/sdf/usd/sdf_parser/Light.hh index 0996ada6b..621d48855 100644 --- a/usd/include/sdf/usd/sdf_parser/Light.hh +++ b/usd/include/sdf/usd/sdf_parser/Light.hh @@ -32,7 +32,7 @@ #include "sdf/config.hh" #include "sdf/system_util.hh" #include "sdf/Light.hh" -#include "../Error.hh" +#include "../UsdError.hh" namespace sdf { @@ -49,7 +49,7 @@ namespace sdf /// be a valid USD path. /// \return Errors, which is a vector of Error objects. Each Error includes /// an error code and message. An empty vector indicates no error. - sdf::usd::Errors SDFORMAT_VISIBLE ParseSdfLight(const sdf::Light &_light, + UsdErrors SDFORMAT_VISIBLE ParseSdfLight(const sdf::Light &_light, pxr::UsdStageRefPtr &_stage, const std::string &_path); } } diff --git a/usd/include/sdf/usd/sdf_parser/World.hh b/usd/include/sdf/usd/sdf_parser/World.hh index 8dcb11ae4..818406149 100644 --- a/usd/include/sdf/usd/sdf_parser/World.hh +++ b/usd/include/sdf/usd/sdf_parser/World.hh @@ -32,7 +32,7 @@ #include "sdf/config.hh" #include "sdf/system_util.hh" #include "sdf/World.hh" -#include "../Error.hh" +#include "../UsdError.hh" namespace sdf { @@ -49,7 +49,7 @@ namespace sdf /// a valid USD path. /// \return Errors, which is a vector of Error objects. Each Error includes /// an error code and message. An empty vector indicates no error. - sdf::usd::Errors SDFORMAT_VISIBLE ParseSdfWorld(const sdf::World &_world, + UsdErrors SDFORMAT_VISIBLE ParseSdfWorld(const sdf::World &_world, pxr::UsdStageRefPtr &_stage, const std::string &_path); } } diff --git a/usd/src/CMakeLists.txt b/usd/src/CMakeLists.txt index 76a5d32e2..623bdc9c5 100644 --- a/usd/src/CMakeLists.txt +++ b/usd/src/CMakeLists.txt @@ -1,5 +1,5 @@ set(sources - Error.cc + UsdError.cc sdf_parser/Light.cc sdf_parser/World.cc ) diff --git a/usd/src/Error.cc b/usd/src/UsdError.cc similarity index 71% rename from usd/src/Error.cc rename to usd/src/UsdError.cc index 0f8e2cd09..7bc3ef7e7 100644 --- a/usd/src/Error.cc +++ b/usd/src/UsdError.cc @@ -15,17 +15,17 @@ * */ -#include "sdf/usd/Error.hh" +#include "sdf/usd/UsdError.hh" #include using namespace sdf::usd; -class sdf::usd::Error::Implementation +class sdf::usd::UsdError::Implementation { /// \brief The error code value. public: - ErrorCode code = ErrorCode::NONE; + UsdErrorCode code = UsdErrorCode::NONE; /// \brief Description of the error. public: @@ -44,10 +44,10 @@ class sdf::usd::Error::Implementation }; ///////////////////////////////////////////////// -Error::Error() : dataPtr(ignition::utils::MakeImpl()) {} +UsdError::UsdError() : dataPtr(ignition::utils::MakeImpl()) {} ///////////////////////////////////////////////// -Error::Error(const ErrorCode _code, const std::string &_message) +UsdError::UsdError(const UsdErrorCode _code, const std::string &_message) : dataPtr(ignition::utils::MakeImpl()) { this->dataPtr->code = _code; @@ -55,7 +55,7 @@ Error::Error(const ErrorCode _code, const std::string &_message) } ///////////////////////////////////////////////// -Error::Error(const ErrorCode _code, const std::string &_message, +UsdError::UsdError(const UsdErrorCode _code, const std::string &_message, const std::string &_filePath) : dataPtr(ignition::utils::MakeImpl()) { @@ -65,7 +65,7 @@ Error::Error(const ErrorCode _code, const std::string &_message, } ///////////////////////////////////////////////// -Error::Error(const ErrorCode _code, const std::string &_message, +UsdError::UsdError(const UsdErrorCode _code, const std::string &_message, const std::string &_filePath, int _lineNumber) : dataPtr(ignition::utils::MakeImpl()) { @@ -76,57 +76,57 @@ Error::Error(const ErrorCode _code, const std::string &_message, } ///////////////////////////////////////////////// -Error::Error(const sdf::Error &_sdf_error) +UsdError::UsdError(const sdf::Error &_sdf_error) : dataPtr(ignition::utils::MakeImpl()) { - this->dataPtr->code = ErrorCode::SDF_ERROR; + this->dataPtr->code = UsdErrorCode::SDF_ERROR; this->dataPtr->sdf_error = _sdf_error; } ///////////////////////////////////////////////// -ErrorCode Error::Code() const { return this->dataPtr->code; } +UsdErrorCode UsdError::Code() const { return this->dataPtr->code; } ///////////////////////////////////////////////// -std::string Error::Message() const { return this->dataPtr->message; } +std::string UsdError::Message() const { return this->dataPtr->message; } ///////////////////////////////////////////////// -std::optional Error::FilePath() const +std::optional UsdError::FilePath() const { return this->dataPtr->filePath; } ///////////////////////////////////////////////// -void Error::SetFilePath(const std::string &_filePath) +void UsdError::SetFilePath(const std::string &_filePath) { this->dataPtr->filePath = _filePath; } ///////////////////////////////////////////////// -std::optional Error::LineNumber() const +std::optional UsdError::LineNumber() const { return this->dataPtr->lineNumber; } ///////////////////////////////////////////////// -void Error::SetLineNumber(int _lineNumber) +void UsdError::SetLineNumber(int _lineNumber) { this->dataPtr->lineNumber = _lineNumber; } ///////////////////////////////////////////////// -std::optional Error::SdfError() const +std::optional UsdError::SdfError() const { return this->dataPtr->sdf_error; } ///////////////////////////////////////////////// -Error::operator bool() const { return this->dataPtr->code != ErrorCode::NONE; } +UsdError::operator bool() const { return this->dataPtr->code != UsdErrorCode::NONE; } ///////////////////////////////////////////////// -bool Error::operator==(const bool _value) const +bool UsdError::operator==(const bool _value) const { - return ((this->dataPtr->code != ErrorCode::NONE) && _value) || - ((this->dataPtr->code == ErrorCode::NONE) && !_value); + return ((this->dataPtr->code != UsdErrorCode::NONE) && _value) || + ((this->dataPtr->code == UsdErrorCode::NONE) && !_value); } namespace sdf @@ -138,9 +138,9 @@ namespace usd { ///////////////////////////////////////////////// -std::ostream &operator<<(std::ostream &_out, const sdf::usd::Error &_err) +std::ostream &operator<<(std::ostream &_out, const sdf::usd::UsdError &_err) { - if (_err.dataPtr->code == ErrorCode::SDF_ERROR) + if (_err.dataPtr->code == UsdErrorCode::SDF_ERROR) { _out << _err.dataPtr->sdf_error.value(); return _out; @@ -156,7 +156,7 @@ std::ostream &operator<<(std::ostream &_out, const sdf::usd::Error &_err) if (!pathInfo.empty()) pathInfo = "[" + pathInfo + "]: "; _out << "Error Code " - << static_cast::type>( + << static_cast::type>( _err.Code()) << ": " << pathInfo << "Msg: " << _err.Message(); return _out; diff --git a/usd/src/sdf_parser/Light.cc b/usd/src/sdf_parser/Light.cc index b9eb1b8b0..c34513f16 100644 --- a/usd/src/sdf_parser/Light.cc +++ b/usd/src/sdf_parser/Light.cc @@ -42,11 +42,11 @@ inline namespace SDF_VERSION_NAMESPACE { // namespace usd { - sdf::usd::Errors ParseSdfLight(const sdf::Light &_light, + UsdErrors ParseSdfLight(const sdf::Light &_light, pxr::UsdStageRefPtr &_stage, const std::string &_path) { const pxr::SdfPath sdfLightPath(_path); - sdf::usd::Errors errors; + UsdErrors errors; switch (_light.Type()) { case sdf::LightType::POINT: @@ -64,8 +64,9 @@ namespace usd break; case sdf::LightType::INVALID: default: - errors.push_back(sdf::Error(sdf::ErrorCode::ATTRIBUTE_INCORRECT_TYPE, - "The light type that was given cannot be parsed to USD.")); + errors.push_back(UsdError(sdf::Error( + sdf::ErrorCode::ATTRIBUTE_INCORRECT_TYPE, + "The light type that was given cannot be parsed to USD."))); return errors; } diff --git a/usd/src/sdf_parser/World.cc b/usd/src/sdf_parser/World.cc index 6be4724a8..2db3f922f 100644 --- a/usd/src/sdf_parser/World.cc +++ b/usd/src/sdf_parser/World.cc @@ -43,10 +43,10 @@ inline namespace SDF_VERSION_NAMESPACE { // namespace usd { - sdf::usd::Errors ParseSdfWorld(const sdf::World &_world, pxr::UsdStageRefPtr &_stage, + UsdErrors ParseSdfWorld(const sdf::World &_world, pxr::UsdStageRefPtr &_stage, const std::string &_path) { - sdf::usd::Errors errors; + UsdErrors errors; _stage->SetMetadata(pxr::UsdGeomTokens->upAxis, pxr::UsdGeomTokens->z); _stage->SetEndTimeCode(100); _stage->SetMetadata(pxr::TfToken("metersPerUnit"), 1.0); From 3952a9a856e8aca6112f75aaa193d3147439e48b Mon Sep 17 00:00:00 2001 From: Teo Koon Peng Date: Thu, 10 Feb 2022 10:37:08 +0800 Subject: [PATCH 04/13] add generic error Signed-off-by: Teo Koon Peng --- include/sdf/Error.hh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/sdf/Error.hh b/include/sdf/Error.hh index fb920ec72..f7f835e35 100644 --- a/include/sdf/Error.hh +++ b/include/sdf/Error.hh @@ -147,6 +147,9 @@ namespace sdf /// \brief Merge include is unspported for the type of entity being /// included. MERGE_INCLUDE_UNSUPPORTED, + + /// \brief A generic error that is not an sdf error. + EXTERNAL_ERROR, }; class SDFORMAT_VISIBLE Error From 760fe4f54284a662fd5833978cf743c95e2a8924 Mon Sep 17 00:00:00 2001 From: Teo Koon Peng Date: Thu, 10 Feb 2022 15:33:31 +0800 Subject: [PATCH 05/13] fix header guard Signed-off-by: Teo Koon Peng --- usd/include/sdf/usd/UsdError.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/usd/include/sdf/usd/UsdError.hh b/usd/include/sdf/usd/UsdError.hh index 25dae607a..6604ba28f 100644 --- a/usd/include/sdf/usd/UsdError.hh +++ b/usd/include/sdf/usd/UsdError.hh @@ -14,8 +14,8 @@ * limitations under the License. * */ -#ifndef SDF_USD_ERROR_HH_ -#define SDF_USD_ERROR_HH_ +#ifndef SDF_USD_USDERROR_HH_ +#define SDF_USD_USDERROR_HH_ #include #include From fad9d72a740a797f08a24dd7845856a6be8abe23 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Thu, 10 Feb 2022 11:25:02 +0100 Subject: [PATCH 06/13] Fix style Signed-off-by: ahcorde --- usd/include/sdf/usd/UsdError.hh | 270 +++++++++++++++----------------- usd/src/UsdError.cc | 40 +++-- 2 files changed, 151 insertions(+), 159 deletions(-) diff --git a/usd/include/sdf/usd/UsdError.hh b/usd/include/sdf/usd/UsdError.hh index 6604ba28f..d853de3bb 100644 --- a/usd/include/sdf/usd/UsdError.hh +++ b/usd/include/sdf/usd/UsdError.hh @@ -35,149 +35,133 @@ namespace sdf { -// Inline bracket to help doxygen filtering. -inline namespace SDF_VERSION_NAMESPACE -{ -namespace usd -{ -// - -/// \enum ErrorCode -/// \brief Set of error codes. Usually one or more errors are returned in -/// an Errors vector. The collection of Errors should be take as a whole, -/// where an error toward the beginning of the vector can inform errors -/// toward the end of the vector. -/// \sa Errors -enum class UsdErrorCode -{ - // \brief No error - NONE = 0, - - /// \brief A wrapped SDF error. - SDF_ERROR, - - /// \brief Parsing a SDF object to a USD object failed. - /// This error type is specific to the USD component. - SDF_TO_USD_PARSING_ERROR, - - /// \brief The pxr::SdfPath does not point to a valid USD prim. - /// This error type is specific to the USD component. - INVALID_PRIM_PATH, - - /// \brief A pxr API was not able to be applied to a USD prim. - /// This error type is specific to the USD component. - FAILED_PRIM_API_APPLY, -}; - -class IGNITION_SDFORMAT_VISIBLE UsdError -{ - /// \brief Default constructor - public: - UsdError(); - - /// \brief Constructor. - /// \param[in] _code The error code. - /// \param[in] _message A description of the error. - /// \sa ErrorCode. - public: - UsdError(const UsdErrorCode _code, const std::string &_message); - - /// \brief Constructor. - /// \param[in] _code The error code. - /// \param[in] _message A description of the error. - /// \param[in] _filePath The file path that is related to this error. - /// \sa ErrorCode. - public: - UsdError(const UsdErrorCode _code, const std::string &_message, - const std::string &_filePath); - - /// \brief Constructor. - /// \param[in] _code The error code. - /// \param[in] _message A description of the error. - /// \param[in] _filePath The file path that is related to this error. - /// \param[in] _lineNumber The line number in the provided file path where - /// this error was raised. - /// \sa ErrorCode. - public: - UsdError(const UsdErrorCode _code, const std::string &_message, - const std::string &_filePath, int _lineNumber); - - /// \brief Constructor. - /// \param[in] _sdf_error Wrap a sdf error. - /// \sa ErrorCode. - public: - explicit UsdError(const sdf::Error& _sdf_error); - - /// \brief Get the error code. - /// \return An error code. - /// \sa ErrorCode. - public: - UsdErrorCode Code() const; - - /// \brief Get the error message, which is a description of the error. - /// \return Error message. - public: - std::string Message() const; - - /// \brief Get the file path associated with this error. - /// \return Returns the path of the file that this error is related to, - /// nullopt otherwise. - public: - std::optional FilePath() const; - - /// \brief Sets the file path that is associated with this error. - /// \param[in] _filePath The file path that is related to this error. (e.g. - /// /tmp/test_file.sdf) - public: - void SetFilePath(const std::string &_filePath); - - /// \brief Get the line number associated with this error. - /// \return Returns the line number. nullopt otherwise. - public: - std::optional LineNumber() const; - - /// \brief Sets the line number that is associated with this error. - /// \param[in] _lineNumber The line number that is related to this error. - public: - void SetLineNumber(int _lineNumber); - - /// \brief Get the underlying sdf error. - /// \return The underlying sdf error. - public: - std::optional SdfError() const; - - /// \brief Safe bool conversion. - /// \return True if this Error's Code() != NONE. In otherwords, this is - /// true when there is an error. - public: - explicit operator bool() const; - - /// \brief Compare this Error to a boolean value. - /// \return True if the boolean evaluation of this Error equals _value. - /// If _value == false, then true is returned when this Error's Code() is - /// equal to NONE and false is returned otherwise. If _value == true, - /// then true is returned when this Error's Code() is not equal to NONE - /// and false is returned otherwise. - /// \sa explicit operator bool() const - public: - bool operator==(const bool _value) const; - - /// \brief Output operator for an error. - /// \param[in,out] _out The output stream. - /// \param[in] _err The error to output. - /// \return Reference to the given output stream - public: - friend IGNITION_SDFORMAT_VISIBLE std::ostream &operator<<( - std::ostream &_out, const sdf::usd::UsdError &_err); - - /// \brief Private data pointer. - IGN_UTILS_IMPL_PTR(dataPtr) -}; - -using UsdErrors = std::vector; - -} // namespace usd -} // namespace SDF_VERSION_NAMESPACE + // Inline bracket to help doxygen filtering. + inline namespace SDF_VERSION_NAMESPACE + { + namespace usd + { + // + /// \enum ErrorCode + /// \brief Set of error codes. Usually one or more errors are returned in + /// an Errors vector. The collection of Errors should be take as a whole, + /// where an error toward the beginning of the vector can inform errors + /// toward the end of the vector. + /// \sa Errors + enum class UsdErrorCode + { + // \brief No error + NONE = 0, + + /// \brief A wrapped SDF error. + SDF_ERROR, + + /// \brief Parsing a SDF object to a USD object failed. + /// This error type is specific to the USD component. + SDF_TO_USD_PARSING_ERROR, + + /// \brief The pxr::SdfPath does not point to a valid USD prim. + /// This error type is specific to the USD component. + INVALID_PRIM_PATH, + + /// \brief A pxr API was not able to be applied to a USD prim. + /// This error type is specific to the USD component. + FAILED_PRIM_API_APPLY, + }; + + class IGNITION_SDFORMAT_VISIBLE UsdError + { + /// \brief Default constructor + public: UsdError(); + + /// \brief Constructor. + /// \param[in] _code The error code. + /// \param[in] _message A description of the error. + /// \sa ErrorCode. + public: UsdError(const UsdErrorCode _code, const std::string &_message); + + /// \brief Constructor. + /// \param[in] _code The error code. + /// \param[in] _message A description of the error. + /// \param[in] _filePath The file path that is related to this error. + /// \sa ErrorCode. + public: UsdError(const UsdErrorCode _code, const std::string &_message, + const std::string &_filePath); + + /// \brief Constructor. + /// \param[in] _code The error code. + /// \param[in] _message A description of the error. + /// \param[in] _filePath The file path that is related to this error. + /// \param[in] _lineNumber The line number in the provided file path where + /// this error was raised. + /// \sa ErrorCode. + public: UsdError(const UsdErrorCode _code, const std::string &_message, + const std::string &_filePath, int _lineNumber); + + /// \brief Constructor. + /// \param[in] _sdf_error Wrap a sdf error. + /// \sa ErrorCode. + public: explicit UsdError(const sdf::Error& _sdf_error); + + /// \brief Get the error code. + /// \return An error code. + /// \sa ErrorCode. + public: UsdErrorCode Code() const; + + /// \brief Get the error message, which is a description of the error. + /// \return Error message. + public: std::string Message() const; + + /// \brief Get the file path associated with this error. + /// \return Returns the path of the file that this error is related to, + /// nullopt otherwise. + public: std::optional FilePath() const; + + /// \brief Sets the file path that is associated with this error. + /// \param[in] _filePath The file path that is related to this error. (e.g. + /// /tmp/test_file.usd) + public: void SetFilePath(const std::string &_filePath); + + /// \brief Get the line number associated with this error. + /// \return Returns the line number. nullopt otherwise. + public: std::optional LineNumber() const; + + /// \brief Sets the line number that is associated with this error. + /// \param[in] _lineNumber The line number that is related to this error. + public: void SetLineNumber(int _lineNumber); + + /// \brief Get the underlying sdf error. + /// \return The underlying sdf error or nullopt otherwise. + public: std::optional SdfError() const; + + /// \brief Safe bool conversion. + /// \return True if this Error's Code() != NONE. In otherwords, this is + /// true when there is an error. + public: explicit operator bool() const; + + /// \brief Compare this Error to a boolean value. + /// \return True if the boolean evaluation of this Error equals _value. + /// If _value == false, then true is returned when this Error's Code() is + /// equal to NONE and false is returned otherwise. If _value == true, + /// then true is returned when this Error's Code() is not equal to NONE + /// and false is returned otherwise. + /// \sa explicit operator bool() const + public: bool operator==(const bool _value) const; + + /// \brief Output operator for an error. + /// \param[in,out] _out The output stream. + /// \param[in] _err The error to output. + /// \return Reference to the given output stream + public: friend IGNITION_SDFORMAT_VISIBLE std::ostream &operator<<( + std::ostream &_out, const sdf::usd::UsdError &_err); + + /// \brief Private data pointer. + IGN_UTILS_IMPL_PTR(dataPtr) + }; + + using UsdErrors = std::vector; + + } // namespace usd + } // namespace SDF_VERSION_NAMESPACE } // namespace sdf #ifdef _WIN32 diff --git a/usd/src/UsdError.cc b/usd/src/UsdError.cc index 7bc3ef7e7..fe37a014b 100644 --- a/usd/src/UsdError.cc +++ b/usd/src/UsdError.cc @@ -24,27 +24,26 @@ using namespace sdf::usd; class sdf::usd::UsdError::Implementation { /// \brief The error code value. - public: - UsdErrorCode code = UsdErrorCode::NONE; + public: UsdErrorCode code = UsdErrorCode::NONE; /// \brief Description of the error. - public: - std::string message = ""; + public: std::string message = ""; /// \brief File path where the error was raised. - public: - std::optional filePath = std::nullopt; + public: std::optional filePath = std::nullopt; /// \brief Line number in the file path where the error was raised. - public: - std::optional lineNumber = std::nullopt; + public: std::optional lineNumber = std::nullopt; - public: - std::optional sdf_error = std::nullopt; + /// \brief SDF error + public: std::optional sdf_error = std::nullopt; }; ///////////////////////////////////////////////// -UsdError::UsdError() : dataPtr(ignition::utils::MakeImpl()) {} +UsdError::UsdError() + : dataPtr(ignition::utils::MakeImpl()) +{ +} ///////////////////////////////////////////////// UsdError::UsdError(const UsdErrorCode _code, const std::string &_message) @@ -56,7 +55,7 @@ UsdError::UsdError(const UsdErrorCode _code, const std::string &_message) ///////////////////////////////////////////////// UsdError::UsdError(const UsdErrorCode _code, const std::string &_message, - const std::string &_filePath) + const std::string &_filePath) : dataPtr(ignition::utils::MakeImpl()) { this->dataPtr->code = _code; @@ -66,7 +65,7 @@ UsdError::UsdError(const UsdErrorCode _code, const std::string &_message, ///////////////////////////////////////////////// UsdError::UsdError(const UsdErrorCode _code, const std::string &_message, - const std::string &_filePath, int _lineNumber) + const std::string &_filePath, int _lineNumber) : dataPtr(ignition::utils::MakeImpl()) { this->dataPtr->code = _code; @@ -84,10 +83,16 @@ UsdError::UsdError(const sdf::Error &_sdf_error) } ///////////////////////////////////////////////// -UsdErrorCode UsdError::Code() const { return this->dataPtr->code; } +UsdErrorCode UsdError::Code() const +{ + return this->dataPtr->code; +} ///////////////////////////////////////////////// -std::string UsdError::Message() const { return this->dataPtr->message; } +std::string UsdError::Message() const +{ + return this->dataPtr->message; +} ///////////////////////////////////////////////// std::optional UsdError::FilePath() const @@ -120,7 +125,10 @@ std::optional UsdError::SdfError() const } ///////////////////////////////////////////////// -UsdError::operator bool() const { return this->dataPtr->code != UsdErrorCode::NONE; } +UsdError::operator bool() const +{ + return this->dataPtr->code != UsdErrorCode::NONE; +} ///////////////////////////////////////////////// bool UsdError::operator==(const bool _value) const From 3bd42e7058f371a18c9ebeffa33895b85db4a523 Mon Sep 17 00:00:00 2001 From: ahcorde Date: Thu, 10 Feb 2022 11:25:18 +0100 Subject: [PATCH 07/13] Added UsdError tests Signed-off-by: ahcorde --- usd/src/CMakeLists.txt | 1 + usd/src/UsdError_TEST.cc | 160 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+) create mode 100644 usd/src/UsdError_TEST.cc diff --git a/usd/src/CMakeLists.txt b/usd/src/CMakeLists.txt index 623bdc9c5..553a934fd 100644 --- a/usd/src/CMakeLists.txt +++ b/usd/src/CMakeLists.txt @@ -21,6 +21,7 @@ set(gtest_sources sdf_parser/sdf2usd_TEST.cc sdf_parser/Light_Sdf2Usd_TEST.cc sdf_parser/World_Sdf2Usd_TEST.cc + UsdError_TEST.cc ) # Build the unit tests diff --git a/usd/src/UsdError_TEST.cc b/usd/src/UsdError_TEST.cc new file mode 100644 index 000000000..fb667c356 --- /dev/null +++ b/usd/src/UsdError_TEST.cc @@ -0,0 +1,160 @@ +/* + * Copyright (C) 2022 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 +#include "sdf/sdf_config.h" +#include "sdf/usd/UsdError.hh" + +///////////////////////////////////////////////// +TEST(Error, DefaultConstruction) +{ + sdf::usd::UsdError error; + EXPECT_EQ(error, false); + EXPECT_EQ(error.Code(), sdf::usd::UsdErrorCode::NONE); + EXPECT_TRUE(error.Message().empty()); + EXPECT_FALSE(error.FilePath().has_value()); + EXPECT_FALSE(error.LineNumber().has_value()); + EXPECT_FALSE(error.SdfError().has_value()); + + if (error) + { + FAIL(); + } + error.SetFilePath("/usd/world"); + ASSERT_TRUE(error.FilePath().has_value()); + EXPECT_EQ("/usd/world", error.FilePath()); + + error.SetLineNumber(5); + ASSERT_TRUE(error.LineNumber().has_value()); + EXPECT_EQ(5, error.LineNumber()); +} + +///////////////////////////////////////////////// +TEST(Error, ValueConstructionWithoutFile) +{ + sdf::usd::UsdError error( + sdf::usd::UsdErrorCode::INVALID_PRIM_PATH, + "Invalid Prim path"); + EXPECT_EQ(error, true); + EXPECT_EQ(error.Code(), sdf::usd::UsdErrorCode::INVALID_PRIM_PATH); + EXPECT_EQ(error.Message(), "Invalid Prim path"); + EXPECT_FALSE(error.FilePath().has_value()); + EXPECT_FALSE(error.LineNumber().has_value()); + EXPECT_FALSE(error.SdfError().has_value()); + + if (!error) + FAIL(); +} + +///////////////////////////////////////////////// +TEST(Error, ValueConstructionWithFile) +{ + const std::string emptyFilePath = "Empty/file/path"; + sdf::usd::UsdError error( + sdf::usd::UsdErrorCode::INVALID_PRIM_PATH, + "Invalid Prim path", + emptyFilePath); + EXPECT_EQ(error, true); + EXPECT_EQ(error.Code(), sdf::usd::UsdErrorCode::INVALID_PRIM_PATH); + EXPECT_EQ(error.Message(), "Invalid Prim path"); + ASSERT_TRUE(error.FilePath().has_value()); + EXPECT_EQ(error.FilePath().value(), emptyFilePath); + EXPECT_FALSE(error.LineNumber().has_value()); + EXPECT_FALSE(error.SdfError().has_value()); + + if (!error) + FAIL(); +} + +///////////////////////////////////////////////// +TEST(Error, ValueConstructionWithLineNumber) +{ + const std::string emptyFilePath = "Empty/file/path"; + const int lineNumber = 10; + sdf::usd::UsdError error( + sdf::usd::UsdErrorCode::INVALID_PRIM_PATH, + "Invalid Prim path", + emptyFilePath, + lineNumber); + EXPECT_EQ(error, true); + EXPECT_EQ(error.Code(), sdf::usd::UsdErrorCode::INVALID_PRIM_PATH); + EXPECT_EQ(error.Message(), "Invalid Prim path"); + ASSERT_TRUE(error.FilePath().has_value()); + EXPECT_EQ(error.FilePath().value(), emptyFilePath); + ASSERT_TRUE(error.LineNumber().has_value()); + EXPECT_EQ(error.LineNumber().value(), lineNumber); + EXPECT_FALSE(error.SdfError().has_value()); + + if (!error) + FAIL(); +} + +///////////////////////////////////////////////// +TEST(Error, ValueConstructionWithPath) +{ + const std::string emptyFilePath = "Empty/file/path"; + const int lineNumber = 10; + sdf::usd::UsdError error( + sdf::usd::UsdErrorCode::INVALID_PRIM_PATH, + "Invalid Prim path", + emptyFilePath, + lineNumber); + EXPECT_EQ(error, true); + EXPECT_EQ(error.Code(), sdf::usd::UsdErrorCode::INVALID_PRIM_PATH); + EXPECT_EQ(error.Message(), "Invalid Prim path"); + ASSERT_TRUE(error.FilePath().has_value()); + EXPECT_EQ(error.FilePath().value(), emptyFilePath); + ASSERT_TRUE(error.LineNumber().has_value()); + EXPECT_EQ(error.LineNumber().value(), lineNumber); + EXPECT_FALSE(error.SdfError().has_value()); + + if (!error) + FAIL(); +} + +///////////////////////////////////////////////// +TEST(Error, sdfError) +{ + const std::string emptyFilePath = "Empty/file/sdfpath"; + + sdf::Error errorSdf( + sdf::ErrorCode::FILE_READ, + "Unable to read a file", + emptyFilePath); + + sdf::usd::UsdError error(errorSdf); + EXPECT_EQ(error, true); + EXPECT_EQ(error.Code(), sdf::usd::UsdErrorCode::SDF_ERROR); + EXPECT_EQ(error.Message(), ""); + EXPECT_FALSE(error.FilePath().has_value()); + EXPECT_FALSE(error.LineNumber().has_value()); + EXPECT_TRUE(error.SdfError().has_value()); + + auto errorSdf2 = error.SdfError(); + ASSERT_TRUE(errorSdf2.has_value()); + EXPECT_EQ(errorSdf2.value().Code(), sdf::ErrorCode::FILE_READ); + EXPECT_EQ(errorSdf2.value().Message(), "Unable to read a file"); + EXPECT_EQ(errorSdf2.value().FilePath().value(), emptyFilePath); + EXPECT_FALSE(errorSdf2.value().XmlPath().has_value()); + + if (!error) + FAIL(); + + if (!errorSdf2) + FAIL(); +} From 3e363662597247120de79e614ea6ebb050155da4 Mon Sep 17 00:00:00 2001 From: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> Date: Thu, 10 Feb 2022 11:16:54 -0700 Subject: [PATCH 08/13] cleanup Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> --- usd/include/sdf/usd/UsdError.hh | 16 +++++----- usd/include/sdf/usd/sdf_parser/Light.hh | 6 ++-- usd/include/sdf/usd/sdf_parser/World.hh | 10 +++---- usd/src/UsdError.cc | 6 ++-- usd/src/UsdError_TEST.cc | 39 +++++-------------------- 5 files changed, 27 insertions(+), 50 deletions(-) diff --git a/usd/include/sdf/usd/UsdError.hh b/usd/include/sdf/usd/UsdError.hh index d853de3bb..5c67f447e 100644 --- a/usd/include/sdf/usd/UsdError.hh +++ b/usd/include/sdf/usd/UsdError.hh @@ -17,15 +17,16 @@ #ifndef SDF_USD_USDERROR_HH_ #define SDF_USD_USDERROR_HH_ -#include -#include -#include -#include - #include #include #include +#include + +#include +#include +#include + #ifdef _WIN32 // Disable warning C4251 which is triggered by // std::string @@ -43,7 +44,7 @@ namespace sdf // /// \enum ErrorCode /// \brief Set of error codes. Usually one or more errors are returned in - /// an Errors vector. The collection of Errors should be take as a whole, + /// an Errors vector. The collection of Errors should be taken as a whole, /// where an error toward the beginning of the vector can inform errors /// toward the end of the vector. /// \sa Errors @@ -56,15 +57,12 @@ namespace sdf SDF_ERROR, /// \brief Parsing a SDF object to a USD object failed. - /// This error type is specific to the USD component. SDF_TO_USD_PARSING_ERROR, /// \brief The pxr::SdfPath does not point to a valid USD prim. - /// This error type is specific to the USD component. INVALID_PRIM_PATH, /// \brief A pxr API was not able to be applied to a USD prim. - /// This error type is specific to the USD component. FAILED_PRIM_API_APPLY, }; diff --git a/usd/include/sdf/usd/sdf_parser/Light.hh b/usd/include/sdf/usd/sdf_parser/Light.hh index 621d48855..76e7a5f1e 100644 --- a/usd/include/sdf/usd/sdf_parser/Light.hh +++ b/usd/include/sdf/usd/sdf_parser/Light.hh @@ -31,8 +31,8 @@ #include "sdf/config.hh" #include "sdf/system_util.hh" +#include "sdf/usd/UsdError.hh" #include "sdf/Light.hh" -#include "../UsdError.hh" namespace sdf { @@ -47,8 +47,8 @@ namespace sdf /// of _light. /// \param[in] _path The USD path of the parsed light in _stage, which must /// be a valid USD path. - /// \return Errors, which is a vector of Error objects. Each Error includes - /// an error code and message. An empty vector indicates no error. + /// \return UsdErrors, which is a vector of Error objects. Each Error + /// includes an error code and message. An empty vector indicates no error. UsdErrors SDFORMAT_VISIBLE ParseSdfLight(const sdf::Light &_light, pxr::UsdStageRefPtr &_stage, const std::string &_path); } diff --git a/usd/include/sdf/usd/sdf_parser/World.hh b/usd/include/sdf/usd/sdf_parser/World.hh index 818406149..f18e7927f 100644 --- a/usd/include/sdf/usd/sdf_parser/World.hh +++ b/usd/include/sdf/usd/sdf_parser/World.hh @@ -31,8 +31,8 @@ #include "sdf/config.hh" #include "sdf/system_util.hh" +#include "sdf/usd/UsdError.hh" #include "sdf/World.hh" -#include "../UsdError.hh" namespace sdf { @@ -45,10 +45,10 @@ namespace sdf /// \param[in] _world The SDF world to parse. /// \param[in] _stage The stage that should contain the USD representation /// of _world. It must be initialized first - /// \param[in] _path The USD path of the parsed world in _stage, which must be - /// a valid USD path. - /// \return Errors, which is a vector of Error objects. Each Error includes - /// an error code and message. An empty vector indicates no error. + /// \param[in] _path The USD path of the parsed world in _stage, which must + /// be a valid USD path. + /// \return UsdErrors, which is a vector of Error objects. Each Error + /// includes an error code and message. An empty vector indicates no error. UsdErrors SDFORMAT_VISIBLE ParseSdfWorld(const sdf::World &_world, pxr::UsdStageRefPtr &_stage, const std::string &_path); } diff --git a/usd/src/UsdError.cc b/usd/src/UsdError.cc index fe37a014b..241ef90cd 100644 --- a/usd/src/UsdError.cc +++ b/usd/src/UsdError.cc @@ -156,12 +156,14 @@ std::ostream &operator<<(std::ostream &_out, const sdf::usd::UsdError &_err) std::string pathInfo = ""; - if (_err.FilePath().has_value()) pathInfo += ":" + _err.FilePath().value(); + if (_err.FilePath().has_value()) + pathInfo += ":" + _err.FilePath().value(); if (_err.LineNumber().has_value()) pathInfo += ":L" + std::to_string(_err.LineNumber().value()); - if (!pathInfo.empty()) pathInfo = "[" + pathInfo + "]: "; + if (!pathInfo.empty()) + pathInfo = "[" + pathInfo + "]: "; _out << "Error Code " << static_cast::type>( diff --git a/usd/src/UsdError_TEST.cc b/usd/src/UsdError_TEST.cc index fb667c356..9c7a87689 100644 --- a/usd/src/UsdError_TEST.cc +++ b/usd/src/UsdError_TEST.cc @@ -24,7 +24,7 @@ TEST(Error, DefaultConstruction) { sdf::usd::UsdError error; - EXPECT_EQ(error, false); + EXPECT_FALSE(error); EXPECT_EQ(error.Code(), sdf::usd::UsdErrorCode::NONE); EXPECT_TRUE(error.Message().empty()); EXPECT_FALSE(error.FilePath().has_value()); @@ -50,7 +50,7 @@ TEST(Error, ValueConstructionWithoutFile) sdf::usd::UsdError error( sdf::usd::UsdErrorCode::INVALID_PRIM_PATH, "Invalid Prim path"); - EXPECT_EQ(error, true); + EXPECT_TRUE(error); EXPECT_EQ(error.Code(), sdf::usd::UsdErrorCode::INVALID_PRIM_PATH); EXPECT_EQ(error.Message(), "Invalid Prim path"); EXPECT_FALSE(error.FilePath().has_value()); @@ -69,7 +69,7 @@ TEST(Error, ValueConstructionWithFile) sdf::usd::UsdErrorCode::INVALID_PRIM_PATH, "Invalid Prim path", emptyFilePath); - EXPECT_EQ(error, true); + EXPECT_TRUE(error); EXPECT_EQ(error.Code(), sdf::usd::UsdErrorCode::INVALID_PRIM_PATH); EXPECT_EQ(error.Message(), "Invalid Prim path"); ASSERT_TRUE(error.FilePath().has_value()); @@ -91,30 +91,7 @@ TEST(Error, ValueConstructionWithLineNumber) "Invalid Prim path", emptyFilePath, lineNumber); - EXPECT_EQ(error, true); - EXPECT_EQ(error.Code(), sdf::usd::UsdErrorCode::INVALID_PRIM_PATH); - EXPECT_EQ(error.Message(), "Invalid Prim path"); - ASSERT_TRUE(error.FilePath().has_value()); - EXPECT_EQ(error.FilePath().value(), emptyFilePath); - ASSERT_TRUE(error.LineNumber().has_value()); - EXPECT_EQ(error.LineNumber().value(), lineNumber); - EXPECT_FALSE(error.SdfError().has_value()); - - if (!error) - FAIL(); -} - -///////////////////////////////////////////////// -TEST(Error, ValueConstructionWithPath) -{ - const std::string emptyFilePath = "Empty/file/path"; - const int lineNumber = 10; - sdf::usd::UsdError error( - sdf::usd::UsdErrorCode::INVALID_PRIM_PATH, - "Invalid Prim path", - emptyFilePath, - lineNumber); - EXPECT_EQ(error, true); + EXPECT_TRUE(error); EXPECT_EQ(error.Code(), sdf::usd::UsdErrorCode::INVALID_PRIM_PATH); EXPECT_EQ(error.Message(), "Invalid Prim path"); ASSERT_TRUE(error.FilePath().has_value()); @@ -132,23 +109,23 @@ TEST(Error, sdfError) { const std::string emptyFilePath = "Empty/file/sdfpath"; + const std::string sdfErrorMessage = "Unable to read a file"; sdf::Error errorSdf( sdf::ErrorCode::FILE_READ, - "Unable to read a file", + sdfErrorMessage, emptyFilePath); sdf::usd::UsdError error(errorSdf); - EXPECT_EQ(error, true); + EXPECT_TRUE(error); EXPECT_EQ(error.Code(), sdf::usd::UsdErrorCode::SDF_ERROR); EXPECT_EQ(error.Message(), ""); EXPECT_FALSE(error.FilePath().has_value()); EXPECT_FALSE(error.LineNumber().has_value()); - EXPECT_TRUE(error.SdfError().has_value()); auto errorSdf2 = error.SdfError(); ASSERT_TRUE(errorSdf2.has_value()); EXPECT_EQ(errorSdf2.value().Code(), sdf::ErrorCode::FILE_READ); - EXPECT_EQ(errorSdf2.value().Message(), "Unable to read a file"); + EXPECT_EQ(errorSdf2.value().Message(), sdfErrorMessage); EXPECT_EQ(errorSdf2.value().FilePath().value(), emptyFilePath); EXPECT_FALSE(errorSdf2.value().XmlPath().has_value()); From 20b4d82f5db4142cafaa580f40fa43f6c4fe71a2 Mon Sep 17 00:00:00 2001 From: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> Date: Thu, 10 Feb 2022 11:46:18 -0700 Subject: [PATCH 09/13] documentation nits and USD visibility macro Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> --- usd/include/sdf/usd/UsdError.hh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/usd/include/sdf/usd/UsdError.hh b/usd/include/sdf/usd/UsdError.hh index 5c67f447e..3856da092 100644 --- a/usd/include/sdf/usd/UsdError.hh +++ b/usd/include/sdf/usd/UsdError.hh @@ -24,7 +24,7 @@ #include #include -#include +#include #include #ifdef _WIN32 @@ -56,7 +56,7 @@ namespace sdf /// \brief A wrapped SDF error. SDF_ERROR, - /// \brief Parsing a SDF object to a USD object failed. + /// \brief Parsing of SDF object to a USD object failed. SDF_TO_USD_PARSING_ERROR, /// \brief The pxr::SdfPath does not point to a valid USD prim. @@ -66,7 +66,7 @@ namespace sdf FAILED_PRIM_API_APPLY, }; - class IGNITION_SDFORMAT_VISIBLE UsdError + class IGNITION_SDFORMAT_USD_VISIBLE UsdError { /// \brief Default constructor public: UsdError(); @@ -96,7 +96,7 @@ namespace sdf const std::string &_filePath, int _lineNumber); /// \brief Constructor. - /// \param[in] _sdf_error Wrap a sdf error. + /// \param[in] _sdf_error Wrap an sdf error. /// \sa ErrorCode. public: explicit UsdError(const sdf::Error& _sdf_error); From 851649b4db395da01b34660369269b980b4ef0d6 Mon Sep 17 00:00:00 2001 From: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> Date: Thu, 10 Feb 2022 13:00:14 -0700 Subject: [PATCH 10/13] remove EXTERNAL_ERROR type Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> --- include/sdf/Error.hh | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/sdf/Error.hh b/include/sdf/Error.hh index 7078d0b03..88558e279 100644 --- a/include/sdf/Error.hh +++ b/include/sdf/Error.hh @@ -147,9 +147,6 @@ namespace sdf /// \brief Merge include is unspported for the type of entity being /// included, or the custom parser does not support merge includes. MERGE_INCLUDE_UNSUPPORTED, - - /// \brief A generic error that is not an sdf error. - EXTERNAL_ERROR, }; class SDFORMAT_VISIBLE Error From 287fa31e0ddbdfcecd3309e108b459b00c21515f Mon Sep 17 00:00:00 2001 From: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> Date: Thu, 10 Feb 2022 16:08:28 -0700 Subject: [PATCH 11/13] review feedback Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> --- usd/include/sdf/usd/UsdError.hh | 20 ++++++++------------ usd/src/UsdError.cc | 27 +++++++++++++++++++-------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/usd/include/sdf/usd/UsdError.hh b/usd/include/sdf/usd/UsdError.hh index 3856da092..68cc1f4ee 100644 --- a/usd/include/sdf/usd/UsdError.hh +++ b/usd/include/sdf/usd/UsdError.hh @@ -27,13 +27,6 @@ #include #include -#ifdef _WIN32 -// Disable warning C4251 which is triggered by -// std::string -#pragma warning(push) -#pragma warning(disable : 4251) -#endif - namespace sdf { // Inline bracket to help doxygen filtering. @@ -72,13 +65,15 @@ namespace sdf public: UsdError(); /// \brief Constructor. - /// \param[in] _code The error code. + /// \param[in] _code The error code. If the error code is SDF_ERROR, the + /// constructor that takes an sdf::Error object should be used instead. /// \param[in] _message A description of the error. /// \sa ErrorCode. public: UsdError(const UsdErrorCode _code, const std::string &_message); /// \brief Constructor. - /// \param[in] _code The error code. + /// \param[in] _code The error code. If the error code is SDF_ERROR, the + /// constructor that takes an sdf::Error object should be used instead. /// \param[in] _message A description of the error. /// \param[in] _filePath The file path that is related to this error. /// \sa ErrorCode. @@ -86,7 +81,8 @@ namespace sdf const std::string &_filePath); /// \brief Constructor. - /// \param[in] _code The error code. + /// \param[in] _code The error code. If the error code is SDF_ERROR, the + /// constructor that takes an sdf::Error object should be used instead. /// \param[in] _message A description of the error. /// \param[in] _filePath The file path that is related to this error. /// \param[in] _lineNumber The line number in the provided file path where @@ -98,7 +94,7 @@ namespace sdf /// \brief Constructor. /// \param[in] _sdf_error Wrap an sdf error. /// \sa ErrorCode. - public: explicit UsdError(const sdf::Error& _sdf_error); + public: explicit UsdError(const sdf::Error& _sdfError); /// \brief Get the error code. /// \return An error code. @@ -107,7 +103,7 @@ namespace sdf /// \brief Get the error message, which is a description of the error. /// \return Error message. - public: std::string Message() const; + public: const std::string &Message() const; /// \brief Get the file path associated with this error. /// \return Returns the path of the file that this error is related to, diff --git a/usd/src/UsdError.cc b/usd/src/UsdError.cc index 241ef90cd..0d31b94f4 100644 --- a/usd/src/UsdError.cc +++ b/usd/src/UsdError.cc @@ -36,7 +36,7 @@ class sdf::usd::UsdError::Implementation public: std::optional lineNumber = std::nullopt; /// \brief SDF error - public: std::optional sdf_error = std::nullopt; + public: std::optional sdfError = std::nullopt; }; ///////////////////////////////////////////////// @@ -75,11 +75,11 @@ UsdError::UsdError(const UsdErrorCode _code, const std::string &_message, } ///////////////////////////////////////////////// -UsdError::UsdError(const sdf::Error &_sdf_error) +UsdError::UsdError(const sdf::Error &_sdfError) : dataPtr(ignition::utils::MakeImpl()) { this->dataPtr->code = UsdErrorCode::SDF_ERROR; - this->dataPtr->sdf_error = _sdf_error; + this->dataPtr->sdfError = _sdfError; } ///////////////////////////////////////////////// @@ -89,7 +89,7 @@ UsdErrorCode UsdError::Code() const } ///////////////////////////////////////////////// -std::string UsdError::Message() const +const std::string &UsdError::Message() const { return this->dataPtr->message; } @@ -121,7 +121,7 @@ void UsdError::SetLineNumber(int _lineNumber) ///////////////////////////////////////////////// std::optional UsdError::SdfError() const { - return this->dataPtr->sdf_error; + return this->dataPtr->sdfError; } ///////////////////////////////////////////////// @@ -148,16 +148,27 @@ namespace usd ///////////////////////////////////////////////// std::ostream &operator<<(std::ostream &_out, const sdf::usd::UsdError &_err) { - if (_err.dataPtr->code == UsdErrorCode::SDF_ERROR) + if (_err.Code() == UsdErrorCode::SDF_ERROR) { - _out << _err.dataPtr->sdf_error.value(); + // make sure that an SdfError was wrapped into the UsdError if the error + // code indicates an SDF error + if (_err.SdfError()) + { + _out << _err.SdfError().value(); + } + else + { + _out << "Error code is of type SDF_ERROR, but no sdf::Error object " + << "has been wrapped into this UsdError object."; + } + return _out; } std::string pathInfo = ""; if (_err.FilePath().has_value()) - pathInfo += ":" + _err.FilePath().value(); + pathInfo += _err.FilePath().value(); if (_err.LineNumber().has_value()) pathInfo += ":L" + std::to_string(_err.LineNumber().value()); From 4be0adaa92bc12505a14a647cc2183b0079ec8c3 Mon Sep 17 00:00:00 2001 From: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> Date: Fri, 11 Feb 2022 12:29:32 -0700 Subject: [PATCH 12/13] const ref return type for UsdError::FilePath Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> --- usd/include/sdf/usd/UsdError.hh | 2 +- usd/src/UsdError.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/usd/include/sdf/usd/UsdError.hh b/usd/include/sdf/usd/UsdError.hh index 68cc1f4ee..dc4517a29 100644 --- a/usd/include/sdf/usd/UsdError.hh +++ b/usd/include/sdf/usd/UsdError.hh @@ -108,7 +108,7 @@ namespace sdf /// \brief Get the file path associated with this error. /// \return Returns the path of the file that this error is related to, /// nullopt otherwise. - public: std::optional FilePath() const; + public: const std::optional &FilePath() const; /// \brief Sets the file path that is associated with this error. /// \param[in] _filePath The file path that is related to this error. (e.g. diff --git a/usd/src/UsdError.cc b/usd/src/UsdError.cc index 0d31b94f4..8d4cbe1e6 100644 --- a/usd/src/UsdError.cc +++ b/usd/src/UsdError.cc @@ -95,7 +95,7 @@ const std::string &UsdError::Message() const } ///////////////////////////////////////////////// -std::optional UsdError::FilePath() const +const std::optional &UsdError::FilePath() const { return this->dataPtr->filePath; } From 85962f6499b09cdbe607ee12ace9d21d86587e23 Mon Sep 17 00:00:00 2001 From: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> Date: Fri, 11 Feb 2022 13:00:14 -0700 Subject: [PATCH 13/13] syntax/code style fix Signed-off-by: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> Co-authored-by: Addisu Z. Taddese --- usd/include/sdf/usd/UsdError.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usd/include/sdf/usd/UsdError.hh b/usd/include/sdf/usd/UsdError.hh index dc4517a29..0da732696 100644 --- a/usd/include/sdf/usd/UsdError.hh +++ b/usd/include/sdf/usd/UsdError.hh @@ -94,7 +94,7 @@ namespace sdf /// \brief Constructor. /// \param[in] _sdf_error Wrap an sdf error. /// \sa ErrorCode. - public: explicit UsdError(const sdf::Error& _sdfError); + public: explicit UsdError(const sdf::Error &_sdfError); /// \brief Get the error code. /// \return An error code.