From 9d83fe58b08eb35653fb7fd631df8344c68bcb5c Mon Sep 17 00:00:00 2001 From: "Marco A. Gutierrez" Date: Fri, 24 Mar 2023 10:32:26 +0800 Subject: [PATCH 1/2] Plugin: update calls to use sdf::Errors output (#1144) Signed-off-by: Marco A. Gutierrez --- include/sdf/Error.hh | 2 +- include/sdf/Plugin.hh | 51 +++++++++++++++++++++++++++++++- src/Plugin.cc | 69 +++++++++++++++++++++++++++++++++++-------- src/Plugin_TEST.cc | 28 ++++++++++++++++++ 4 files changed, 135 insertions(+), 15 deletions(-) diff --git a/include/sdf/Error.hh b/include/sdf/Error.hh index 5db10b399..50178747c 100644 --- a/include/sdf/Error.hh +++ b/include/sdf/Error.hh @@ -167,7 +167,7 @@ namespace sdf /// This has been created to help preserve behavior. FATAL_ERROR, - /// \brief Generic warning saved as error due to WarningsPolicy config + /// \brief Generic warning saved as error due to WarningsPolicy config. WARNING, /// \brief The joint axis expressed-in value does not match the name of an diff --git a/include/sdf/Plugin.hh b/include/sdf/Plugin.hh index 05fbc8a52..b7f790e40 100644 --- a/include/sdf/Plugin.hh +++ b/include/sdf/Plugin.hh @@ -68,6 +68,18 @@ namespace sdf public: Plugin(const std::string &_filename, const std::string &_name, const std::string &_xmlContent = ""); + /// \brief A constructor that initializes the plugin's filename, name, and + /// optionally the content. + /// \param[out] _errors Vector of errors. + /// \param[in] _filename Filename of the shared library associated with + /// this plugin. + /// \param[in] _name The name of the plugin. + /// \param[in] _xmlContent Optional XML content that will be stored in + /// this plugin. + public: Plugin(sdf::Errors &_errors, const std::string &_filename, + const std::string &_name, + const std::string &_xmlContent = ""); + /// \brief Load the plugin based on a element pointer. This is *not* the /// usual entry point. Typical usage of the SDF DOM is through the Root /// object. @@ -107,6 +119,14 @@ namespace sdf /// \param[in] _elem Element to insert. public: void InsertContent(const sdf::ElementPtr _elem); + /// \brief Insert an element into the plugin content. This does not + /// modify the values in the sdf::ElementPtr returned by the `Element()` + /// function. + /// \param[out] _errors Vector of errors. + /// \param[in] _elem Element to insert. + public: void InsertContent(sdf::Errors &_errors, + const sdf::ElementPtr _elem); + /// \brief Insert XML content into this plugin. This function does not /// modify the values in the sdf::ElementPtr returned by the `Element()` /// function. The provided content must be valid XML. @@ -116,6 +136,17 @@ namespace sdf /// content of this plugin is not modified. True otherwise public: bool InsertContent(const std::string _content); + /// \brief Insert XML content into this plugin. This function does not + /// modify the values in the sdf::ElementPtr returned by the `Element()` + /// function. The provided content must be valid XML. + /// \param[out] _errors Vector of errors. + /// \param[in] _content A string that contains valid XML. The XML is + /// inserted into this plugin if it is valid. + /// \return False if the provided content was invalid, in which case the + /// content of this plugin is not modified. True otherwise + public: bool InsertContent(sdf::Errors &_errors, + const std::string _content); + /// \brief Set the filename of the shared library. /// \param[in] _filename Filename of the shared library associated with /// this plugin. @@ -134,6 +165,14 @@ namespace sdf /// \return SDF element pointer with updated plugin values. public: sdf::ElementPtr ToElement() const; + /// \brief Create and return an SDF element filled with data from this + /// plugin. + /// Note that parameter passing functionality is not captured with this + /// function. + /// \param[out] _errors Vector of errors. + /// \return SDF element pointer with updated plugin values. + public: sdf::ElementPtr ToElement(sdf::Errors &_errors) const; + /// \brief Copy assignment operator /// \param[in] _plugin Plugin to copy /// \return A reference to this plugin @@ -188,8 +227,18 @@ namespace sdf return _in; } + /// \brief Initializer function to help Plugin constructors. + /// \param[out] _errors Vector of errors. + /// \param[in] _filename Filename of the shared library associated with + /// this plugin. + /// \param[in] _name The name of the plugin. + /// \param[in] _xmlContent Optional XML content that will be stored in + /// this plugin. + private: void Init(sdf::Errors &_errors, const std::string &_filename, + const std::string &_name, const std::string &_xmlContent); + /// \brief Private data pointer. - std::unique_ptr dataPtr; + public: std::unique_ptr dataPtr; }; /// \brief A vector of Plugin. diff --git a/src/Plugin.cc b/src/Plugin.cc index c3c325979..c85563519 100644 --- a/src/Plugin.cc +++ b/src/Plugin.cc @@ -47,12 +47,28 @@ Plugin::Plugin() Plugin::Plugin(const std::string &_filename, const std::string &_name, const std::string &_xmlContent) : dataPtr(std::make_unique()) +{ + sdf::Errors errors; + this->Init(errors, _filename, _name, _xmlContent); + sdf::throwOrPrintErrors(errors); +} + +///////////////////////////////////////////////// +Plugin::Plugin(sdf::Errors &_errors, const std::string &_filename, + const std::string &_name, const std::string &_xmlContent) + : dataPtr(std::make_unique()) +{ + this->Init(_errors, _filename, _name, _xmlContent); +} + +void Plugin::Init(sdf::Errors &_errors, const std::string &_filename, + const std::string &_name, const std::string &_xmlContent) { this->SetFilename(_filename); this->SetName(_name); std::string trimmed = sdf::trim(_xmlContent); if (!trimmed.empty()) - this->InsertContent(trimmed); + this->InsertContent(_errors, trimmed); } ///////////////////////////////////////////////// @@ -102,7 +118,7 @@ Errors Plugin::Load(ElementPtr _sdf) // Read the filename std::pair filenamePair = - _sdf->Get("filename", this->dataPtr->filename); + _sdf->Get(errors, "filename", this->dataPtr->filename); this->dataPtr->filename = filenamePair.first; if (!filenamePair.second) { @@ -114,7 +130,7 @@ Errors Plugin::Load(ElementPtr _sdf) for (sdf::ElementPtr innerElem = _sdf->GetFirstElement(); innerElem; innerElem = innerElem->GetNextElement("")) { - this->dataPtr->contents.push_back(innerElem->Clone()); + this->dataPtr->contents.push_back(innerElem->Clone(errors)); } return errors; @@ -150,14 +166,22 @@ sdf::ElementPtr Plugin::Element() const return this->dataPtr->sdf; } -///////////////////////////////////////////////// sdf::ElementPtr Plugin::ToElement() const +{ + sdf::Errors errors; + auto result = this->ToElement(errors); + sdf::throwOrPrintErrors(errors); + return result; +} + +///////////////////////////////////////////////// +sdf::ElementPtr Plugin::ToElement(sdf::Errors &_errors) const { sdf::ElementPtr elem(new sdf::Element); sdf::initFile("plugin.sdf", elem); - elem->GetAttribute("name")->Set(this->Name()); - elem->GetAttribute("filename")->Set(this->Filename()); + elem->GetAttribute("name")->Set(this->Name(), _errors); + elem->GetAttribute("filename")->Set(this->Filename(), _errors); // Insert plugin content for (const sdf::ElementPtr &content : this->dataPtr->contents) @@ -181,18 +205,37 @@ const std::vector &Plugin::Contents() const ///////////////////////////////////////////////// void Plugin::InsertContent(const sdf::ElementPtr _elem) { - this->dataPtr->contents.push_back(_elem->Clone()); + sdf::Errors errors; + this->InsertContent(errors, _elem); + sdf::throwOrPrintErrors(errors); +} + +///////////////////////////////////////////////// +void Plugin::InsertContent(sdf::Errors &_errors, const sdf::ElementPtr _elem) +{ + this->dataPtr->contents.push_back(_elem->Clone(_errors)); } ///////////////////////////////////////////////// bool Plugin::InsertContent(const std::string _content) +{ + sdf::Errors errors; + bool result = this->InsertContent(errors, _content); + sdf::throwOrPrintErrors(errors); + return result; +} + +///////////////////////////////////////////////// +bool Plugin::InsertContent(sdf::Errors &_errors, const std::string _content) { // Read the XML content - auto xmlDoc = tinyxml2::XMLDocument(true, tinyxml2::COLLAPSE_WHITESPACE);; + auto xmlDoc = tinyxml2::XMLDocument(true, tinyxml2::COLLAPSE_WHITESPACE); xmlDoc.Parse(_content.c_str()); if (xmlDoc.Error()) { - sdferr << "Error parsing XML from string: " << xmlDoc.ErrorStr() << '\n'; + std::stringstream ss; + ss << "Error parsing XML from string: " << xmlDoc.ErrorStr(); + _errors.push_back({ErrorCode::PARSING_ERROR, ss.str()}); return false; } @@ -209,20 +252,20 @@ bool Plugin::InsertContent(const std::string _content) for (const tinyxml2::XMLAttribute *attribute = xml->FirstAttribute(); attribute; attribute = attribute->Next()) { - element->AddAttribute(attribute->Name(), "string", "", 1, ""); + element->AddAttribute(attribute->Name(), "string", "", 1, _errors, ""); element->GetAttribute(attribute->Name())->SetFromString( - attribute->Value()); + attribute->Value(), _errors); } // Copy the value if (xml->GetText() != nullptr) - element->AddValue("string", xml->GetText(), true); + element->AddValue("string", xml->GetText(), true, _errors); // Copy all children copyChildren(element, xml, false); // Add the element to this plugin - this->InsertContent(element); + this->InsertContent(_errors, element); } return true; diff --git a/src/Plugin_TEST.cc b/src/Plugin_TEST.cc index c04cd8694..c74122472 100644 --- a/src/Plugin_TEST.cc +++ b/src/Plugin_TEST.cc @@ -19,6 +19,7 @@ #include "sdf/parser.hh" #include "sdf/Plugin.hh" #include "sdf/Element.hh" +#include "test_utils.hh" ///////////////////////////////////////////////// TEST(DOMPlugin, Construction) @@ -471,3 +472,30 @@ TEST(DOMPlugin, EqualityOperators) plugin.SetFilename("new-filename"); EXPECT_EQ(plugin, plugin2); } + +/////////////////////////////////////////////// +TEST(DOMPlugin, ErrorOutput) +{ + 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::Errors errors; + sdf::Plugin Plugin(errors, "", "", "Not valid xml"); + ASSERT_EQ(errors.size(), 1u); + EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::PARSING_ERROR); + EXPECT_NE(std::string::npos, errors[0].Message().find( + "Error parsing XML from string: ")); + + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); +} From 5e97862559c57222d5e512e6ada02b6a908409cc Mon Sep 17 00:00:00 2001 From: "Marco A. Gutierrez" Date: Tue, 28 Mar 2023 11:47:25 +0800 Subject: [PATCH 2/2] Noise: update calls to use sdf::Errors parameters (#1151) * updating Element calls to use Errors Signed-off-by: Marco A. Gutierrez * adding errors parameter to function calls Signed-off-by: Marco A. Gutierrez * Fixes for new Element API Signed-off-by: Marco A. Gutierrez * adding test for ToElement and Load Signed-off-by: Marco A. Gutierrez --------- Signed-off-by: Marco A. Gutierrez --- include/sdf/Noise.hh | 8 ++++++ src/Noise.cc | 52 ++++++++++++++++++++++++-------------- src/Noise_TEST.cc | 60 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 19 deletions(-) diff --git a/include/sdf/Noise.hh b/include/sdf/Noise.hh index 0d99c4dcf..361e12f7e 100644 --- a/include/sdf/Noise.hh +++ b/include/sdf/Noise.hh @@ -169,6 +169,14 @@ namespace sdf /// \return SDF element pointer with updated noise values. public: sdf::ElementPtr ToElement() const; + /// \brief Create and return an SDF element filled with data from this + /// noise. + /// Note that parameter passing functionality is not captured with this + /// function. + /// \param[out] _errors Vector of errors. + /// \return SDF element pointer with updated noise values. + public: sdf::ElementPtr ToElement(sdf::Errors &_errors) const; + /// \brief Private data pointer. GZ_UTILS_IMPL_PTR(dataPtr) }; diff --git a/src/Noise.cc b/src/Noise.cc index e286ed51a..b4507e957 100644 --- a/src/Noise.cc +++ b/src/Noise.cc @@ -19,6 +19,7 @@ #include "sdf/Noise.hh" #include "sdf/parser.hh" #include "sdf/Types.hh" +#include "Utils.hh" using namespace sdf; @@ -78,7 +79,8 @@ Errors Noise::Load(ElementPtr _sdf) return errors; } - std::pair type = _sdf->Get("type", "none"); + std::pair type = + _sdf->Get(errors, "type", "none"); if (!type.second) { errors.push_back({ErrorCode::ELEMENT_MISSING, @@ -102,26 +104,26 @@ Errors Noise::Load(ElementPtr _sdf) this->dataPtr->type = NoiseType::NONE; } - this->dataPtr->mean = _sdf->Get("mean", + this->dataPtr->mean = _sdf->Get(errors, "mean", this->dataPtr->mean).first; - this->dataPtr->stdDev = _sdf->Get("stddev", + this->dataPtr->stdDev = _sdf->Get(errors, "stddev", this->dataPtr->stdDev).first; - this->dataPtr->biasMean = _sdf->Get("bias_mean", + this->dataPtr->biasMean = _sdf->Get(errors, "bias_mean", this->dataPtr->biasMean).first; - this->dataPtr->biasStdDev = _sdf->Get("bias_stddev", + this->dataPtr->biasStdDev = _sdf->Get(errors, "bias_stddev", this->dataPtr->biasStdDev).first; - this->dataPtr->precision = _sdf->Get("precision", + this->dataPtr->precision = _sdf->Get(errors, "precision", this->dataPtr->precision).first; - this->dataPtr->dynamicBiasStdDev = _sdf->Get("dynamic_bias_stddev", - this->dataPtr->dynamicBiasStdDev).first; + this->dataPtr->dynamicBiasStdDev = _sdf->Get( + errors, "dynamic_bias_stddev", this->dataPtr->dynamicBiasStdDev).first; this->dataPtr->dynamicBiasCorrelationTime = _sdf->Get( - "dynamic_bias_correlation_time", + errors, "dynamic_bias_correlation_time", this->dataPtr->dynamicBiasCorrelationTime).first; return errors; @@ -252,6 +254,15 @@ bool Noise::operator==(const Noise &_noise) const ///////////////////////////////////////////////// sdf::ElementPtr Noise::ToElement() const +{ + sdf::Errors errors; + auto result = this->ToElement(errors); + sdf::throwOrPrintErrors(errors); + return result; +} + +///////////////////////////////////////////////// +sdf::ElementPtr Noise::ToElement(sdf::Errors &_errors) const { sdf::ElementPtr elem(new sdf::Element); sdf::initFile("noise.sdf", elem); @@ -271,18 +282,21 @@ sdf::ElementPtr Noise::ToElement() const default: noiseType = "none"; } - elem->GetAttribute("type")->Set(noiseType); - elem->GetElement("mean")->Set(this->Mean()); - elem->GetElement("stddev")->Set(this->StdDev()); + elem->GetAttribute("type")->Set(noiseType, _errors); + elem->GetElement("mean", _errors)->Set(_errors, this->Mean()); + elem->GetElement("stddev", _errors)->Set(_errors, this->StdDev()); // camera and lidar does not have the sdf params below - elem->GetElement("bias_mean")->Set(this->BiasMean()); - elem->GetElement("bias_stddev")->Set(this->BiasStdDev()); - elem->GetElement("dynamic_bias_stddev")->Set( - this->DynamicBiasStdDev()); - elem->GetElement("dynamic_bias_correlation_time")->Set( - this->DynamicBiasCorrelationTime()); - elem->GetElement("precision")->Set(this->Precision()); + elem->GetElement("bias_mean", _errors)->Set( + _errors, this->BiasMean()); + elem->GetElement("bias_stddev", _errors)->Set( + _errors, this->BiasStdDev()); + elem->GetElement("dynamic_bias_stddev", _errors)->Set( + _errors, this->DynamicBiasStdDev()); + elem->GetElement("dynamic_bias_correlation_time", _errors)->Set( + _errors, this->DynamicBiasCorrelationTime()); + elem->GetElement("precision", _errors)->Set( + _errors, this->Precision()); return elem; } diff --git a/src/Noise_TEST.cc b/src/Noise_TEST.cc index 06e0b88ac..614a0c331 100644 --- a/src/Noise_TEST.cc +++ b/src/Noise_TEST.cc @@ -17,6 +17,7 @@ #include #include "sdf/Noise.hh" +#include "test_utils.hh" ///////////////////////////////////////////////// TEST(DOMNoise, ConstructionAndSet) @@ -259,3 +260,62 @@ TEST(DOMNoise, ToElement) noise3.Load(noise2Elem); EXPECT_DOUBLE_EQ(0.1234, noise3.Precision()); } + +///////////////////////////////////////////////// +TEST(DOMNoise, ToElementErrorOutput) +{ + 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 + + // test calling ToElement on a DOM object constructed without calling Load + sdf::Noise noise; + sdf::Errors errors; + noise.SetType(sdf::NoiseType::GAUSSIAN); + noise.SetMean(1.2); + noise.SetStdDev(2.3); + noise.SetBiasMean(4.5); + noise.SetBiasStdDev(6.7); + noise.SetPrecision(8.9); + noise.SetDynamicBiasStdDev(9.1); + noise.SetDynamicBiasCorrelationTime(19.12); + + sdf::ElementPtr noiseElem = noise.ToElement(errors); + EXPECT_TRUE(errors.empty()); + EXPECT_NE(nullptr, noiseElem); + EXPECT_EQ(nullptr, noise.Element()); + + // verify values after loading the element back + sdf::Noise noise2; + errors = noise2.Load(noiseElem); + EXPECT_TRUE(errors.empty()); + + EXPECT_EQ(sdf::NoiseType::GAUSSIAN, noise2.Type()); + EXPECT_DOUBLE_EQ(1.2, noise2.Mean()); + EXPECT_DOUBLE_EQ(2.3, noise2.StdDev()); + EXPECT_DOUBLE_EQ(4.5, noise2.BiasMean()); + EXPECT_DOUBLE_EQ(6.7, noise2.BiasStdDev()); + EXPECT_DOUBLE_EQ(8.9, noise2.Precision()); + EXPECT_DOUBLE_EQ(9.1, noise2.DynamicBiasStdDev()); + EXPECT_DOUBLE_EQ(19.12, noise2.DynamicBiasCorrelationTime()); + + // make changes to DOM and verify ToElement produces updated values + noise2.SetPrecision(0.1234); + sdf::ElementPtr noise2Elem = noise2.ToElement(); + EXPECT_NE(nullptr, noise2Elem); + sdf::Noise noise3; + noise3.Load(noise2Elem); + EXPECT_DOUBLE_EQ(0.1234, noise3.Precision()); + + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); +}