From fcb8feb9868cad62acdfea37aa3aef7869300911 Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Tue, 14 Dec 2021 16:06:11 -0800 Subject: [PATCH 1/8] Added plugin to SDF DOM Signed-off-by: Nate Koenig --- include/sdf/CMakeLists.txt | 1 + include/sdf/Gui.hh | 33 +++++ include/sdf/Plugin.hh | 99 +++++++++++++++ src/CMakeLists.txt | 2 + src/Gui.cc | 55 ++++++++ src/Gui_TEST.cc | 21 ++++ src/Plugin.cc | 159 ++++++++++++++++++++++++ src/Plugin_TEST.cc | 248 +++++++++++++++++++++++++++++++++++++ 8 files changed, 618 insertions(+) create mode 100644 include/sdf/Plugin.hh create mode 100644 src/Plugin.cc create mode 100644 src/Plugin_TEST.cc diff --git a/include/sdf/CMakeLists.txt b/include/sdf/CMakeLists.txt index e1422b316..686bf7c8a 100644 --- a/include/sdf/CMakeLists.txt +++ b/include/sdf/CMakeLists.txt @@ -47,6 +47,7 @@ set (headers Pbr.hh Physics.hh Plane.hh + Plugin.hh PrintConfig.hh Root.hh Scene.hh diff --git a/include/sdf/Gui.hh b/include/sdf/Gui.hh index 37e8391e9..33d47b87c 100644 --- a/include/sdf/Gui.hh +++ b/include/sdf/Gui.hh @@ -17,8 +17,11 @@ #ifndef SDF_GUI_HH_ #define SDF_GUI_HH_ +#include + #include #include "sdf/Element.hh" +#include "sdf/Plugin.hh" #include "sdf/Types.hh" #include "sdf/sdf_config.h" #include "sdf/system_util.hh" @@ -66,6 +69,36 @@ namespace sdf /// \return SDF element pointer with updated gui values. public: sdf::ElementPtr ToElement() const; + /// \brief Get the number of plugins. + /// \return Number of plugins contained in this Gui object. + public: uint64_t PluginCount() const; + + /// \brief Get a plugin based on an index. + /// \param[in] _index Index of the plugin. The index should be in the + /// range [0..PluginCount()). + /// \return Pointer to the plugin. Nullptr if the index does not exist. + /// \sa uint64_t PluginCount() const + public: const Plugin *PluginByIndex(const uint64_t _index) const; + + /// \brief Get whether a plugin name exists. + /// \param[in] _name Name of the plugin to check. + /// \return True if there exists a plugin with the given name. + public: bool PluginNameExists(const std::string &_name) const; + + /// \brief Get a plugin based on a name. + /// \param[in] _name Name of the plugin. + /// \return Pointer to the plugin. Nullptr if the name does not exist. + public: const Plugin *PluginByName(const std::string &_name) const; + + /// \brief Remove all plugins + public: void ClearPlugins(); + + /// \brief Add a plugin to the link. + /// \param[in] _plugin Plugin to add. + /// \return True if successful, false if a plugin with the name already + /// exists. + public: bool AddPlugin(const Plugin &_plugin); + /// \brief Private data pointer. IGN_UTILS_IMPL_PTR(dataPtr) }; diff --git a/include/sdf/Plugin.hh b/include/sdf/Plugin.hh new file mode 100644 index 000000000..fbf0fa884 --- /dev/null +++ b/include/sdf/Plugin.hh @@ -0,0 +1,99 @@ +/* + * Copyright 2021 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_PLUGIN_HH_ +#define SDF_PLUGIN_HH_ + +#include +#include + +#include +#include +#include +#include +#include "sdf/sdf_config.h" +#include "sdf/system_util.hh" + +namespace sdf +{ + // Inline bracket to help doxygen filtering. + inline namespace SDF_VERSION_NAMESPACE { + // + + class SDFORMAT_VISIBLE Plugin + { + /// \brief Default constructor + public: Plugin(); + + /// \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. + /// \param[in] _sdf The SDF Element pointer + /// \return Errors, which is a vector of Error objects. Each Error includes + /// an error code and message. An empty vector indicates no error. + public: Errors Load(ElementPtr _sdf); + + /// \brief Get the name of the plugin. + /// The name of the plugin should be unique within the scope of its + /// parent. + /// \return Name of the plugin. + public: std::string Name() const; + + /// \brief Set the name of the plugin. + /// The name of the plugin should be unique within the scope of its + // /parent. + /// \param[in] _name Name of the plugin. + public: void SetName(const std::string &_name); + + /// \brief Get the filename of the shared library. + /// \return Filanem of the shared library associated with the plugin. + public: std::string Filename() const; + + /// \brief Remove the contents of the plugin, this is everything that + /// is a child element of the ``. + public: void ClearContents(); + + /// \brief Get the plugin contents. This is all the SDF elements that + /// are children of the ``. + /// \return The child elements of this plugin. + public: const std::vector Contents() const; + + /// \brief Insert an element into the plugin content. + /// \param[in] _elem Element to insert. + public: void InsertContent(const sdf::ElementPtr _elem); + + /// \brief Set the filename of the shared library. + /// \param[in] _filename Filename of the shared library associated with + /// this plugin. + public: void SetFilename(const std::string &_name); + + /// \brief Get a pointer to the SDF element that was used during + /// load. + /// \return SDF element pointer. The value will be nullptr if Load has + /// not been called. + public: sdf::ElementPtr Element() const; + + /// \brief Create and return an SDF element filled with data from this + /// plugin. + /// \return SDF element pointer with updated plugin values. + public: sdf::ElementPtr ToElement() const; + + /// \brief Private data pointer. + IGN_UTILS_IMPL_PTR(dataPtr) + }; +} +} +#endif diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a36fe8eda..597c6f5d8 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -60,6 +60,7 @@ set (sources Pbr.cc Physics.cc Plane.cc + Plugin.cc PrintConfig.cc Root.cc Scene.cc @@ -134,6 +135,7 @@ if (BUILD_SDF_TEST) ParticleEmitter_TEST.cc Pbr_TEST.cc Physics_TEST.cc + Plugin_TEST.cc PrintConfig_TEST.cc Plane_TEST.cc Root_TEST.cc diff --git a/src/Gui.cc b/src/Gui.cc index 06620426b..afd550a1f 100644 --- a/src/Gui.cc +++ b/src/Gui.cc @@ -28,6 +28,9 @@ class sdf::Gui::Implementation /// \brief The SDF element pointer used during load. public: sdf::ElementPtr sdf; + + /// \brief GUI plugins. + public: std::vector plugins; }; ///////////////////////////////////////////////// @@ -57,6 +60,10 @@ Errors Gui::Load(ElementPtr _sdf) this->dataPtr->fullscreen = _sdf->Get("fullscreen", this->dataPtr->fullscreen).first; + Errors pluginErrors = loadUniqueRepeated(_sdf, "plugin", + this->dataPtr->plugins); + errors.insert(errors.end(), pluginErrors.begin(), pluginErrors.end()); + // \todo(nkoenig) Parse all the elements in gui.sdf return errors; @@ -95,3 +102,51 @@ sdf::ElementPtr Gui::ToElement() const elem->GetAttribute("fullscreen")->Set(this->dataPtr->fullscreen); return elem; } + +///////////////////////////////////////////////// +uint64_t Gui::PluginCount() const +{ + return this->dataPtr->plugins.size(); +} + +///////////////////////////////////////////////// +const Plugin *Gui::PluginByIndex(const uint64_t _index) const +{ + if (_index < this->dataPtr->plugins.size()) + return &this->dataPtr->plugins[_index]; + return nullptr; +} + +///////////////////////////////////////////////// +bool Gui::PluginNameExists(const std::string &_name) const +{ + return nullptr != this->PluginByName(_name); +} + +///////////////////////////////////////////////// +const Plugin *Gui::PluginByName(const std::string &_name) const +{ + for (auto const &p : this->dataPtr->plugins) + { + if (p.Name() == _name) + { + return &p; + } + } + return nullptr; +} + +///////////////////////////////////////////////// +void Gui::ClearPlugins() +{ + this->dataPtr->plugins.clear(); +} + +///////////////////////////////////////////////// +bool Gui::AddPlugin(const Plugin &_plugin) +{ + if (this->PluginNameExists(_plugin.Name())) + return false; + this->dataPtr->plugins.push_back(_plugin); + return true; +} diff --git a/src/Gui_TEST.cc b/src/Gui_TEST.cc index aaef5ab26..6173472d6 100644 --- a/src/Gui_TEST.cc +++ b/src/Gui_TEST.cc @@ -131,6 +131,27 @@ TEST(DOMGui, ToElement) gui.SetFullscreen(true); + for (int j = 0; j <= 1; ++j) + { + for (int i = 0; i < 3; ++i) + { + sdf::Plugin plugin; + plugin.SetName("name" + std::to_string(i)); + plugin.SetFilename("filename" + std::to_string(i)); + EXPECT_TRUE(gui.AddPlugin(plugin)); + EXPECT_TRUE(gui.PluginNameExists(plugin.Name())); + EXPECT_FALSE(gui.PluginNameExists(plugin.Name()+"a")); + EXPECT_FALSE(gui.AddPlugin(plugin)); + } + if (j == 0) + { + EXPECT_EQ(3u, gui.PluginCount()); + gui.ClearPlugins(); + EXPECT_EQ(0u, gui.PluginCount()); + } + } + + sdf::ElementPtr elem = gui.ToElement(); ASSERT_NE(nullptr, elem); diff --git a/src/Plugin.cc b/src/Plugin.cc new file mode 100644 index 000000000..18fc9c35b --- /dev/null +++ b/src/Plugin.cc @@ -0,0 +1,159 @@ +/* + * Copyright 2021 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/Plugin.hh" +#include "sdf/parser.hh" +#include "Utils.hh" + +using namespace sdf; + +class sdf::Plugin::Implementation +{ + /// \brief Name of the plugin + public: std::string name = ""; + + /// \brief Filename of the shared library + public: std::string filename = ""; + + /// \brief The SDF element pointer used during load. + public: sdf::ElementPtr sdf; + + /// \brief SDF elements inside the plugin. + public: std::vector contents; +}; + +///////////////////////////////////////////////// +Plugin::Plugin() + : dataPtr(ignition::utils::MakeImpl()) +{ +} + +///////////////////////////////////////////////// +Errors Plugin::Load(ElementPtr _sdf) +{ + Errors errors; + + this->dataPtr->sdf = _sdf; + + // Check that sdf is a valid pointer + if (!_sdf) + { + errors.push_back({ErrorCode::ELEMENT_MISSING, + "Attempting to load a plugin, but the provided SDF " + "element is null."}); + return errors; + } + + // We need a plugin element + if (_sdf->GetName() != "plugin") + { + errors.push_back({ErrorCode::ELEMENT_INCORRECT_TYPE, + "Attempting to load a plugin, but the provided SDF " + "element is not a ."}); + return errors; + } + + // Read the models's name + if (!loadName(_sdf, this->dataPtr->name)) + { + errors.push_back({ErrorCode::ATTRIBUTE_MISSING, + "A plugin name is required, but the name is not set."}); + } + + // Read the filename + std::pair filenamePair = + _sdf->Get("filename", this->dataPtr->filename); + this->dataPtr->filename = filenamePair.first; + if (!filenamePair.second) + { + errors.push_back({ErrorCode::ATTRIBUTE_MISSING, + "A plugin filename is required, but the filename is not set."}); + } + + // Copy the contents of the plugin + for (sdf::ElementPtr innerElem = _sdf->GetFirstElement(); + innerElem; innerElem = innerElem->GetNextElement("")) + { + this->dataPtr->contents.push_back(innerElem->Clone()); + } + + return errors; +} + +///////////////////////////////////////////////// +std::string Plugin::Name() const +{ + return this->dataPtr->name; +} + +///////////////////////////////////////////////// +void Plugin::SetName(const std::string &_name) +{ + this->dataPtr->name = _name; +} + +///////////////////////////////////////////////// +std::string Plugin::Filename() const +{ + return this->dataPtr->filename; +} + +///////////////////////////////////////////////// +void Plugin::SetFilename(const std::string &_filename) +{ + this->dataPtr->filename = _filename; +} + +///////////////////////////////////////////////// +sdf::ElementPtr Plugin::Element() const +{ + return this->dataPtr->sdf; +} + +///////////////////////////////////////////////// +sdf::ElementPtr Plugin::ToElement() const +{ + sdf::ElementPtr elem(new sdf::Element); + sdf::initFile("plugin.sdf", elem); + + elem->GetAttribute("name")->Set(this->Name()); + elem->GetAttribute("filename")->Set(this->Filename()); + + // Insert plugin content + for (const sdf::ElementPtr content : this->dataPtr->contents) + elem->InsertElement(content, true); + + return elem; +} + +///////////////////////////////////////////////// +void Plugin::ClearContents() +{ + this->dataPtr->contents.clear(); +} + +///////////////////////////////////////////////// +const std::vector Plugin::Contents() const +{ + return this->dataPtr->contents; +} + +///////////////////////////////////////////////// +void Plugin::InsertContent(const sdf::ElementPtr _elem) +{ + this->dataPtr->contents.push_back(_elem->Clone()); +} diff --git a/src/Plugin_TEST.cc b/src/Plugin_TEST.cc new file mode 100644 index 000000000..1b8cb9538 --- /dev/null +++ b/src/Plugin_TEST.cc @@ -0,0 +1,248 @@ +/* + * Copyright (C) 2021 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 "sdf/parser.hh" +#include "sdf/Plugin.hh" +#include "sdf/Element.hh" + +///////////////////////////////////////////////// +TEST(DOMPlugin, Construction) +{ + sdf::Plugin plugin; + EXPECT_EQ(nullptr, plugin.Element()); + + EXPECT_TRUE(plugin.Name().empty()); + EXPECT_TRUE(plugin.Filename().empty()); + EXPECT_TRUE(plugin.Contents().empty()); + + plugin.SetName("my-plugin"); + EXPECT_EQ("my-plugin", plugin.Name()); + + plugin.SetFilename("filename.so"); + EXPECT_EQ("filename.so", plugin.Filename()); + + sdf::ElementPtr content(new sdf::Element); + content->SetName("an-element"); + plugin.InsertContent(content); + EXPECT_EQ(1u, plugin.Contents().size()); + + sdf::ElementPtr elem = plugin.ToElement(); + ASSERT_NE(nullptr, elem); + + ASSERT_NE(nullptr, elem->GetAttribute("name")); + EXPECT_EQ("my-plugin", elem->GetAttribute("name")->GetAsString()); + + ASSERT_NE(nullptr, elem->GetAttribute("filename")); + EXPECT_EQ("filename.so", elem->GetAttribute("filename")->GetAsString()); +} + +///////////////////////////////////////////////// +TEST(DOMPlugin, MoveConstructor) +{ + sdf::Plugin plugin; + plugin.SetName("pluginname"); + plugin.SetFilename("filename"); + + sdf::ElementPtr content(new sdf::Element); + content->SetName("an-element"); + plugin.InsertContent(content); + + sdf::Plugin plugin2(std::move(plugin)); + EXPECT_EQ("pluginname", plugin2.Name()); + EXPECT_EQ("filename", plugin2.Filename()); + ASSERT_EQ(1u, plugin2.Contents().size()); + EXPECT_EQ("an-element", plugin2.Contents()[0]->GetName()); +} + +///////////////////////////////////////////////// +TEST(DOMPlugin, CopyConstructor) +{ + sdf::Plugin plugin; + plugin.SetName("pluginname"); + plugin.SetFilename("filename"); + + sdf::Plugin plugin2(plugin); + EXPECT_EQ("pluginname", plugin2.Name()); + EXPECT_EQ("filename", plugin2.Filename()); + + EXPECT_EQ("pluginname", plugin.Name()); + EXPECT_EQ("filename", plugin.Filename()); +} + +///////////////////////////////////////////////// +TEST(DOMPlugin, CopyAssigmentOperator) +{ + sdf::Plugin plugin; + plugin.SetName("pluginname"); + plugin.SetFilename("filename"); + + sdf::Plugin plugin2; + plugin2 = plugin; + EXPECT_EQ("pluginname", plugin2.Name()); + EXPECT_EQ("filename", plugin2.Filename()); + + EXPECT_EQ("pluginname", plugin.Name()); + EXPECT_EQ("filename", plugin.Filename()); +} + +///////////////////////////////////////////////// +TEST(DOMPlugin, MoveAssignmentConstructor) +{ + sdf::Plugin plugin; + plugin.SetName("pluginname"); + plugin.SetFilename("filename"); + + sdf::Plugin plugin2; + plugin2 = std::move(plugin); + EXPECT_EQ("pluginname", plugin2.Name()); + EXPECT_EQ("filename", plugin2.Filename()); +} + +///////////////////////////////////////////////// +TEST(DOMPlugin, CopyAssignmentAfterMove) +{ + sdf::Plugin plugin; + plugin.SetName("pluginname"); + plugin.SetFilename("filename"); + + sdf::Plugin plugin2; + plugin2.SetName("pluginname2"); + plugin2.SetFilename("filename2"); + + // This is similar to what std::swap does except it uses std::move for each + // assignment + sdf::Plugin tmp = std::move(plugin); + plugin = plugin2; + plugin2 = tmp; + + EXPECT_EQ("pluginname", plugin2.Name()); + EXPECT_EQ("filename", plugin2.Filename()); + + EXPECT_EQ("pluginname2", plugin.Name()); + EXPECT_EQ("filename2", plugin.Filename()); +} + +///////////////////////////////////////////////// +TEST(DOMPlugin, Load) +{ + sdf::Plugin plugin; + sdf::Errors errors; + + // Null sdf + errors = plugin.Load(nullptr); + ASSERT_EQ(1u, errors.size()); + EXPECT_EQ(sdf::ErrorCode::ELEMENT_MISSING, errors[0].Code()); + + // Bad element name + sdf::ElementPtr sdf(new sdf::Element()); + sdf->SetName("bad"); + errors = plugin.Load(sdf); + ASSERT_EQ(1u, errors.size()); + EXPECT_EQ(sdf::ErrorCode::ELEMENT_INCORRECT_TYPE, errors[0].Code()); + EXPECT_NE(nullptr, plugin.Element()); + + sdf->SetName("plugin"); + + // Missing name and filename attribute + errors = plugin.Load(sdf); + ASSERT_EQ(2u, errors.size()); + EXPECT_EQ(sdf::ErrorCode::ATTRIBUTE_MISSING, errors[0].Code()); + EXPECT_EQ(sdf::ErrorCode::ATTRIBUTE_MISSING, errors[1].Code()); + + sdf->AddAttribute("name", "string", "__default__", true); + sdf->GetAttribute("name")->Set("my-plugin-name"); + + // Now just missing filename + errors = plugin.Load(sdf); + ASSERT_EQ(1u, errors.size()); + EXPECT_EQ(sdf::ErrorCode::ATTRIBUTE_MISSING, errors[0].Code()); + + sdf->AddAttribute("filename", "string", "__default__", true); + sdf->GetAttribute("filename")->Set("filename.so"); + + // No errors. + errors = plugin.Load(sdf); + ASSERT_TRUE(errors.empty()); +} + +///////////////////////////////////////////////// +TEST(DOMPlugin, LoadWithChildren) +{ + std::string pluginStr = R"( + + + + 3D View + false + docked + + ogre + scene + 0.4 0.4 0.4 + 0.8 0.8 0.8 + -6 0 6 0 0.5 0 + + )"; + + sdf::ElementPtr elem(new sdf::Element); + sdf::initFile("plugin.sdf", elem); + sdf::readString(pluginStr, elem); + + sdf::Plugin plugin; + sdf::Errors errors; + errors = plugin.Load(elem); + ASSERT_EQ(0u, errors.size()); + + EXPECT_EQ("3D View", plugin.Name()); + EXPECT_EQ("MinimalScene", plugin.Filename()); + + // The elements should be the same + EXPECT_EQ(elem->ToString(""), plugin.Element()->ToString("")); + + sdf::ElementPtr toElem = plugin.ToElement(); + + // The elements should be the same + EXPECT_EQ(elem->ToString(""), toElem->ToString("")); +} + +///////////////////////////////////////////////// +TEST(DOMPlugin, ToElement) +{ + sdf::Plugin plugin; + plugin.SetName("my-plugin"); + EXPECT_EQ("my-plugin", plugin.Name()); + + plugin.SetFilename("filename.so"); + EXPECT_EQ("filename.so", plugin.Filename()); + + sdf::ElementPtr content(new sdf::Element); + content->SetName("an-element"); + plugin.InsertContent(content); + EXPECT_EQ(1u, plugin.Contents().size()); + + sdf::ElementPtr elem = plugin.ToElement(); + ASSERT_NE(nullptr, elem); + + sdf::Plugin plugin2; + plugin2.Load(elem); + + EXPECT_EQ(plugin.Name(), plugin2.Name()); + EXPECT_EQ(plugin.Filename(), plugin2.Filename()); + EXPECT_EQ(1u, plugin2.Contents().size()); + EXPECT_EQ("an-element", plugin2.Contents()[0]->GetName()); +} From 00261ce7bcda56a5d1233d00d6473d6963d68fc3 Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Tue, 14 Dec 2021 16:23:03 -0800 Subject: [PATCH 2/8] tweaks Signed-off-by: Nate Koenig --- include/sdf/Plugin.hh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/sdf/Plugin.hh b/include/sdf/Plugin.hh index fbf0fa884..4ec39ac17 100644 --- a/include/sdf/Plugin.hh +++ b/include/sdf/Plugin.hh @@ -54,12 +54,12 @@ namespace sdf /// \brief Set the name of the plugin. /// The name of the plugin should be unique within the scope of its - // /parent. + /// parent. /// \param[in] _name Name of the plugin. public: void SetName(const std::string &_name); /// \brief Get the filename of the shared library. - /// \return Filanem of the shared library associated with the plugin. + /// \return Filename of the shared library associated with the plugin. public: std::string Filename() const; /// \brief Remove the contents of the plugin, this is everything that @@ -72,7 +72,7 @@ namespace sdf public: const std::vector Contents() const; /// \brief Insert an element into the plugin content. - /// \param[in] _elem Element to insert. + /// \param[in] _elem Element to insert. public: void InsertContent(const sdf::ElementPtr _elem); /// \brief Set the filename of the shared library. From 74bd0d1ef1e983d38f73205c4e0fd9586380e6e1 Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Wed, 15 Dec 2021 08:49:10 -0800 Subject: [PATCH 3/8] Fixed doxygen Signed-off-by: Nate Koenig --- include/sdf/Plugin.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/sdf/Plugin.hh b/include/sdf/Plugin.hh index 4ec39ac17..48dc5f2fa 100644 --- a/include/sdf/Plugin.hh +++ b/include/sdf/Plugin.hh @@ -78,7 +78,7 @@ namespace sdf /// \brief Set the filename of the shared library. /// \param[in] _filename Filename of the shared library associated with /// this plugin. - public: void SetFilename(const std::string &_name); + public: void SetFilename(const std::string &_filename); /// \brief Get a pointer to the SDF element that was used during /// load. From 1f3b67563c4671888e69c25c5f3d313505a8404a Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Wed, 15 Dec 2021 12:48:50 -0800 Subject: [PATCH 4/8] Updates Signed-off-by: Nate Koenig --- include/sdf/Plugin.hh | 6 ++++-- src/Plugin.cc | 2 +- src/Plugin_TEST.cc | 35 ++++++++++++++++++----------------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/include/sdf/Plugin.hh b/include/sdf/Plugin.hh index 48dc5f2fa..170c2d844 100644 --- a/include/sdf/Plugin.hh +++ b/include/sdf/Plugin.hh @@ -69,9 +69,11 @@ namespace sdf /// \brief Get the plugin contents. This is all the SDF elements that /// are children of the ``. /// \return The child elements of this plugin. - public: const std::vector Contents() const; + public: const std::vector &Contents() const; - /// \brief Insert an element into the plugin content. + /// \brief Insert an element into the plugin content. This does not + /// modify the values in the sdf::ElementPtr returned by the `Element()` + /// function. /// \param[in] _elem Element to insert. public: void InsertContent(const sdf::ElementPtr _elem); diff --git a/src/Plugin.cc b/src/Plugin.cc index 18fc9c35b..0e034c935 100644 --- a/src/Plugin.cc +++ b/src/Plugin.cc @@ -147,7 +147,7 @@ void Plugin::ClearContents() } ///////////////////////////////////////////////// -const std::vector Plugin::Contents() const +const std::vector &Plugin::Contents() const { return this->dataPtr->contents; } diff --git a/src/Plugin_TEST.cc b/src/Plugin_TEST.cc index 1b8cb9538..2111b18e3 100644 --- a/src/Plugin_TEST.cc +++ b/src/Plugin_TEST.cc @@ -183,25 +183,25 @@ TEST(DOMPlugin, Load) ///////////////////////////////////////////////// TEST(DOMPlugin, LoadWithChildren) { - std::string pluginStr = R"( - - - - 3D View - false - docked - - ogre - scene - 0.4 0.4 0.4 - 0.8 0.8 0.8 - -6 0 6 0 0.5 0 - - )"; - + std::string pluginStr = R"( + + 3D View + false + docked + + ogre + scene + 0.4 0.4 0.4 + 0.8 0.8 0.8 + -6 0 6 0 0.5 0 + +)"; + + std::string pluginStrWithSdf = std::string("") + + pluginStr + ""; sdf::ElementPtr elem(new sdf::Element); sdf::initFile("plugin.sdf", elem); - sdf::readString(pluginStr, elem); + sdf::readString(pluginStrWithSdf, elem); sdf::Plugin plugin; sdf::Errors errors; @@ -218,6 +218,7 @@ TEST(DOMPlugin, LoadWithChildren) // The elements should be the same EXPECT_EQ(elem->ToString(""), toElem->ToString("")); + EXPECT_EQ(pluginStr, toElem->ToString("")); } ///////////////////////////////////////////////// From 980d5d229dba42cbdfb6ed1e9ed71006693fad87 Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Fri, 17 Dec 2021 09:31:26 -0800 Subject: [PATCH 5/8] Update plugin copy and tests Signed-off-by: Nate Koenig --- include/sdf/Gui.hh | 14 +------------- include/sdf/Plugin.hh | 10 ++++++++++ sdf/1.9/plugin.sdf | 2 +- src/Gui.cc | 31 +++++++------------------------ src/Gui_TEST.cc | 10 +++++----- src/Plugin.cc | 26 ++++++++++++++++++++++++++ src/Plugin_TEST.cc | 14 ++++++++++++++ 7 files changed, 64 insertions(+), 43 deletions(-) diff --git a/include/sdf/Gui.hh b/include/sdf/Gui.hh index 33d47b87c..13c0c3093 100644 --- a/include/sdf/Gui.hh +++ b/include/sdf/Gui.hh @@ -80,24 +80,12 @@ namespace sdf /// \sa uint64_t PluginCount() const public: const Plugin *PluginByIndex(const uint64_t _index) const; - /// \brief Get whether a plugin name exists. - /// \param[in] _name Name of the plugin to check. - /// \return True if there exists a plugin with the given name. - public: bool PluginNameExists(const std::string &_name) const; - - /// \brief Get a plugin based on a name. - /// \param[in] _name Name of the plugin. - /// \return Pointer to the plugin. Nullptr if the name does not exist. - public: const Plugin *PluginByName(const std::string &_name) const; - /// \brief Remove all plugins public: void ClearPlugins(); /// \brief Add a plugin to the link. /// \param[in] _plugin Plugin to add. - /// \return True if successful, false if a plugin with the name already - /// exists. - public: bool AddPlugin(const Plugin &_plugin); + public: void AddPlugin(const Plugin &_plugin); /// \brief Private data pointer. IGN_UTILS_IMPL_PTR(dataPtr) diff --git a/include/sdf/Plugin.hh b/include/sdf/Plugin.hh index 170c2d844..ba549afa8 100644 --- a/include/sdf/Plugin.hh +++ b/include/sdf/Plugin.hh @@ -38,6 +38,11 @@ namespace sdf /// \brief Default constructor public: Plugin(); + /// \brief Copy constructor. This is here so that we can copy the + /// plugin contents. + /// \param[in] _plugin Plugin to copy. + public: Plugin(const Plugin &_plugin); + /// \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. @@ -93,6 +98,11 @@ namespace sdf /// \return SDF element pointer with updated plugin values. public: sdf::ElementPtr ToElement() const; + /// \brief Copy assignment operator + /// \param[in] _plugin Plugin to copy + /// \return A reference to this plugin + public: Plugin &operator=(const Plugin &_plugin); + /// \brief Private data pointer. IGN_UTILS_IMPL_PTR(dataPtr) }; diff --git a/sdf/1.9/plugin.sdf b/sdf/1.9/plugin.sdf index 236ea31af..81452b64c 100644 --- a/sdf/1.9/plugin.sdf +++ b/sdf/1.9/plugin.sdf @@ -2,7 +2,7 @@ A plugin is a dynamically loaded chunk of code. It can exist as a child of world, model, and sensor. - A unique name for the plugin, scoped to its parent. + A name for the plugin. Name of the shared library to load. If the filename is not a full path name, the file will be searched for in the configuration paths. diff --git a/src/Gui.cc b/src/Gui.cc index afd550a1f..dc0a9e3e0 100644 --- a/src/Gui.cc +++ b/src/Gui.cc @@ -60,7 +60,7 @@ Errors Gui::Load(ElementPtr _sdf) this->dataPtr->fullscreen = _sdf->Get("fullscreen", this->dataPtr->fullscreen).first; - Errors pluginErrors = loadUniqueRepeated(_sdf, "plugin", + Errors pluginErrors = loadRepeated(_sdf, "plugin", this->dataPtr->plugins); errors.insert(errors.end(), pluginErrors.begin(), pluginErrors.end()); @@ -100,6 +100,11 @@ sdf::ElementPtr Gui::ToElement() const sdf::initFile("gui.sdf", elem); elem->GetAttribute("fullscreen")->Set(this->dataPtr->fullscreen); + + // Add in the plugins + for (const Plugin &plugin : this->dataPtr->plugins) + elem->InsertElement(plugin.ToElement(), true); + return elem; } @@ -117,25 +122,6 @@ const Plugin *Gui::PluginByIndex(const uint64_t _index) const return nullptr; } -///////////////////////////////////////////////// -bool Gui::PluginNameExists(const std::string &_name) const -{ - return nullptr != this->PluginByName(_name); -} - -///////////////////////////////////////////////// -const Plugin *Gui::PluginByName(const std::string &_name) const -{ - for (auto const &p : this->dataPtr->plugins) - { - if (p.Name() == _name) - { - return &p; - } - } - return nullptr; -} - ///////////////////////////////////////////////// void Gui::ClearPlugins() { @@ -143,10 +129,7 @@ void Gui::ClearPlugins() } ///////////////////////////////////////////////// -bool Gui::AddPlugin(const Plugin &_plugin) +void Gui::AddPlugin(const Plugin &_plugin) { - if (this->PluginNameExists(_plugin.Name())) - return false; this->dataPtr->plugins.push_back(_plugin); - return true; } diff --git a/src/Gui_TEST.cc b/src/Gui_TEST.cc index 6173472d6..f068d5034 100644 --- a/src/Gui_TEST.cc +++ b/src/Gui_TEST.cc @@ -138,20 +138,19 @@ TEST(DOMGui, ToElement) sdf::Plugin plugin; plugin.SetName("name" + std::to_string(i)); plugin.SetFilename("filename" + std::to_string(i)); - EXPECT_TRUE(gui.AddPlugin(plugin)); - EXPECT_TRUE(gui.PluginNameExists(plugin.Name())); - EXPECT_FALSE(gui.PluginNameExists(plugin.Name()+"a")); - EXPECT_FALSE(gui.AddPlugin(plugin)); + gui.AddPlugin(plugin); + gui.AddPlugin(plugin); } if (j == 0) { - EXPECT_EQ(3u, gui.PluginCount()); + EXPECT_EQ(6u, gui.PluginCount()); gui.ClearPlugins(); EXPECT_EQ(0u, gui.PluginCount()); } } + EXPECT_EQ(6u, gui.PluginCount()); sdf::ElementPtr elem = gui.ToElement(); ASSERT_NE(nullptr, elem); @@ -159,4 +158,5 @@ TEST(DOMGui, ToElement) gui2.Load(elem); EXPECT_EQ(gui.Fullscreen(), gui2.Fullscreen()); + EXPECT_EQ(6u, gui2.PluginCount()); } diff --git a/src/Plugin.cc b/src/Plugin.cc index 0e034c935..f526d7c4c 100644 --- a/src/Plugin.cc +++ b/src/Plugin.cc @@ -42,6 +42,14 @@ Plugin::Plugin() { } +///////////////////////////////////////////////// +Plugin::Plugin(const Plugin &_plugin) + : dataPtr(ignition::utils::MakeImpl()) +{ + // Copy + *this = _plugin; +} + ///////////////////////////////////////////////// Errors Plugin::Load(ElementPtr _sdf) { @@ -157,3 +165,21 @@ void Plugin::InsertContent(const sdf::ElementPtr _elem) { this->dataPtr->contents.push_back(_elem->Clone()); } + +///////////////////////////////////////////////// +Plugin &Plugin::operator=(const Plugin &_plugin) +{ + this->dataPtr->name = _plugin.Name(); + this->dataPtr->filename = _plugin.Filename(); + if (_plugin.Element()) + this->dataPtr->sdf = _plugin.Element()->Clone(); + + this->dataPtr->contents.clear(); + // Copy the contents of the plugin + for (const sdf::ElementPtr content : _plugin.Contents()) + { + this->dataPtr->contents.push_back(content->Clone()); + } + + return *this; +} diff --git a/src/Plugin_TEST.cc b/src/Plugin_TEST.cc index 2111b18e3..00d5b6245 100644 --- a/src/Plugin_TEST.cc +++ b/src/Plugin_TEST.cc @@ -219,6 +219,20 @@ TEST(DOMPlugin, LoadWithChildren) // The elements should be the same EXPECT_EQ(elem->ToString(""), toElem->ToString("")); EXPECT_EQ(pluginStr, toElem->ToString("")); + + // Test plugin copy + sdf::Plugin plugin3; + plugin3 = plugin; + plugin.ClearContents(); + sdf::Plugin plugin4(plugin3); + + toElem = plugin3.ToElement(); + EXPECT_EQ(6u, plugin3.Contents().size()); + EXPECT_EQ(pluginStr, toElem->ToString("")); + + toElem = plugin4.ToElement(); + EXPECT_EQ(6u, plugin4.Contents().size()); + EXPECT_EQ(pluginStr, toElem->ToString("")); } ///////////////////////////////////////////////// From cd4e1b942e2a4bccf2b5fc282e376e85c429343b Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Fri, 17 Dec 2021 11:04:20 -0800 Subject: [PATCH 6/8] Remove string and add move functions Signed-off-by: Nate Koenig --- include/sdf/Gui.hh | 2 -- include/sdf/Plugin.hh | 12 ++++++++++-- src/Plugin.cc | 6 ++++++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/include/sdf/Gui.hh b/include/sdf/Gui.hh index 13c0c3093..35f9f0b6a 100644 --- a/include/sdf/Gui.hh +++ b/include/sdf/Gui.hh @@ -17,8 +17,6 @@ #ifndef SDF_GUI_HH_ #define SDF_GUI_HH_ -#include - #include #include "sdf/Element.hh" #include "sdf/Plugin.hh" diff --git a/include/sdf/Plugin.hh b/include/sdf/Plugin.hh index ba549afa8..1d357ff9b 100644 --- a/include/sdf/Plugin.hh +++ b/include/sdf/Plugin.hh @@ -38,11 +38,14 @@ namespace sdf /// \brief Default constructor public: Plugin(); - /// \brief Copy constructor. This is here so that we can copy the - /// plugin contents. + /// \brief Copy constructor. /// \param[in] _plugin Plugin to copy. public: Plugin(const Plugin &_plugin); + /// \brief Move constructor. + /// \param[in] _plugin Plugin to copy. + public: Plugin(Plugin &&_plugin) noexcept; + /// \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. @@ -103,6 +106,11 @@ namespace sdf /// \return A reference to this plugin public: Plugin &operator=(const Plugin &_plugin); + /// \brief Move assignment operator + /// \param[in] _plugin Plugin to move + /// \return A reference to this plugin + public: Plugin &operator=(Plugin &&_plugin) noexcept; + /// \brief Private data pointer. IGN_UTILS_IMPL_PTR(dataPtr) }; diff --git a/src/Plugin.cc b/src/Plugin.cc index f526d7c4c..2b87e5503 100644 --- a/src/Plugin.cc +++ b/src/Plugin.cc @@ -50,6 +50,9 @@ Plugin::Plugin(const Plugin &_plugin) *this = _plugin; } +///////////////////////////////////////////////// +Plugin::Plugin(Plugin &&_plugin) noexcept = default; + ///////////////////////////////////////////////// Errors Plugin::Load(ElementPtr _sdf) { @@ -183,3 +186,6 @@ Plugin &Plugin::operator=(const Plugin &_plugin) return *this; } + +///////////////////////////////////////////////// +Plugin &Plugin::operator=(Plugin &&_plugin) noexcept = default; From 9996e03b9667b1d9ff3a5cf8fa79937f4c9073da Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Fri, 17 Dec 2021 11:53:22 -0800 Subject: [PATCH 7/8] Fix build and tests Signed-off-by: Nate Koenig --- include/sdf/Plugin.hh | 8 ++++++-- src/Plugin.cc | 23 ++++++++++++++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/include/sdf/Plugin.hh b/include/sdf/Plugin.hh index 1d357ff9b..7aa2b82ab 100644 --- a/include/sdf/Plugin.hh +++ b/include/sdf/Plugin.hh @@ -17,10 +17,10 @@ #ifndef SDF_PLUGIN_HH_ #define SDF_PLUGIN_HH_ +#include #include #include -#include #include #include #include @@ -32,12 +32,16 @@ namespace sdf // Inline bracket to help doxygen filtering. inline namespace SDF_VERSION_NAMESPACE { // + class PluginPrivate; class SDFORMAT_VISIBLE Plugin { /// \brief Default constructor public: Plugin(); + /// \brief Default destructor + public: ~Plugin(); + /// \brief Copy constructor. /// \param[in] _plugin Plugin to copy. public: Plugin(const Plugin &_plugin); @@ -112,7 +116,7 @@ namespace sdf public: Plugin &operator=(Plugin &&_plugin) noexcept; /// \brief Private data pointer. - IGN_UTILS_IMPL_PTR(dataPtr) + std::unique_ptr dataPtr; }; } } diff --git a/src/Plugin.cc b/src/Plugin.cc index 2b87e5503..d59e6bb64 100644 --- a/src/Plugin.cc +++ b/src/Plugin.cc @@ -21,7 +21,7 @@ using namespace sdf; -class sdf::Plugin::Implementation +class sdf::PluginPrivate { /// \brief Name of the plugin public: std::string name = ""; @@ -38,20 +38,26 @@ class sdf::Plugin::Implementation ///////////////////////////////////////////////// Plugin::Plugin() - : dataPtr(ignition::utils::MakeImpl()) + : dataPtr(std::make_unique()) { } +///////////////////////////////////////////////// +Plugin::~Plugin() = default; + ///////////////////////////////////////////////// Plugin::Plugin(const Plugin &_plugin) - : dataPtr(ignition::utils::MakeImpl()) + : dataPtr(std::make_unique()) { // Copy *this = _plugin; } ///////////////////////////////////////////////// -Plugin::Plugin(Plugin &&_plugin) noexcept = default; +Plugin::Plugin(Plugin &&_plugin) noexcept +{ + this->dataPtr = std::move(_plugin.dataPtr); +} ///////////////////////////////////////////////// Errors Plugin::Load(ElementPtr _sdf) @@ -172,6 +178,9 @@ void Plugin::InsertContent(const sdf::ElementPtr _elem) ///////////////////////////////////////////////// Plugin &Plugin::operator=(const Plugin &_plugin) { + if (!this->dataPtr) + this->dataPtr = std::make_unique(); + this->dataPtr->name = _plugin.Name(); this->dataPtr->filename = _plugin.Filename(); if (_plugin.Element()) @@ -188,4 +197,8 @@ Plugin &Plugin::operator=(const Plugin &_plugin) } ///////////////////////////////////////////////// -Plugin &Plugin::operator=(Plugin &&_plugin) noexcept = default; +Plugin &Plugin::operator=(Plugin &&_plugin) noexcept +{ + this->dataPtr = std::move(_plugin.dataPtr); + return *this; +} From 60dc7cd6b203060a25f1edbebee026f8c763bc3e Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Fri, 17 Dec 2021 13:32:09 -0800 Subject: [PATCH 8/8] Fix windows warnings Signed-off-by: Nate Koenig --- include/sdf/Plugin.hh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/sdf/Plugin.hh b/include/sdf/Plugin.hh index 7aa2b82ab..2026b8ae9 100644 --- a/include/sdf/Plugin.hh +++ b/include/sdf/Plugin.hh @@ -27,6 +27,13 @@ #include "sdf/sdf_config.h" #include "sdf/system_util.hh" +#ifdef _WIN32 +// Disable warning C4251 which is triggered by +// std::unique_ptr +#pragma warning(push) +#pragma warning(disable: 4251) +#endif + namespace sdf { // Inline bracket to help doxygen filtering. @@ -120,4 +127,9 @@ namespace sdf }; } } + +#ifdef _WIN32 +#pragma warning(pop) +#endif + #endif