diff --git a/Changelog.md b/Changelog.md index eaf1adf458..8a5168e857 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,13 @@ ## Gazebo 9 +## Gazebo 9.19.0 (2021-06-xx) + +1. Enable output of gzerr for SDF sibling elements of same type with same name, + following the SDF 1.6 specification. + Environment variable GAZEBO9_BACKWARDS_COMPAT_WARNINGS_ERRORS can be set to + use the previous behaviour and do not report these problems. + * [Pull request #3016](https://github.com/osrf/gazebo/pull/3016) + ## Gazebo 9.18.0 (2021-05-07) 1. Check for nullptr in TopicManager::ConnectPubToSub diff --git a/gazebo/Server.cc b/gazebo/Server.cc index 3927d5c865..9151cd2a31 100644 --- a/gazebo/Server.cc +++ b/gazebo/Server.cc @@ -30,6 +30,7 @@ #include #include +#include #include #include @@ -69,6 +70,16 @@ namespace gazebo { struct ServerPrivate { + void InspectSDFElement(const sdf::ElementPtr _elem) + { + if (common::getEnv("GAZEBO9_BACKWARDS_COMPAT_WARNINGS_ERRORS")) + return; + + if (not sdf::recursiveSameTypeUniqueNames(_elem)) + gzerr << "SDF is not valid, see errors above. " + << "This can lead to an unexpected behaviour." << "\n"; + } + /// \brief Boolean used to stop the server. static bool stop; @@ -452,6 +463,8 @@ bool Server::PreLoad() bool Server::LoadImpl(sdf::ElementPtr _elem, const std::string &_physics) { + this->dataPtr->InspectSDFElement(_elem); + // If a physics engine is specified, if (_physics.length()) { diff --git a/test/integration/CMakeLists.txt b/test/integration/CMakeLists.txt index ae2b37d415..851949af7f 100644 --- a/test/integration/CMakeLists.txt +++ b/test/integration/CMakeLists.txt @@ -95,6 +95,7 @@ set(tests rest_web.cc saving_and_loading.cc sdf.cc + sdf_errors.cc sensor.cc server_fixture.cc sim_events.cc diff --git a/test/integration/sdf_errors.cc b/test/integration/sdf_errors.cc new file mode 100644 index 0000000000..5011a2dfa8 --- /dev/null +++ b/test/integration/sdf_errors.cc @@ -0,0 +1,137 @@ +/* + * 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 "gazebo/common/CommonIface.hh" +#include "gazebo/test/ServerFixture.hh" + +using namespace gazebo; + +///////////////////////////////////////////////// +#ifdef _WIN32 +static int setenv(const char *envname, const char *envval, int overwrite) +{ + char *original = getenv(envname); + if (!original || !!overwrite) + { + std::string envstring = std::string(envname) + "=" + envval; + return _putenv(envstring.c_str()); + } + return 0; +} +#endif + +class SDFLogsTest : public ServerFixture +{ + public: void SetUp() + { +#ifndef _WIN32 + const boost::filesystem::path home = common::getEnv("HOME"); +#else + const boost::filesystem::path home = common::getEnv("HOMEPATH"); +#endif + boost::filesystem::path log_path("/.gazebo/server-11345/default.log"); + path = home / log_path; + } + + public: void EXPECT_LOG_STRING(const std::string expected_text) + { + EXPECT_TRUE(log_string_search(expected_text)) << + "The text '" + expected_text + "'" + + " was not found in the log. The test expects it to be there"; + } + + public: void EXPECT_NO_ERR_IN_LOG() + { + EXPECT_NO_LOG_STRING("[Err] "); + } + + public: void EXPECT_NO_LOG_STRING(const std::string no_expected_text) + { + EXPECT_FALSE(log_string_search(no_expected_text)) << + "The text '" + no_expected_text + "'" + + " was found in the log. The test does not expect it be there"; + } + + public: void EXPECT_SDF_ERR_IN_LOG() + { + EXPECT_LOG_STRING("[Err] "); + std::string sdfErrorString = "SDF is not valid"; + EXPECT_LOG_STRING(sdfErrorString); + } + + private: bool log_string_search(const std::string expected_text) + { + // Open the log file, and read back the string + std::ifstream ifs(path.string().c_str(), std::ios::in); + std::string loggedString; + + while (!ifs.eof()) + { + std::string line; + std::getline(ifs, line); + loggedString += line; + } + + return loggedString.find(expected_text) != std::string::npos; + } + + private: boost::filesystem::path path; +}; + +///////////////////////////////////////////////// +TEST_F(SDFLogsTest, EmptyWorldNoErrors) +{ + Load("worlds/empty.world"); + EXPECT_NO_ERR_IN_LOG(); +} + +///////////////////////////////////////////////// +TEST_F(SDFLogsTest, DuplicateSiblingSameType) +{ + Load("worlds/test_sdf16_err_sibling_same_type.world"); + EXPECT_SDF_ERR_IN_LOG(); +} + +///////////////////////////////////////////////// +TEST_F(SDFLogsTest, DuplicateSiblingSameTypeDisabled) +{ + setenv("GAZEBO9_BACKWARDS_COMPAT_WARNINGS_ERRORS", "", 1); + Load("worlds/test_sdf16_err_sibling_same_type.world"); + EXPECT_NO_ERR_IN_LOG(); +#ifdef _WIN32 + _putenv("GAZEBO9_BACKWARDS_COMPAT_WARNINGS_ERRORS"); +#else + unsetenv("GAZEBO9_BACKWARDS_COMPAT_WARNINGS_ERRORS"); +#endif +} + +///////////////////////////////////////////////// +TEST_F(SDFLogsTest, DuplicateSiblingDifferentType) +{ + Load("worlds/test_sdf16_err_sibling_different_type.world"); + // 1.6 SDF does not enforce different names for different types + EXPECT_NO_ERR_IN_LOG(); +} + +///////////////////////////////////////////////// +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/worlds/test_sdf16_err_sibling_different_type.world b/test/worlds/test_sdf16_err_sibling_different_type.world new file mode 100644 index 0000000000..da8182fb94 --- /dev/null +++ b/test/worlds/test_sdf16_err_sibling_different_type.world @@ -0,0 +1,46 @@ + + + + + + + 1 + 0 0 1 0 0 1 + + + -2.5 0 0 0 0 0 + + + 5 0.2 2 + + + + + -2.5 0 0 0 0 0 + + + 5 0.2 2 + + + + + -2.5 0 0 0 0 0 + + + 5 0.2 2 + + + + + -2.5 0 0 0 0 0 + + + 5 0.2 2 + + + + + + + + diff --git a/test/worlds/test_sdf16_err_sibling_same_type.world b/test/worlds/test_sdf16_err_sibling_same_type.world new file mode 100644 index 0000000000..600a92dce0 --- /dev/null +++ b/test/worlds/test_sdf16_err_sibling_same_type.world @@ -0,0 +1,46 @@ + + + + + + + 1 + 0 0 1 0 0 1 + + + -2.5 0 0 0 0 0 + + + 5 0.2 2 + + + + + -2.5 0 0 0 0 0 + + + 5 0.2 2 + + + + + -2.5 0 0 0 0 0 + + + 5 0.2 2 + + + + + -2.5 0 0 0 0 0 + + + 5 0.2 2 + + + + + + + +