diff --git a/CMakeLists.txt b/CMakeLists.txt index af09e1629..aecdfcddb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -43,7 +43,7 @@ if (BUILD_SDF) gz_configure_project( NO_PROJECT_PREFIX REPLACE_INCLUDE_PATH sdf - VERSION_SUFFIX) + VERSION_SUFFIX pre1) ################################################# # Find tinyxml2. diff --git a/Changelog.md b/Changelog.md index b1332f0e8..1a14ddc43 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,6 +4,36 @@ ## libsdformat 13.X +### libsdformat 13.6.0 (2023-08-30) + +1. Use relative path in an urdf include to avoid confusion between internal and system headers + * [Pull request #1259](https://github.com/gazebosim/sdformat/pull/1259) + +1. parser.cc update calls to use sdf::Errors output + * [Pull request #1294](https://github.com/gazebosim/sdformat/pull/1294) + +1. Fix deeply nested merge-include for custom parsed files + * [Pull request #1293](https://github.com/gazebosim/sdformat/pull/1293) + +1. Updated findfile() to search localpath first + * [Pull request #1292](https://github.com/gazebosim/sdformat/pull/1292) + +1. World requires a scene and atmosphere + * [Pull request #1308](https://github.com/gazebosim/sdformat/pull/1308) + +1. Infrastructure + * [Pull request #1307](https://github.com/gazebosim/sdformat/pull/1307) + * [Pull request #1306](https://github.com/gazebosim/sdformat/pull/1306) + +1. Remove robot not found error when parsing fails + * [Pull request #1290](https://github.com/gazebosim/sdformat/pull/1290) + +1. Make some sdfdbg messages sdfmsgs + * [Pull request #1288](https://github.com/gazebosim/sdformat/pull/1288) + +1. Minor clean up of tests + * [Pull request #1289](https://github.com/gazebosim/sdformat/pull/1289) + ### libsdformat 13.5.0 (2023-05-18) 1. Added projector Python wrapper diff --git a/include/sdf/Error.hh b/include/sdf/Error.hh index 09d43e32b..0aace5ef7 100644 --- a/include/sdf/Error.hh +++ b/include/sdf/Error.hh @@ -223,6 +223,10 @@ namespace sdf /// \return Error message. public: std::string Message() const; + /// \brief Sets the message associated with this error. + /// \param [in] _message Message that describes this error. + public: void SetMessage(const std::string &_message); + /// \brief Get the file path associated with this error. /// \return Returns the path of the file that this error is related to, /// nullopt otherwise. diff --git a/include/sdf/parser.hh b/include/sdf/parser.hh index e919b72fa..a7dee740a 100644 --- a/include/sdf/parser.hh +++ b/include/sdf/parser.hh @@ -55,6 +55,14 @@ namespace sdf SDFORMAT_VISIBLE bool init(SDFPtr _sdf, const ParserConfig &_config); + /// \brief Initialize the SDF interface from the embedded root spec file + /// \param[out] _errors Vector of errors. + /// \param[out] _sdf Pointer to an SDF object. + /// \param[in] _config Custom parser configuration + /// \return True if successful. + SDFORMAT_VISIBLE + bool init(sdf::Errors &_errors, SDFPtr _sdf, const ParserConfig &_config); + /// \brief Initialize the SDF interface using a file /// \param[in] _filename Name of the SDF file /// \param[out] _sdf Pointer to an SDF object. @@ -71,9 +79,20 @@ namespace sdf bool initFile( const std::string &_filename, const ParserConfig &_config, SDFPtr _sdf); + /// \brief Initialize the SDF interface using a file + /// \param[in] _filename Name of the SDF file + /// \param[in] _config Custom parser configuration + /// \param[out] _sdf Pointer to an SDF object. + /// \param[out] _errors Vector of errors. + /// \return True if successful. + SDFORMAT_VISIBLE + bool initFile( + const std::string &_filename, const ParserConfig &_config, SDFPtr _sdf, + sdf::Errors &_errors); + /// \brief Initialize an SDF Element interface using a file /// \param[in] _filename Name of the SDF file - /// \param[in] _sdf Pointer to an SDF Element object. + /// \param[out] _sdf Pointer to an SDF Element object. /// \return True if successful. SDFORMAT_VISIBLE bool initFile(const std::string &_filename, ElementPtr _sdf); @@ -87,6 +106,16 @@ namespace sdf bool initFile(const std::string &_filename, const ParserConfig &_config, ElementPtr _sdf); + /// \brief Initialize an SDFElement interface using a file + /// \param[in] _filename Name of the SDF file + /// \param[in] _config Custom parser configuration + /// \param[out] _sdf Pointer to an SDF Element object. + /// \param[out] _errors Vector of errors. + /// \return True if successful. + SDFORMAT_VISIBLE + bool initFile(const std::string &_filename, const ParserConfig &_config, + ElementPtr _sdf, sdf::Errors &_errors); + /// \brief Initialize the SDF interface using a string /// \param[in] _xmlString XML string to be parsed. /// \param[out] _sdf Pointer to an SDF object. @@ -103,6 +132,16 @@ namespace sdf bool initString( const std::string &_xmlString, const ParserConfig &_config, SDFPtr _sdf); + /// \brief Initialize the SDF interface using a string + /// \param[in] _xmlString XML string to be parsed. + /// \param[in] _config Custom parser configuration + /// \param[out] _sdf Pointer to an SDF object. + /// \param[out] _errors Vector of errors. + /// \return True if successful. + SDFORMAT_VISIBLE + bool initString(const std::string &_xmlString, const ParserConfig &_config, + SDFPtr _sdf, sdf::Errors &_errors); + /// \brief Populate the SDF values from a file /// /// This populates a new SDF pointer from a file. If the file is a URDF @@ -317,6 +356,16 @@ namespace sdf SDFORMAT_VISIBLE std::string getModelFilePath(const std::string &_modelDirPath); + /// \brief Get the file path to the model file + /// \param[out] _errors Vector of errors. + /// \param[in] _modelDirPath directory system path of the model + /// \return string with the full filesystem path to the best version (greater + /// SDF protocol supported by this sdformat version) of the .sdf + /// model files hosted by _modelDirPath. + SDFORMAT_VISIBLE + std::string getModelFilePath(sdf::Errors &_errors, + const std::string &_modelDirPath); + /// \brief Convert an SDF file to a specific SDF version. /// \param[in] _filename Name of the SDF file to convert. /// \param[in] _version Version to convert _filename to. @@ -387,6 +436,17 @@ namespace sdf SDFORMAT_VISIBLE bool checkCanonicalLinkNames(const sdf::Root *_root); + /// \brief Check that for each model, the canonical_link attribute value + /// matches the name of a link in the model if the attribute is set and + /// not empty. + /// This checks recursively and should check the files exhaustively + /// rather than terminating early when the first error is found. + /// \param[out] _errors Vector of errors. + /// \param[in] _root SDF Root object to check recursively. + /// \return True if all models have valid canonical_link attributes. + SDFORMAT_VISIBLE + bool checkCanonicalLinkNames(sdf::Errors &_errors, const sdf::Root *_root); + /// \brief For the world and each model, check that the attached_to graphs /// build without errors and have no cycles. /// Confirm that following directed edges from each vertex in the graph @@ -398,6 +458,18 @@ namespace sdf SDFORMAT_VISIBLE bool checkFrameAttachedToGraph(const sdf::Root *_root); + /// \brief For the world and each model, check that the attached_to graphs + /// build without errors and have no cycles. + /// Confirm that following directed edges from each vertex in the graph + /// leads to a model, link, or world frame. + /// This checks recursively and should check the files exhaustively + /// rather than terminating early when the first error is found. + /// \param[out] _errors Vector of errors. + /// \param[in] _root SDF Root object to check recursively. + /// \return True if all attached_to graphs are valid. + SDFORMAT_VISIBLE + bool checkFrameAttachedToGraph(sdf::Errors &_errors, const sdf::Root *_root); + /// \brief Check that for each frame, the attached_to attribute value /// does not match its own frame name but does match the name of a /// link, joint, or other frame in the model if the attribute is set and @@ -409,6 +481,18 @@ namespace sdf SDFORMAT_VISIBLE bool checkFrameAttachedToNames(const sdf::Root *_root); + /// \brief Check that for each frame, the attached_to attribute value + /// does not match its own frame name but does match the name of a + /// link, joint, or other frame in the model if the attribute is set and + /// not empty. + /// This checks recursively and should check the files exhaustively + /// rather than terminating early when the first error is found. + /// \param[out] _errors Vector of errors. + /// \param[in] _root SDF Root object to check recursively. + /// \return True if all frames have valid attached_to attributes. + SDFORMAT_VISIBLE + bool checkFrameAttachedToNames(sdf::Errors &_errors, const sdf::Root *_root); + /// \brief Check that all joints in contained models specify parent /// and child link names that match the names of sibling links. /// This checks recursively and should check the files exhaustively @@ -469,6 +553,18 @@ namespace sdf SDFORMAT_VISIBLE bool checkPoseRelativeToGraph(const sdf::Root *_root); + /// \brief For the world and each model, check that the attached_to graphs + /// build without errors and have no cycles. + /// Confirm that following directed edges from each vertex in the graph + /// leads to a model, link, or world frame. + /// This checks recursively and should check the files exhaustively + /// rather than terminating early when the first error is found. + /// \param[out] _errors Vector of errors. + /// \param[in] _root SDF Root object to check recursively. + /// \return True if all attached_to graphs are valid. + SDFORMAT_VISIBLE + bool checkPoseRelativeToGraph(sdf::Errors &_errors, const sdf::Root *_root); + /// \brief Check that all sibling elements of the same type have unique names. /// This checks recursively and should check the files exhaustively /// rather than terminating early when the first duplicate name is found. @@ -478,6 +574,17 @@ namespace sdf SDFORMAT_VISIBLE bool recursiveSameTypeUniqueNames(sdf::ElementPtr _elem); + /// \brief Check that all sibling elements of the same type have unique names. + /// This checks recursively and should check the files exhaustively + /// rather than terminating early when the first duplicate name is found. + /// \param[out] _errors Vector of errors. + /// \param[in] _elem SDF Element to check recursively. + /// \return True if all contained elements have do not share a name with + /// sibling elements of the same type. + SDFORMAT_VISIBLE + bool recursiveSameTypeUniqueNames(sdf::Errors &_errors, + sdf::ElementPtr _elem); + /// \brief Check that all sibling elements of the any type have unique names. /// This checks recursively and should check the files exhaustively /// rather than terminating early when the first duplicate name is found. @@ -487,6 +594,16 @@ namespace sdf SDFORMAT_VISIBLE bool recursiveSiblingUniqueNames(sdf::ElementPtr _elem); + /// \brief Check that all sibling elements of the any type have unique names. + /// This checks recursively and should check the files exhaustively + /// rather than terminating early when the first duplicate name is found. + /// \param[out] _errors Vector of errors. + /// \param[in] _elem SDF Element to check recursively. + /// \return True if all contained elements have do not share a name with + /// sibling elements of any type. + SDFORMAT_VISIBLE + bool recursiveSiblingUniqueNames(sdf::Errors &_errors, sdf::ElementPtr _elem); + /// \brief Check that all sibling elements do not contain the delimiter /// double colons '::' in element names, which is reserved for forming scopes /// in SDFormat 1.8. This checks recursively and should check the files @@ -497,6 +614,18 @@ namespace sdf SDFORMAT_VISIBLE bool recursiveSiblingNoDoubleColonInNames(sdf::ElementPtr _elem); + /// \brief Check that all sibling elements do not contain the delimiter + /// double colons '::' in element names, which is reserved for forming scopes + /// in SDFormat 1.8. This checks recursively and should check the files + /// exhaustively rather than terminating early when the first name + /// containing '::' is found. + /// \param[out] _errors Vector of errors. + /// \param[in] _elem SDF Element to check recursively. + /// \return True if all contained element names do not have the delimiter '::' + SDFORMAT_VISIBLE + bool recursiveSiblingNoDoubleColonInNames(sdf::Errors &_errors, + sdf::ElementPtr _elem); + /// \brief Check whether the element should be validated. If this returns /// false, validators such as the unique name and reserve name checkers should /// skip this element and its descendants. diff --git a/src/Error.cc b/src/Error.cc index 8a8325d03..2045d26ff 100644 --- a/src/Error.cc +++ b/src/Error.cc @@ -84,6 +84,12 @@ std::string Error::Message() const return this->dataPtr->message; } +///////////////////////////////////////////////// +void Error::SetMessage(const std::string &_message) +{ + this->dataPtr->message = _message; +} + ///////////////////////////////////////////////// std::optional Error::FilePath() const { diff --git a/src/Root.cc b/src/Root.cc index 3230184de..03349d2af 100644 --- a/src/Root.cc +++ b/src/Root.cc @@ -197,7 +197,7 @@ Errors Root::Load(const std::string &_filename, const ParserConfig &_config) if (!sdfParsed) { errors.push_back( - {ErrorCode::FILE_READ, "Unable to read file:" + _filename}); + {ErrorCode::FILE_READ, "Unable to read file: [" + _filename + "]"}); return errors; } diff --git a/src/parser.cc b/src/parser.cc index 673f4ba75..84a7593d6 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -158,17 +158,19 @@ static bool isSdfFile(const std::string &_fileName) template static inline bool _initFile(const std::string &_filename, const ParserConfig &_config, - TPtr _sdf) + TPtr _sdf, + sdf::Errors &_errors) { auto xmlDoc = makeSdfDoc(); if (tinyxml2::XML_SUCCESS != xmlDoc.LoadFile(_filename.c_str())) { - sdferr << "Unable to load file[" - << _filename << "]: " << xmlDoc.ErrorStr() << "\n"; + _errors.emplace_back(sdf::Error(ErrorCode::FILE_READ, + "Unable to load file[" + _filename + + xmlDoc.ErrorStr() + "]")); return false; } - return initDoc(&xmlDoc, _config, _sdf); + return initDoc(_errors, _sdf, &xmlDoc, _config); } ////////////////////////////////////////////////// @@ -414,11 +416,21 @@ bool init(SDFPtr _sdf) ////////////////////////////////////////////////// bool init(SDFPtr _sdf, const ParserConfig &_config) +{ + sdf::Errors errors; + bool result = init(errors, _sdf, _config); + sdf::throwOrPrintErrors(errors); + return result; + +} + +////////////////////////////////////////////////// +bool init(sdf::Errors &_errors, SDFPtr _sdf, const ParserConfig &_config) { std::string xmldata = SDF::EmbeddedSpec("root.sdf", false); auto xmlDoc = makeSdfDoc(); xmlDoc.Parse(xmldata.c_str()); - return initDoc(&xmlDoc, _config, _sdf); + return initDoc(_errors, _sdf, &xmlDoc, _config); } ////////////////////////////////////////////////// @@ -428,18 +440,28 @@ bool initFile(const std::string &_filename, SDFPtr _sdf) } ////////////////////////////////////////////////// -bool initFile( - const std::string &_filename, const ParserConfig &_config, SDFPtr _sdf) +bool initFile(const std::string &_filename, const ParserConfig &_config, + SDFPtr _sdf) +{ + sdf::Errors errors; + bool result = initFile(_filename, _config, _sdf, errors); + sdf::throwOrPrintErrors(errors); + return result; +} + +////////////////////////////////////////////////// +bool initFile(const std::string &_filename, const ParserConfig &_config, + SDFPtr _sdf, sdf::Errors &_errors) { std::string xmldata = SDF::EmbeddedSpec(_filename, true); if (!xmldata.empty()) { auto xmlDoc = makeSdfDoc(); xmlDoc.Parse(xmldata.c_str()); - return initDoc(&xmlDoc, _config, _sdf); + return initDoc(_errors, _sdf, &xmlDoc, _config); } return _initFile(sdf::findFile(_filename, true, false, _config), _config, - _sdf); + _sdf, _errors); } ////////////////////////////////////////////////// @@ -451,51 +473,78 @@ bool initFile(const std::string &_filename, ElementPtr _sdf) ////////////////////////////////////////////////// bool initFile( const std::string &_filename, const ParserConfig &_config, ElementPtr _sdf) +{ + sdf::Errors errors; + bool result = initFile(_filename, _config, _sdf, errors); + sdf::throwOrPrintErrors(errors); + return result; +} + +////////////////////////////////////////////////// +bool initFile(const std::string &_filename, const ParserConfig &_config, + ElementPtr _sdf, sdf::Errors &_errors) { std::string xmldata = SDF::EmbeddedSpec(_filename, true); if (!xmldata.empty()) { auto xmlDoc = makeSdfDoc(); xmlDoc.Parse(xmldata.c_str()); - return initDoc(&xmlDoc, _config, _sdf); + return initDoc(_errors, _sdf, &xmlDoc, _config); } return _initFile(sdf::findFile(_filename, true, false, _config), _config, - _sdf); + _sdf, _errors); } ////////////////////////////////////////////////// -bool initString( - const std::string &_xmlString, const ParserConfig &_config, SDFPtr _sdf) +bool initString(const std::string &_xmlString, const ParserConfig &_config, + SDFPtr _sdf) +{ + sdf::Errors errors; + bool result = initString(_xmlString, _config, _sdf, errors); + sdf::throwOrPrintErrors(errors); + return result; +} + +////////////////////////////////////////////////// +bool initString(const std::string &_xmlString, const ParserConfig &_config, + SDFPtr _sdf, sdf::Errors &_errors) { auto xmlDoc = makeSdfDoc(); if (xmlDoc.Parse(_xmlString.c_str())) { - sdferr << "Failed to parse string as XML: " << xmlDoc.ErrorStr() << '\n'; + _errors.push_back({ErrorCode::PARSING_ERROR, "Failed to parse string" + " as XML: " + std::string(xmlDoc.ErrorStr())}); return false; } - return initDoc(&xmlDoc, _config, _sdf); + return initDoc(_errors, _sdf, &xmlDoc, _config); } ////////////////////////////////////////////////// bool initString(const std::string &_xmlString, SDFPtr _sdf) { - return initString(_xmlString, ParserConfig::GlobalConfig(), _sdf); + sdf::Errors errors; + bool result = initString(_xmlString, ParserConfig::GlobalConfig(), + _sdf, errors); + sdf::throwOrPrintErrors(errors); + return result; } ////////////////////////////////////////////////// -inline tinyxml2::XMLElement *_initDocGetElement(tinyxml2::XMLDocument *_xmlDoc) +inline tinyxml2::XMLElement *_initDocGetElement(tinyxml2::XMLDocument *_xmlDoc, + sdf::Errors &_errors) { if (!_xmlDoc) { - sdferr << "Could not parse the xml\n"; + _errors.push_back({ErrorCode::PARSING_ERROR, "Could not parse the xml"}); return nullptr; } tinyxml2::XMLElement *element = _xmlDoc->FirstChildElement("element"); if (!element) { - sdferr << "Could not find the 'element' element in the xml file\n"; + _errors.push_back({ErrorCode::ELEMENT_MISSING, "Could not find the " + "'element' element in the xml file"}); return nullptr; } @@ -503,37 +552,40 @@ inline tinyxml2::XMLElement *_initDocGetElement(tinyxml2::XMLDocument *_xmlDoc) } ////////////////////////////////////////////////// -bool initDoc(tinyxml2::XMLDocument *_xmlDoc, - const ParserConfig &_config, - SDFPtr _sdf) +bool initDoc(sdf::Errors &_errors, + SDFPtr _sdf, + tinyxml2::XMLDocument *_xmlDoc, + const ParserConfig &_config) { - auto element = _initDocGetElement(_xmlDoc); + auto element = _initDocGetElement(_xmlDoc, _errors); if (!element) { return false; } - return initXml(element, _config, _sdf->Root()); + return initXml(_errors, _sdf->Root(), element, _config); } ////////////////////////////////////////////////// -bool initDoc(tinyxml2::XMLDocument *_xmlDoc, - const ParserConfig &_config, - ElementPtr _sdf) +bool initDoc(sdf::Errors &_errors, + ElementPtr _sdf, + tinyxml2::XMLDocument *_xmlDoc, + const ParserConfig &_config) { - auto element = _initDocGetElement(_xmlDoc); + auto element = _initDocGetElement(_xmlDoc, _errors); if (!element) { return false; } - return initXml(element, _config, _sdf); + return initXml(_errors, _sdf, element, _config); } ////////////////////////////////////////////////// -bool initXml(tinyxml2::XMLElement *_xml, - const ParserConfig &_config, - ElementPtr _sdf) +bool initXml(sdf::Errors &_errors, + ElementPtr _sdf, + tinyxml2::XMLElement *_xml, + const ParserConfig &_config) { const char *refString = _xml->Attribute("ref"); if (refString) @@ -544,7 +596,8 @@ bool initXml(tinyxml2::XMLElement *_xml, const char *nameString = _xml->Attribute("name"); if (!nameString) { - sdferr << "Element is missing the name attribute\n"; + _errors.push_back({ErrorCode::ATTRIBUTE_MISSING, + "Element is missing the name attribute"}); return false; } _sdf->SetName(std::string(nameString)); @@ -552,7 +605,8 @@ bool initXml(tinyxml2::XMLElement *_xml, const char *requiredString = _xml->Attribute("required"); if (!requiredString) { - sdferr << "Element is missing the required attribute\n"; + _errors.push_back({ErrorCode::ATTRIBUTE_MISSING, + "Element is missing the required attribute"}); return false; } _sdf->SetRequired(requiredString); @@ -600,22 +654,27 @@ bool initXml(tinyxml2::XMLElement *_xml, if (!name) { - sdferr << "Attribute is missing a name\n"; + _errors.push_back({ErrorCode::ATTRIBUTE_INVALID, + "Attribute is missing a name"}); return false; } if (!type) { - sdferr << "Attribute is missing a type\n"; + _errors.push_back({ErrorCode::ATTRIBUTE_INVALID, + "Attribute is missing a type"}); return false; } if (!defaultValue) { - sdferr << "Attribute[" << name << "] is missing a default\n"; + _errors.push_back({ErrorCode::ATTRIBUTE_INVALID, + "Attribute[" + std::string(name) + + "] is missing a default"}); return false; } if (!requiredString) { - sdferr << "Attribute is missing a required string\n"; + _errors.push_back({ErrorCode::ATTRIBUTE_INVALID, + "Attribute is missing a required string"}); return false; } std::string requiredStr = sdf::trim(requiredString); @@ -651,7 +710,7 @@ bool initXml(tinyxml2::XMLElement *_xml, else { ElementPtr element(new Element); - initXml(child, _config, element); + initXml(_errors, element, child, _config); _sdf->AddElementDescription(element); } } @@ -765,26 +824,29 @@ bool readFileInternal(const std::string &_filename, const bool _convert, if (filename.empty()) { - sdferr << "Error finding file [" << _filename << "].\n"; + _errors.push_back({ErrorCode::FILE_READ, "Error finding file [" + + std::string(_filename) + "]."}); return false; } if (filesystem::is_directory(filename)) { - filename = getModelFilePath(filename); + filename = getModelFilePath(_errors, filename); } if (!filesystem::exists(filename)) { - sdferr << "File [" << filename << "] doesn't exist.\n"; + _errors.push_back({ErrorCode::FILE_READ, "File [" + + std::string(filename) + "] doesn't exist."}); return false; } auto error_code = xmlDoc.LoadFile(filename.c_str()); if (error_code) { - sdferr << "Error parsing XML in file [" << filename << "]: " - << xmlDoc.ErrorStr() << '\n'; + _errors.push_back({ErrorCode::FILE_READ, "Error parsing XML in file [" + + std::string(filename) + "]: " + + std::string(xmlDoc.ErrorStr())}); return false; } @@ -810,14 +872,16 @@ bool readFileInternal(const std::string &_filename, const bool _convert, } else { - sdferr << "Failed to parse the URDF file after converting to" - << " SDFormat.\n"; + _errors.push_back({ErrorCode::PARSING_ERROR, + "Failed to parse the URDF file after converting to SDFormat."}); return false; } } else { - sdferr << "XML does not seem to be an SDFormat or an URDF file.\n"; + _errors.push_back({ErrorCode::PARSING_ERROR, + "XML does not seem to be an SDFormat or an URDF file."}); + return false; } } @@ -873,7 +937,9 @@ bool readStringInternal(const std::string &_xmlString, const bool _convert, xmlDoc.Parse(_xmlString.c_str()); if (xmlDoc.Error()) { - sdferr << "Error parsing XML from string: " << xmlDoc.ErrorStr() << '\n'; + _errors.push_back({ErrorCode::STRING_READ, + "Error parsing XML from string: " + + std::string(xmlDoc.ErrorStr())}); return false; } tinyxml2::XMLElement *sdfXml = xmlDoc.FirstChildElement("sdf"); @@ -899,14 +965,15 @@ bool readStringInternal(const std::string &_xmlString, const bool _convert, } else { - sdferr << "Failed to parse the URDF file after converting to" - << " SDFormat\n"; + _errors.push_back({ErrorCode::PARSING_ERROR, + "Failed to parse the URDF file after converting to SDFormat."}); return false; } } else { - sdferr << "XML does not seem to be an SDFormat or an URDF string.\n"; + _errors.push_back({ErrorCode::PARSING_ERROR, + "XML does not seem to be an SDFormat or an URDF string."}); return false; } } @@ -940,7 +1007,9 @@ bool readString(const std::string &_xmlString, const ParserConfig &_config, xmlDoc.Parse(_xmlString.c_str()); if (xmlDoc.Error()) { - sdferr << "Error parsing XML from string: " << xmlDoc.ErrorStr() << '\n'; + _errors.push_back({ErrorCode::PARSING_ERROR, + "Error parsing XML from string: " + + std::string(xmlDoc.ErrorStr())}); return false; } if (readDoc(&xmlDoc, _sdf, std::string(kSdfStringSource), true, _config, @@ -950,8 +1019,9 @@ bool readString(const std::string &_xmlString, const ParserConfig &_config, } else { - sdferr << "parse as sdf version " << SDF::Version() << " failed, " - << "should try to parse as old deprecated format\n"; + _errors.push_back({ErrorCode::PARSING_ERROR, + "parse as sdf version " + SDF::Version() + " failed, " + "should try to parse as old deprecated format"}); return false; } } @@ -963,7 +1033,8 @@ bool readDoc(tinyxml2::XMLDocument *_xmlDoc, SDFPtr _sdf, { if (!_xmlDoc) { - sdfwarn << "Could not parse the xml from source[" << _source << "]\n"; + _errors.push_back({ErrorCode::WARNING, "Could not parse the xml" + " from source[" + _source + "]"}); return false; } @@ -977,7 +1048,8 @@ bool readDoc(tinyxml2::XMLDocument *_xmlDoc, SDFPtr _sdf, if (nullptr == _sdf || nullptr == _sdf->Root()) { - sdferr << "SDF pointer or its Root is null.\n"; + _errors.push_back({ErrorCode::PARSING_ERROR, + "SDF pointer or its Root is null."}); return false; } @@ -1037,7 +1109,7 @@ bool readDoc(tinyxml2::XMLDocument *_xmlDoc, SDFPtr _sdf, // delimiter '::' in element names not allowed in SDFormat >= 1.8 gz::math::SemanticVersion sdfVersion(_sdf->Root()->OriginalVersion()); if (sdfVersion >= gz::math::SemanticVersion(1, 8) - && !recursiveSiblingNoDoubleColonInNames(_sdf->Root())) + && !recursiveSiblingNoDoubleColonInNames(_errors, _sdf->Root())) { _errors.push_back({ErrorCode::RESERVED_NAME, "Delimiter '::' found in attribute names of element <" @@ -1063,7 +1135,7 @@ bool readDoc(tinyxml2::XMLDocument *_xmlDoc, ElementPtr _sdf, { if (!_xmlDoc) { - sdfwarn << "Could not parse the xml\n"; + _errors.push_back({ErrorCode::WARNING, "Could not parse the xml."}); return false; } @@ -1101,7 +1173,6 @@ bool readDoc(tinyxml2::XMLDocument *_xmlDoc, ElementPtr _sdf, && strcmp(sdfNode->Attribute("version"), SDF::Version().c_str()) != 0) { sdfdbg << "Converting a deprecated SDF source[" << _source << "].\n"; - Converter::Convert(_errors, _xmlDoc, SDF::Version(), _config); } @@ -1132,12 +1203,11 @@ bool readDoc(tinyxml2::XMLDocument *_xmlDoc, ElementPtr _sdf, // delimiter '::' in element names not allowed in SDFormat >= 1.8 gz::math::SemanticVersion sdfVersion(_sdf->OriginalVersion()); if (sdfVersion >= gz::math::SemanticVersion(1, 8) - && !recursiveSiblingNoDoubleColonInNames(_sdf)) + && !recursiveSiblingNoDoubleColonInNames(_errors, _sdf)) { _errors.push_back({ErrorCode::RESERVED_NAME, - "Delimiter '::' found in attribute names of element <" - + _sdf->GetName() + - ">, which is not allowed in SDFormat >= 1.8"}); + "Delimiter '::' found in attribute names of element <" + + _sdf->GetName() + ">, which is not allowed in SDFormat >= 1.8"}); return false; } } @@ -1198,8 +1268,9 @@ bool checkXmlFromRoot(tinyxml2::XMLElement *_xmlRoot, } ////////////////////////////////////////////////// -std::string getBestSupportedModelVersion(tinyxml2::XMLElement *_modelXML, - std::string &_modelFileName) +std::string getBestSupportedModelVersion(std::string &_modelFileName, + sdf::Errors &_errors, + tinyxml2::XMLElement *_modelXML) { tinyxml2::XMLElement *sdfXML = _modelXML->FirstChildElement("sdf"); tinyxml2::XMLElement *nameSearch = _modelXML->FirstChildElement("name"); @@ -1228,10 +1299,10 @@ std::string getBestSupportedModelVersion(tinyxml2::XMLElement *_modelXML, } else { - sdfwarn << "Ignoring version " << version - << " for model " << nameSearch->GetText() - << " because is newer than this sdf parser" - << " (version " << SDF_VERSION << ")\n"; + _errors.push_back({ErrorCode::WARNING, "Ignoring version " + + version + " for model " + nameSearch->GetText() + + " because is newer than this sdf parser" + + " (version " + SDF_VERSION + ")"}); } } } @@ -1240,8 +1311,10 @@ std::string getBestSupportedModelVersion(tinyxml2::XMLElement *_modelXML, if (!sdfXML || !sdfXML->GetText()) { - sdferr << "Failure to detect an sdf tag in the model config file" - << " for model: " << nameSearch->GetText() << "\n"; + _errors.push_back({ErrorCode::PARSING_ERROR, + "Failure to detect an sdf tag in the model " + "config file for model: " + + std::string(nameSearch->GetText())}); _modelFileName = ""; return ""; @@ -1249,11 +1322,13 @@ std::string getBestSupportedModelVersion(tinyxml2::XMLElement *_modelXML, if (!sdfXML->Attribute("version")) { - sdfwarn << "Can not find the XML attribute 'version'" - << " in sdf XML tag for model: " << nameSearch->GetText() << "." - << " Please specify the SDF protocol supported in the model" - << " configuration file. The first sdf tag in the config file" - << " will be used \n"; + _errors.push_back({ErrorCode::WARNING, + "Can not find the XML attribute 'version'" + " in sdf XML tag for model: " + + std::string(nameSearch->GetText()) + "." + " Please specify the SDF protocol supported in the model" + " configuration file. The first sdf tag in the config file" + " will be used "}); } _modelFileName = sdfXML->GetText(); @@ -1262,6 +1337,16 @@ std::string getBestSupportedModelVersion(tinyxml2::XMLElement *_modelXML, ////////////////////////////////////////////////// std::string getModelFilePath(const std::string &_modelDirPath) +{ + sdf::Errors errors; + std::string result = getModelFilePath(errors, _modelDirPath); + sdf::throwOrPrintErrors(errors); + return result; +} + +////////////////////////////////////////////////// +std::string getModelFilePath(sdf::Errors &_errors, + const std::string &_modelDirPath) { std::string configFilePath; @@ -1275,25 +1360,26 @@ std::string getModelFilePath(const std::string &_modelDirPath) if (!sdf::filesystem::exists(configFilePath)) { // We didn't find manifest.xml either, output an error and get out. - sdferr << "Could not find model.config or manifest.xml in [" - << _modelDirPath << "]\n"; + _errors.push_back({ErrorCode::FILE_READ, + "Could not find model.config or manifest.xml in [" + + _modelDirPath + "]"}); return std::string(); } else { // We found manifest.xml, but since it is deprecated print a warning. - sdfwarn << "The manifest.xml for a model is deprecated. " - << "Please rename manifest.xml to " - << "model.config" << ".\n"; + _errors.push_back({ErrorCode::WARNING, + "The manifest.xml for a model is deprecated. " + "Please rename manifest.xml to model.config."}); } } auto configFileDoc = makeSdfDoc(); if (tinyxml2::XML_SUCCESS != configFileDoc.LoadFile(configFilePath.c_str())) { - sdferr << "Error parsing XML in file [" - << configFilePath << "]: " - << configFileDoc.ErrorStr() << '\n'; + _errors.push_back({ErrorCode::PARSING_ERROR, + "Error parsing XML in file [" + configFilePath + + "]: " + configFileDoc.ErrorStr()}); return std::string(); } @@ -1301,12 +1387,14 @@ std::string getModelFilePath(const std::string &_modelDirPath) if (!modelXML) { - sdferr << "No element in configFile[" << configFilePath << "]\n"; + _errors.push_back({ErrorCode::PARSING_ERROR, + "No element in configFile[" + + configFilePath + "]"}); return std::string(); } std::string modelFileName; - if (getBestSupportedModelVersion(modelXML, modelFileName).empty()) + if (getBestSupportedModelVersion(modelFileName, _errors, modelXML).empty()) { return std::string(); } @@ -1472,7 +1560,7 @@ static bool resolveFileNameFromUri(tinyxml2::XMLElement *_includeXml, if (sdf::filesystem::is_directory(modelPath)) { // Get the model.config filename - _fileName = getModelFilePath(modelPath); + _fileName = getModelFilePath(_errors, modelPath); if (_fileName.empty()) { @@ -1588,7 +1676,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf, ElementPtr refSDF; refSDF.reset(new Element); std::string refFilename = refSDFStr + ".sdf"; - initFile(refFilename, _config, refSDF); + initFile(refFilename, _config, refSDF, _errors); _sdf->RemoveFromParent(); _sdf->Copy(refSDF); @@ -1667,7 +1755,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf, { Error err( ErrorCode::FILE_READ, - "Unable to read file[" + filename + "]", + "Unable to read file: [" + filename + "]", _source, uriElement->GetLineNum()); err.SetXmlPath(uriXmlPath); @@ -2146,28 +2234,36 @@ sdf::Errors convertString(SDFPtr _sdf, const std::string &_sdfString, ////////////////////////////////////////////////// bool checkCanonicalLinkNames(const sdf::Root *_root) +{ + sdf::Errors errors; + bool result = checkCanonicalLinkNames(errors, _root); + sdf::throwOrPrintErrors(errors); + return result; +} + +////////////////////////////////////////////////// +bool checkCanonicalLinkNames(sdf::Errors &_errors, const sdf::Root *_root) { if (!_root) { - std::cerr << "Error: invalid sdf::Root pointer, unable to " - << "check canonical link names." - << std::endl; + _errors.push_back({ErrorCode::FATAL_ERROR, "Error: invalid sdf::Root " + "pointer, unable to check canonical link names."}); return false; } bool result = true; - auto checkModelCanonicalLinkName = []( + auto checkModelCanonicalLinkName = [&_errors]( const sdf::Model *_model) -> bool { bool modelResult = true; std::string canonicalLink = _model->CanonicalLinkName(); if (!canonicalLink.empty() && !_model->LinkNameExists(canonicalLink)) { - std::cerr << "Error: canonical_link with name[" << canonicalLink - << "] not found in model with name[" << _model->Name() - << "]." - << std::endl; + _errors.push_back({ErrorCode::MODEL_CANONICAL_LINK_INVALID, + "Error: canonical_link with name[" + canonicalLink + + "] not found in model with name[" + _model->Name() + + "]."}); modelResult = false; } return modelResult; @@ -2193,10 +2289,19 @@ bool checkCanonicalLinkNames(const sdf::Root *_root) ////////////////////////////////////////////////// bool checkFrameAttachedToNames(const sdf::Root *_root) +{ + sdf::Errors errors; + bool result = checkFrameAttachedToNames(errors, _root); + sdf::throwOrPrintErrors(errors); + return result; +} + +////////////////////////////////////////////////// +bool checkFrameAttachedToNames(sdf::Errors &_errors, const sdf::Root *_root) { bool result = true; - auto checkModelFrameAttachedToNames = []( + auto checkModelFrameAttachedToNames = [&_errors]( const sdf::Model *_model) -> bool { bool modelResult = true; @@ -2214,12 +2319,11 @@ bool checkFrameAttachedToNames(const sdf::Root *_root) if (attachedTo == frame->Name()) { - std::cerr << "Error: attached_to name[" << attachedTo - << "] is identical to frame name[" << frame->Name() - << "], causing a graph cycle " - << "in model with name[" << _model->Name() - << "]." - << std::endl; + _errors.push_back({ErrorCode::FRAME_ATTACHED_TO_CYCLE, + "Error: attached_to name[" + attachedTo + + "] is identical to frame name[" + frame->Name() + + "], causing a graph cycle in model with name[" + + _model->Name() + "]."}); modelResult = false; } else if (!_model->LinkNameExists(attachedTo) && @@ -2227,19 +2331,19 @@ bool checkFrameAttachedToNames(const sdf::Root *_root) !_model->JointNameExists(attachedTo) && !_model->FrameNameExists(attachedTo)) { - std::cerr << "Error: attached_to name[" << attachedTo - << "] specified by frame with name[" << frame->Name() - << "] does not match a nested model, link, joint, " - << "or frame name in model with name[" << _model->Name() - << "]." - << std::endl; + _errors.push_back({ErrorCode::FRAME_ATTACHED_TO_INVALID, + "Error: attached_to name[" + attachedTo + + "] specified by frame with name[" + frame->Name() + + "] does not match a nested model, link, joint, " + "or frame name in model with name[" + + _model->Name() + "]."}); modelResult = false; } } return modelResult; }; - auto checkWorldFrameAttachedToNames = []( + auto checkWorldFrameAttachedToNames = [&_errors]( const sdf::World *_world) -> bool { auto findNameInWorld = [](const sdf::World *_inWorld, @@ -2287,22 +2391,20 @@ bool checkFrameAttachedToNames(const sdf::Root *_root) if (attachedTo == frame->Name()) { - std::cerr << "Error: attached_to name[" << attachedTo - << "] is identical to frame name[" << frame->Name() - << "], causing a graph cycle " - << "in world with name[" << _world->Name() - << "]." - << std::endl; + _errors.push_back({ErrorCode::FRAME_ATTACHED_TO_CYCLE, + "Error: attached_to name[" + attachedTo + + "] is identical to frame name[" + frame->Name() + + "], causing a graph cycle in world with name[" + + _world->Name() + "]."}); worldResult = false; } else if (!findNameInWorld(_world, attachedTo)) { - std::cerr << "Error: attached_to name[" << attachedTo - << "] specified by frame with name[" << frame->Name() - << "] does not match a model or frame name " - << "in world with name[" << _world->Name() - << "]." - << std::endl; + _errors.push_back({ErrorCode::FRAME_ATTACHED_TO_INVALID, + "Error: attached_to name[" + attachedTo + + "] specified by frame with name[" + frame->Name() + + "] does not match a model or frame name in world " + "with name[" + _world->Name() + "]."}); worldResult = false; } } @@ -2330,6 +2432,15 @@ bool checkFrameAttachedToNames(const sdf::Root *_root) ////////////////////////////////////////////////// bool recursiveSameTypeUniqueNames(sdf::ElementPtr _elem) +{ + sdf::Errors errors; + bool result = recursiveSameTypeUniqueNames(errors, _elem); + sdf::throwOrPrintErrors(errors); + return result; +} + +////////////////////////////////////////////////// +bool recursiveSameTypeUniqueNames(sdf::Errors &_errors, sdf::ElementPtr _elem) { if (!shouldValidateElement(_elem)) return true; @@ -2340,10 +2451,9 @@ bool recursiveSameTypeUniqueNames(sdf::ElementPtr _elem) { if (!_elem->HasUniqueChildNames(typeName)) { - std::cerr << "Error: Non-unique names detected in type " - << typeName << " in\n" - << _elem->ToString("") - << std::endl; + _errors.push_back({ErrorCode::DUPLICATE_NAME, + "Error: Non-unique names detected in type " + + typeName +" in\n" + _elem->ToString("")}); result = false; } } @@ -2351,7 +2461,7 @@ bool recursiveSameTypeUniqueNames(sdf::ElementPtr _elem) sdf::ElementPtr child = _elem->GetFirstElement(); while (child) { - result = recursiveSameTypeUniqueNames(child) && result; + result = recursiveSameTypeUniqueNames(_errors, child) && result; child = child->GetNextElement(); } @@ -2360,6 +2470,15 @@ bool recursiveSameTypeUniqueNames(sdf::ElementPtr _elem) ////////////////////////////////////////////////// bool recursiveSiblingUniqueNames(sdf::ElementPtr _elem) +{ + sdf::Errors errors; + bool result = recursiveSiblingUniqueNames(errors, _elem); + sdf::throwOrPrintErrors(errors); + return result; +} + +////////////////////////////////////////////////// +bool recursiveSiblingUniqueNames(sdf::Errors &_errors, sdf::ElementPtr _elem) { if (!shouldValidateElement(_elem)) return true; @@ -2368,16 +2487,16 @@ bool recursiveSiblingUniqueNames(sdf::ElementPtr _elem) _elem->HasUniqueChildNames("", Element::NameUniquenessExceptions()); if (!result) { - std::cerr << "Error: Non-unique names detected in " - << _elem->ToString("") - << std::endl; + _errors.push_back({ErrorCode::PARSING_ERROR, + "Error: Non-unique names detected in " + + _elem->ToString("")}); result = false; } sdf::ElementPtr child = _elem->GetFirstElement(); while (child) { - result = recursiveSiblingUniqueNames(child) && result; + result = recursiveSiblingUniqueNames(_errors, child) && result; child = child->GetNextElement(); } @@ -2386,6 +2505,16 @@ bool recursiveSiblingUniqueNames(sdf::ElementPtr _elem) ////////////////////////////////////////////////// bool recursiveSiblingNoDoubleColonInNames(sdf::ElementPtr _elem) +{ + sdf::Errors errors; + bool result = recursiveSiblingNoDoubleColonInNames(errors, _elem); + sdf::throwOrPrintErrors(errors); + return result; +} + +////////////////////////////////////////////////// +bool recursiveSiblingNoDoubleColonInNames(sdf::Errors &_errors, + sdf::ElementPtr _elem) { if (!shouldValidateElement(_elem)) return true; @@ -2394,16 +2523,16 @@ bool recursiveSiblingNoDoubleColonInNames(sdf::ElementPtr _elem) if (_elem->HasAttribute("name") && _elem->Get("name").find("::") != std::string::npos) { - std::cerr << "Error: Detected delimiter '::' in element name in\n" - << _elem->ToString("") - << std::endl; + _errors.push_back({ErrorCode::RESERVED_NAME, + "Error: Detected delimiter '::' in element name in" + + _elem->ToString("")}); result = false; } sdf::ElementPtr child = _elem->GetFirstElement(); while (child) { - result = recursiveSiblingNoDoubleColonInNames(child) && result; + result = recursiveSiblingNoDoubleColonInNames(_errors, child) && result; child = child->GetNextElement(); } @@ -2412,61 +2541,72 @@ bool recursiveSiblingNoDoubleColonInNames(sdf::ElementPtr _elem) ////////////////////////////////////////////////// bool checkFrameAttachedToGraph(const sdf::Root *_root) +{ + sdf::Errors errors; + bool result = checkFrameAttachedToGraph(errors, _root); + sdf::throwOrPrintErrors(errors); + return result; +} + +////////////////////////////////////////////////// +bool checkFrameAttachedToGraph(sdf::Errors &_errors, const sdf::Root *_root) { bool result = true; - auto checkModelFrameAttachedToGraph = []( + auto checkModelFrameAttachedToGraph = [&_errors]( const sdf::Model *_model) -> bool { bool modelResult = true; auto ownedGraph = std::make_shared(); sdf::ScopedGraph graph(ownedGraph); - auto errors = sdf::buildFrameAttachedToGraph(graph, _model); - if (!errors.empty()) + auto buildErrors = sdf::buildFrameAttachedToGraph(graph, _model); + if (!buildErrors.empty()) { - for (auto &error : errors) + for (auto &error : buildErrors) { - std::cerr << "Error: " << error.Message() << std::endl; + error.SetMessage("Error: " + error.Message()); + _errors.push_back(error); } modelResult = false; } - errors = sdf::validateFrameAttachedToGraph(graph); - if (!errors.empty()) + auto validateErrors = sdf::validateFrameAttachedToGraph(graph); + if (!validateErrors.empty()) { - for (auto &error : errors) + for (auto &error : validateErrors) { - std::cerr << "Error in validateFrameAttachedToGraph: " - << error.Message() - << std::endl; + error.SetMessage("Error in validateFrameAttachedToGraph: " + + error.Message()); + _errors.push_back(error); } modelResult = false; } return modelResult; }; - auto checkWorldFrameAttachedToGraph = []( + auto checkWorldFrameAttachedToGraph = [&_errors]( const sdf::World *_world) -> bool { bool worldResult = true; auto ownedGraph = std::make_shared(); sdf::ScopedGraph graph(ownedGraph); - auto errors = sdf::buildFrameAttachedToGraph(graph, _world); - if (!errors.empty()) + auto buildErrors = sdf::buildFrameAttachedToGraph(graph, _world); + if (!buildErrors.empty()) { - for (auto &error : errors) + for (auto &error : buildErrors) { - std::cerr << "Error: " << error.Message() << std::endl; + error.SetMessage("Error: " + error.Message()); + _errors.push_back(error); } worldResult = false; } - errors = sdf::validateFrameAttachedToGraph(graph); - if (!errors.empty()) + auto validateErrors = sdf::validateFrameAttachedToGraph(graph); + if (!validateErrors.empty()) { - for (auto &error : errors) + for (auto &error : validateErrors) { - std::cerr << "Error in validateFrameAttachedToGraph: " - << error.Message() - << std::endl; + error.SetMessage("Error in validateFrameAttachedToGraph: " + + error.Message()); + _errors.push_back(error); } worldResult = false; } @@ -2494,61 +2634,71 @@ bool checkFrameAttachedToGraph(const sdf::Root *_root) ////////////////////////////////////////////////// bool checkPoseRelativeToGraph(const sdf::Root *_root) +{ + sdf::Errors errors; + bool result = checkPoseRelativeToGraph(errors, _root); + sdf::throwOrPrintErrors(errors); + return result; +} + +////////////////////////////////////////////////// +bool checkPoseRelativeToGraph(sdf::Errors &_errors, const sdf::Root *_root) { bool result = true; - auto checkModelPoseRelativeToGraph = []( + auto checkModelPoseRelativeToGraph = [&_errors]( const sdf::Model *_model) -> bool { bool modelResult = true; auto ownedGraph = std::make_shared(); sdf::ScopedGraph graph(ownedGraph); - auto errors = sdf::buildPoseRelativeToGraph(graph, _model); - if (!errors.empty()) + auto buildErrors = sdf::buildPoseRelativeToGraph(graph, _model); + if (!buildErrors.empty()) { - for (auto &error : errors) + for (auto &error : buildErrors) { - std::cerr << "Error: " << error.Message() << std::endl; + error.SetMessage("Error: " + error.Message()); + _errors.push_back(error); } modelResult = false; } - errors = sdf::validatePoseRelativeToGraph(graph); - if (!errors.empty()) + auto validateErrors = sdf::validatePoseRelativeToGraph(graph); + if (!validateErrors.empty()) { - for (auto &error : errors) + for (auto &error : validateErrors) { - std::cerr << "Error in validatePoseRelativeToGraph: " - << error.Message() - << std::endl; + error.SetMessage("Error in validatePoseRelativeToGraph: " + + error.Message()); + _errors.push_back(error); } modelResult = false; } return modelResult; }; - auto checkWorldPoseRelativeToGraph = []( + auto checkWorldPoseRelativeToGraph = [&_errors]( const sdf::World *_world) -> bool { bool worldResult = true; auto ownedGraph = std::make_shared(); sdf::ScopedGraph graph(ownedGraph); - auto errors = sdf::buildPoseRelativeToGraph(graph, _world); - if (!errors.empty()) + auto buildErrors = sdf::buildPoseRelativeToGraph(graph, _world); + if (!buildErrors.empty()) { - for (auto &error : errors) + for (auto &error : buildErrors) { - std::cerr << "Error: " << error.Message() << std::endl; + error.SetMessage("Error: " + error.Message()); } worldResult = false; } - errors = sdf::validatePoseRelativeToGraph(graph); - if (!errors.empty()) + auto validateErrors = sdf::validatePoseRelativeToGraph(graph); + if (!validateErrors.empty()) { - for (auto &error : errors) + for (auto &error : validateErrors) { - std::cerr << "Error in validatePoseRelativeToGraph: " - << error.Message() - << std::endl; + error.SetMessage("Error in validatePoseRelativeToGraph: " + + error.Message()); + _errors.push_back(error); } worldResult = false; } diff --git a/src/parser_TEST.cc b/src/parser_TEST.cc index 10f87290f..7eb54d5e4 100644 --- a/src/parser_TEST.cc +++ b/src/parser_TEST.cc @@ -526,8 +526,14 @@ TEST(Parser, DoubleColonNameAttrError) sdf::Errors errors; EXPECT_FALSE(sdf::readString(stream.str(), sdf, errors)); - ASSERT_EQ(errors.size(), 1u); + ASSERT_EQ(errors.size(), 2u); EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::RESERVED_NAME); + EXPECT_NE(std::string::npos, errors[0].Message().find( + "Detected delimiter '::' in element name in")); + EXPECT_EQ(errors[1].Code(), sdf::ErrorCode::RESERVED_NAME); + EXPECT_NE(std::string::npos, errors[1].Message().find( + "Delimiter '::' found in attribute names of element ," + " which is not allowed in SDFormat >= 1.8")); sdf = InitSDF(); stream.str(""); @@ -858,9 +864,12 @@ TEST(Parser, ReadStringErrorNotSDForURDF) )"; sdf::Errors errors; EXPECT_FALSE(sdf::readString(testString, sdf, errors)); - ASSERT_EQ(errors.size(), 0u); - EXPECT_NE(std::string::npos, buffer.str().find( + ASSERT_EQ(errors.size(), 1u); + EXPECT_NE(std::string::npos, errors[0].Message().find( "XML does not seem to be an SDFormat or an URDF string.")); + + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); } ///////////////////////////////////////////////// @@ -885,9 +894,13 @@ TEST(Parser, ReadFileErrorNotSDForURDF) sdf::Errors errors; sdf::SDFPtr sdf = sdf::readFile(path, errors); - ASSERT_EQ(errors.size(), 0u); - EXPECT_NE(std::string::npos, buffer.str().find( + ASSERT_EQ(errors.size(), 1u); + EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::PARSING_ERROR); + EXPECT_NE(std::string::npos, errors[0].Message().find( "XML does not seem to be an SDFormat or an URDF file.")); + + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); } ///////////////////////////////////////////////// diff --git a/src/parser_private.hh b/src/parser_private.hh index 70a5a5c5d..031f6aa51 100644 --- a/src/parser_private.hh +++ b/src/parser_private.hh @@ -34,49 +34,57 @@ namespace sdf // /// \brief Get the best SDF version from models supported by this sdformat + /// \param[out] _modelFileName file name of the best model file + /// \param[out] _errors Vector of errors. /// \param[in] _modelXML XML element from config file pointing to the /// model XML tag - /// \param[out] _modelFileName file name of the best model file /// \return string with the best SDF version supported - std::string getBestSupportedModelVersion( - tinyxml2::XMLElement *_modelXML, std::string &_modelFileName); + std::string getBestSupportedModelVersion(std::string &_modelFileName, + sdf::Errors &_errors, + tinyxml2::XMLElement *_modelXML); /// \brief Initialize the SDF interface using a TinyXML2 document. /// /// This actually forwards to initXml after converting the inputs + /// \param[out] _errors Vector of errors. + /// \param[out] _sdf SDF interface to be initialized /// \param[in] _xmlDoc TinyXML2 document containing the SDFormat description /// file that corresponds with the input SDFPtr /// \param[in] _config Custom parser configuration - /// \param[out] _sdf SDF interface to be initialized /// \return True on success, false on error. - bool initDoc(tinyxml2::XMLDocument *_xmlDoc, - const ParserConfig &_config, - SDFPtr _sdf); + bool initDoc(sdf::Errors &_errors, + SDFPtr _sdf, + tinyxml2::XMLDocument *_xmlDoc, + const ParserConfig &_config); /// \brief Initialize the SDF Element using a TinyXML2 document /// /// This actually forwards to initXml after converting the inputs + /// \param[out] _errors Vector of errors. + /// \param[out] _sdf SDF Element to be initialized /// \param[in] _xmlDoc TinyXML2 document containing the SDFormat description /// file that corresponds with the input ElementPtr /// \param[in] _config Custom parser configuration - /// \param[out] _sdf SDF Element to be initialized /// \return True on success, false on error. - bool initDoc(tinyxml2::XMLDocument *_xmlDoc, - const ParserConfig &_config, - ElementPtr _sdf); + bool initDoc(sdf::Errors &_errors, + ElementPtr _sdf, + tinyxml2::XMLDocument *_xmlDoc, + const ParserConfig &_config); /// \brief Initialize the SDF Element by parsing the SDFormat description in /// the input TinyXML2 element. This is where SDFormat spec/description files /// are parsed /// \remark For internal use only. Do not use this function. + /// \param[out] _errors Vector of errors. + /// \param[out] _sdf SDF ElementPtr to be initialized /// \param[in] _xml TinyXML2 element containing the SDFormat description /// file that corresponds with the input ElementPtr /// \param[in] _config Custom parser configuration - /// \param[out] _sdf SDF ElementPtr to be initialized /// \return True on success, false on error. - bool initXml(tinyxml2::XMLElement *_xml, - const ParserConfig &_config, - ElementPtr _sdf); + bool initXml(sdf::Errors &_errors, + ElementPtr _sdf, + tinyxml2::XMLElement *_xml, + const ParserConfig &_config); /// \brief Populate the SDF values from a TinyXML document bool readDoc(tinyxml2::XMLDocument *_xmlDoc, SDFPtr _sdf, diff --git a/src/urdf/urdf_parser/check_urdf.cpp b/src/urdf/urdf_parser/check_urdf.cpp index 1222f2c05..50624c71a 100644 --- a/src/urdf/urdf_parser/check_urdf.cpp +++ b/src/urdf/urdf_parser/check_urdf.cpp @@ -35,7 +35,7 @@ /* Author: Wim Meeussen */ #pragma warning(push, 0) -#include "urdf_parser/urdf_parser.h" +#include "../urdf_parser/urdf_parser.h" #include #include diff --git a/src/urdf/urdf_parser/joint.cpp b/src/urdf/urdf_parser/joint.cpp index 48765718d..c00958ff9 100644 --- a/src/urdf/urdf_parser/joint.cpp +++ b/src/urdf/urdf_parser/joint.cpp @@ -41,7 +41,7 @@ #include // #include #include -#include +#include "../urdf_parser/urdf_parser.h" namespace urdf{ diff --git a/src/urdf/urdf_parser/link.cpp b/src/urdf/urdf_parser/link.cpp index 415c4ee5d..8c7555cfe 100644 --- a/src/urdf/urdf_parser/link.cpp +++ b/src/urdf/urdf_parser/link.cpp @@ -35,7 +35,7 @@ /* Author: Wim Meeussen */ #pragma warning(push, 0) -#include +#include "../urdf_parser/urdf_parser.h" #include #include #include diff --git a/src/urdf/urdf_parser/model.cpp b/src/urdf/urdf_parser/model.cpp index 804c39435..9ff9b15e9 100644 --- a/src/urdf/urdf_parser/model.cpp +++ b/src/urdf/urdf_parser/model.cpp @@ -36,7 +36,11 @@ #pragma warning(push, 0) #include -#include "urdf_parser/urdf_parser.h" +// Use relative path to avoid confusion with urdf system headers (if present). +// The change to the relative path is only required to be present in this +// file given how MSVC include headers. See: +// https://github.com/gazebosim/sdformat/pull/1259/files#r1149821498 +#include "../urdf_parser/urdf_parser.h" // #include #include diff --git a/src/urdf/urdf_parser/pose.cpp b/src/urdf/urdf_parser/pose.cpp index 32207e1be..a47d5c11e 100644 --- a/src/urdf/urdf_parser/pose.cpp +++ b/src/urdf/urdf_parser/pose.cpp @@ -41,7 +41,7 @@ #include // #include #include -#include +#include "../urdf_parser/urdf_parser.h" namespace urdf_export_helpers { diff --git a/src/urdf/urdf_parser/urdf_to_graphiz.cpp b/src/urdf/urdf_parser/urdf_to_graphiz.cpp index eae2c8229..9f72efd12 100644 --- a/src/urdf/urdf_parser/urdf_to_graphiz.cpp +++ b/src/urdf/urdf_parser/urdf_to_graphiz.cpp @@ -35,7 +35,7 @@ /* Author: Wim Meeussen */ #pragma warning(push, 0) -#include "urdf_parser/urdf_parser.h" +#include "../urdf_parser/urdf_parser.h" #include #include diff --git a/src/urdf/urdf_parser/world.cpp b/src/urdf/urdf_parser/world.cpp index c1dcb7d37..0f27bdf18 100644 --- a/src/urdf/urdf_parser/world.cpp +++ b/src/urdf/urdf_parser/world.cpp @@ -37,7 +37,7 @@ #include #include -#include +#include "../urdf_parser/urdf_parser.h" #include #include #include diff --git a/test/integration/includes.cc b/test/integration/includes.cc index 38a7b8167..adc80248e 100644 --- a/test/integration/includes.cc +++ b/test/integration/includes.cc @@ -55,9 +55,7 @@ TEST(IncludesTest, Includes) sdf::Root root; sdf::Errors errors = root.Load(worldFile); - for (auto e : errors) - std::cout << e.Message() << std::endl; - EXPECT_TRUE(errors.empty()); + EXPECT_TRUE(errors.empty()) << errors; ASSERT_NE(nullptr, root.Element()); EXPECT_EQ(worldFile, root.Element()->FilePath()); @@ -234,8 +232,6 @@ TEST(IncludesTest, Includes_15) sdf::Root root; sdf::Errors errors = root.Load(worldFile); - for (auto e : errors) - std::cout << e.Message() << std::endl; EXPECT_TRUE(errors.empty()); ASSERT_NE(nullptr, root.Element()); @@ -349,20 +345,19 @@ TEST(IncludesTest, IncludeModelMissingConfig) sdf::Errors errors; ASSERT_TRUE(sdf::readString(stream.str(), sdfParsed, errors)); - ASSERT_GE(1u, errors.size()); - EXPECT_EQ(1u, errors.size()); - std::cout << errors[0] << std::endl; - EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::URI_LOOKUP); - EXPECT_NE(std::string::npos, errors[0].Message().find( - "Unable to resolve uri[box_missing_config] to model path")) << errors[0]; + EXPECT_EQ(2u, errors.size()); + EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::FILE_READ); EXPECT_NE(std::string::npos, errors[0].Message().find( + "Could not find model.config or manifest.xml in")) << errors[0]; + EXPECT_EQ(errors[1].Code(), sdf::ErrorCode::URI_LOOKUP); + EXPECT_NE(std::string::npos, errors[1].Message().find( + "Unable to resolve uri[box_missing_config] to model path")) << errors[1]; + EXPECT_NE(std::string::npos, errors[1].Message().find( "box_missing_config] since it does not contain a model.config file")) - << errors[0]; + << errors[1]; sdf::Root root; errors = root.Load(sdfParsed); - for (auto e : errors) - std::cout << e.Message() << std::endl; EXPECT_TRUE(errors.empty()); EXPECT_EQ(nullptr, root.Model()); @@ -651,7 +646,23 @@ TEST(IncludesTest, MergeIncludePlacementFrame) ////////////////////////////////////////////////// TEST(IncludesTest, InvalidMergeInclude) { + // Redirect sdferr output + std::stringstream buffer; + sdf::testing::RedirectConsoleStream redir( + sdf::Console::Instance()->GetMsgStream(), &buffer); +#ifdef _WIN32 + sdf::Console::Instance()->SetQuiet(false); + sdf::testing::ScopeExit revertSetQuiet( + [] + { + sdf::Console::Instance()->SetQuiet(true); + }); +#endif + sdf::ParserConfig config; + // Set policies to Error to make sure nothing gets printed + config.SetWarningsPolicy(sdf::EnforcementPolicy::ERR); + config.SetUnrecognizedElementsPolicy(sdf::EnforcementPolicy::ERR); // Using the "file://" URI scheme to allow multiple search paths config.AddURIPath("file://", sdf::testing::TestFile("sdf")); config.AddURIPath("file://", sdf::testing::TestFile("integration", "model")); @@ -672,6 +683,7 @@ TEST(IncludesTest, InvalidMergeInclude) ASSERT_FALSE(errors.empty()); EXPECT_EQ(sdf::ErrorCode::MODEL_WITHOUT_LINK, errors[0].Code()); } + { const std::string sdfString = R"( @@ -737,29 +749,22 @@ TEST(IncludesTest, InvalidMergeInclude) )"; sdf::Root root; sdf::Errors errors = root.LoadSdfString(sdfString, config); - ASSERT_FALSE(errors.empty()); - EXPECT_EQ(sdf::ErrorCode::MERGE_INCLUDE_UNSUPPORTED, errors[0].Code()); - EXPECT_EQ("Merge-include does not support parent element of type frame", - errors[0].Message()); - EXPECT_EQ(5, errors[0].LineNumber()); + ASSERT_EQ(3, errors.size()); + EXPECT_EQ(sdf::ErrorCode::ELEMENT_INCORRECT_TYPE, errors[0].Code()); + EXPECT_NE(std::string::npos, errors[0].Message().find( + "child of element[sensor], not defined in SDF.")); + EXPECT_EQ(sdf::ErrorCode::ELEMENT_INCORRECT_TYPE, errors[1].Code()); + EXPECT_NE(std::string::npos, errors[1].Message().find( + "XML Element[unknown_element], child of element[model], not" + " defined in SDF. Copying[unknown_element] as children of [model]")); + EXPECT_EQ(sdf::ErrorCode::MERGE_INCLUDE_UNSUPPORTED, errors[2].Code()); + EXPECT_NE(std::string::npos, errors[2].Message().find( + "Merge-include does not support parent element of type frame")); + EXPECT_EQ(5, errors[2].LineNumber()); } // Syntax error in included file { - // Redirect sdferr output - std::stringstream buffer; - sdf::testing::RedirectConsoleStream redir( - sdf::Console::Instance()->GetMsgStream(), &buffer); -#ifdef _WIN32 - sdf::Console::Instance()->SetQuiet(false); - sdf::testing::ScopeExit revertSetQuiet( - [] - { - sdf::Console::Instance()->SetQuiet(true); - }); -#endif - - const std::string sdfString = R"( @@ -770,11 +775,24 @@ TEST(IncludesTest, InvalidMergeInclude) )"; sdf::Root root; sdf::Errors errors = root.LoadSdfString(sdfString, config); - ASSERT_FALSE(errors.empty()); - EXPECT_EQ(sdf::ErrorCode::FILE_READ, errors[0].Code()); - EXPECT_EQ(0u, errors[0].Message().find("Unable to read file")); - EXPECT_EQ(5, *errors[0].LineNumber()); - EXPECT_TRUE(buffer.str().find("Error parsing XML in file") != - std::string::npos) << buffer.str(); + ASSERT_EQ(errors.size(), 5u); + EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::FILE_READ); + EXPECT_NE(std::string::npos, + errors[0].Message().find("Error parsing XML in file")); + EXPECT_EQ(errors[1].Code(), sdf::ErrorCode::FILE_READ); + EXPECT_NE(std::string::npos, + errors[1].Message().find("Unable to read file")); + EXPECT_EQ(5, *errors[1].LineNumber()); + EXPECT_EQ(errors[2].Code(), sdf::ErrorCode::ELEMENT_INVALID); + EXPECT_NE(std::string::npos, + errors[2].Message().find("Error reading element ")); + EXPECT_EQ(errors[3].Code(), sdf::ErrorCode::ELEMENT_INVALID); + EXPECT_NE(std::string::npos, + errors[3].Message().find("Error reading element ")); + EXPECT_EQ(errors[4].Code(), sdf::ErrorCode::STRING_READ); + EXPECT_NE(std::string::npos, + errors[4].Message().find("Unable to read SDF string")); } + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); } diff --git a/test/integration/root_dom.cc b/test/integration/root_dom.cc index d6eb68aa6..69d682dd8 100644 --- a/test/integration/root_dom.cc +++ b/test/integration/root_dom.cc @@ -26,22 +26,57 @@ #include "sdf/Types.hh" #include "sdf/World.hh" #include "test_config.hh" +#include "test_utils.hh" ///////////////////////////////////////////////// TEST(DOMRoot, InvalidSDF) { + // Redirect sdferr output + std::stringstream buffer; + sdf::testing::RedirectConsoleStream redir( + sdf::Console::Instance()->GetMsgStream(), &buffer); +#ifdef _WIN32 + sdf::Console::Instance()->SetQuiet(false); + sdf::testing::ScopeExit revertSetQuiet( + [] + { + sdf::Console::Instance()->SetQuiet(true); + }); +#endif + const std::string testFile = sdf::testing::TestFile("sdf", "empty_invalid.sdf"); sdf::Root root; sdf::Errors errors = root.Load(testFile); - EXPECT_FALSE(errors.empty()); - EXPECT_EQ(sdf::ErrorCode::FILE_READ, errors[0].Code()); + EXPECT_EQ(errors.size(), 2u); + EXPECT_EQ(sdf::ErrorCode::PARSING_ERROR, errors[0].Code()); + EXPECT_NE(std::string::npos, errors[0].Message().find( + "XML does not seem to be an SDFormat or an URDF file.")); + EXPECT_EQ(sdf::ErrorCode::FILE_READ, errors[1].Code()); + EXPECT_NE(std::string::npos, errors[1].Message().find( + "Unable to read file")); + + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); } ///////////////////////////////////////////////// TEST(DOMRoot, NoVersion) { + // Redirect sdferr output + std::stringstream buffer; + sdf::testing::RedirectConsoleStream redir( + sdf::Console::Instance()->GetMsgStream(), &buffer); +#ifdef _WIN32 + sdf::Console::Instance()->SetQuiet(false); + sdf::testing::ScopeExit revertSetQuiet( + [] + { + sdf::Console::Instance()->SetQuiet(true); + }); +#endif + const std::string testFile = sdf::testing::TestFile("sdf", "empty_noversion.sdf"); @@ -49,11 +84,27 @@ TEST(DOMRoot, NoVersion) sdf::Errors errors = root.Load(testFile); EXPECT_FALSE(errors.empty()); EXPECT_EQ(sdf::ErrorCode::FILE_READ, errors[0].Code()); + + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); } ///////////////////////////////////////////////// TEST(DOMRoot, Load) { + // Redirect sdferr output + std::stringstream buffer; + sdf::testing::RedirectConsoleStream redir( + sdf::Console::Instance()->GetMsgStream(), &buffer); +#ifdef _WIN32 + sdf::Console::Instance()->SetQuiet(false); + sdf::testing::ScopeExit revertSetQuiet( + [] + { + sdf::Console::Instance()->SetQuiet(true); + }); +#endif + const std::string testFile = sdf::testing::TestFile("sdf", "empty.sdf"); @@ -72,11 +123,27 @@ TEST(DOMRoot, Load) ASSERT_TRUE(root.WorldByIndex(0)->ModelByIndex(0) != nullptr); EXPECT_EQ("ground_plane", root.WorldByIndex(0)->ModelByIndex(0)->Name()); EXPECT_TRUE(root.WorldByIndex(0)->ModelNameExists("ground_plane")); + + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); } ///////////////////////////////////////////////// TEST(DOMRoot, LoadMultipleModels) { + // Redirect sdferr output + std::stringstream buffer; + sdf::testing::RedirectConsoleStream redir( + sdf::Console::Instance()->GetMsgStream(), &buffer); +#ifdef _WIN32 + sdf::Console::Instance()->SetQuiet(false); + sdf::testing::ScopeExit revertSetQuiet( + [] + { + sdf::Console::Instance()->SetQuiet(true); + }); +#endif + const std::string testFile = sdf::testing::TestFile("sdf", "root_multiple_models.sdf"); @@ -92,11 +159,26 @@ TEST(DOMRoot, LoadMultipleModels) ASSERT_NE(nullptr, root.Model()); EXPECT_EQ("robot1", root.Model()->Name()); + + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); } ///////////////////////////////////////////////// TEST(DOMRoot, LoadDuplicateModels) { + // Redirect sdferr output + std::stringstream buffer; + sdf::testing::RedirectConsoleStream redir( + sdf::Console::Instance()->GetMsgStream(), &buffer); +#ifdef _WIN32 + sdf::Console::Instance()->SetQuiet(false); + sdf::testing::ScopeExit revertSetQuiet( + [] + { + sdf::Console::Instance()->SetQuiet(true); + }); +#endif const std::string testFile = sdf::testing::TestFile("sdf", "root_duplicate_models.sdf"); @@ -108,6 +190,9 @@ TEST(DOMRoot, LoadDuplicateModels) EXPECT_NE(nullptr, root.Model()); EXPECT_EQ("robot1", root.Model()->Name()); + + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); } /////////////////////////////////////////////////