From e15b6733fa769ce2f37ed255a220dd52546751e9 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Wed, 23 Dec 2020 09:27:31 -0800 Subject: [PATCH] Suggestions to #281 (#508) Signed-off-by: Louise Poubel --- include/ignition/gazebo/config.hh.in | 2 +- src/ServerConfig_TEST.cc | 93 ++++++++++++++++++++-------- src/ServerPrivate.cc | 54 ++++++++++++++++ src/Server_TEST.cc | 10 +-- src/SimulationRunner_TEST.cc | 34 +++++----- src/cmd/cmdgazebo.rb.in | 3 +- src/systems/log/LogRecord.cc | 12 +++- tutorials/server_config.md | 2 +- 8 files changed, 154 insertions(+), 56 deletions(-) diff --git a/include/ignition/gazebo/config.hh.in b/include/ignition/gazebo/config.hh.in index eacad933bf..050a0304a4 100644 --- a/include/ignition/gazebo/config.hh.in +++ b/include/ignition/gazebo/config.hh.in @@ -15,7 +15,7 @@ #define IGNITION_GAZEBO_GUI_CONFIG_PATH "${CMAKE_INSTALL_PREFIX}/${IGN_DATA_INSTALL_DIR}/gui" #define IGNITION_GAZEBO_SYSTEM_CONFIG_PATH "${CMAKE_INSTALL_PREFIX}/${IGN_DATA_INSTALL_DIR}/systems" -#define IGNITION_GAZEBO_SERVER_CONFIG_PATH "${CMAKE_INSTALL_PREFIX}/${IGN_DATA_INSTALL_DIR}/" +#define IGNITION_GAZEBO_SERVER_CONFIG_PATH "${CMAKE_INSTALL_PREFIX}/${IGN_DATA_INSTALL_DIR}" #define IGN_GAZEBO_PLUGIN_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/${IGN_LIB_INSTALL_DIR}/ign-${IGN_DESIGNATION}-${PROJECT_VERSION_MAJOR}/plugins" #define IGN_GAZEBO_GUI_PLUGIN_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/${IGN_LIB_INSTALL_DIR}/ign-${IGN_DESIGNATION}-${PROJECT_VERSION_MAJOR}/plugins/gui" #define IGN_GAZEBO_WORLD_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/${IGN_DATA_INSTALL_DIR}/worlds" diff --git a/src/ServerConfig_TEST.cc b/src/ServerConfig_TEST.cc index 78fa99b8eb..733a55ef37 100644 --- a/src/ServerConfig_TEST.cc +++ b/src/ServerConfig_TEST.cc @@ -26,7 +26,7 @@ using namespace ignition; using namespace gazebo; ////////////////////////////////////////////////// -TEST(parsePluginsFromString, valid) +TEST(ParsePluginsFromString, Valid) { std::string config = R"( @@ -58,14 +58,30 @@ TEST(parsePluginsFromString, valid) auto plugins = parsePluginsFromString(config); ASSERT_EQ(3u, plugins.size()); - EXPECT_EQ("default", plugins.begin()->EntityName()); - EXPECT_EQ("world", plugins.begin()->EntityType()); - EXPECT_EQ("TestWorldSystem", plugins.begin()->Filename()); - EXPECT_EQ("ignition::gazebo::TestWorldSystem", plugins.begin()->Name()); + auto plugin = plugins.begin(); + + EXPECT_EQ("default", plugin->EntityName()); + EXPECT_EQ("world", plugin->EntityType()); + EXPECT_EQ("TestWorldSystem", plugin->Filename()); + EXPECT_EQ("ignition::gazebo::TestWorldSystem", plugin->Name()); + + plugin = std::next(plugin, 1); + + EXPECT_EQ("box", plugin->EntityName()); + EXPECT_EQ("model", plugin->EntityType()); + EXPECT_EQ("TestModelSystem", plugin->Filename()); + EXPECT_EQ("ignition::gazebo::TestModelSystem", plugin->Name()); + + plugin = std::next(plugin, 1); + + EXPECT_EQ("default::box::link_1::camera", plugin->EntityName()); + EXPECT_EQ("sensor", plugin->EntityType()); + EXPECT_EQ("TestSensorSystem", plugin->Filename()); + EXPECT_EQ("ignition::gazebo::TestSensorSystem", plugin->Name()); } ////////////////////////////////////////////////// -TEST(parsePluginsFromString, invalid) +TEST(ParsePluginsFromString, Invalid) { std::string config = R"( @@ -86,25 +102,41 @@ TEST(parsePluginsFromString, invalid) } ////////////////////////////////////////////////// -TEST(parsePluginsFromFile, valid) +TEST(ParsePluginsFromFile, Valid) { auto config = common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/server_valid.config"); + "test", "worlds", "server_valid.config"); auto plugins = parsePluginsFromFile(config); ASSERT_EQ(3u, plugins.size()); - EXPECT_EQ("default", plugins.begin()->EntityName()); - EXPECT_EQ("world", plugins.begin()->EntityType()); - EXPECT_EQ("TestWorldSystem", plugins.begin()->Filename()); - EXPECT_EQ("ignition::gazebo::TestWorldSystem", plugins.begin()->Name()); + auto plugin = plugins.begin(); + + EXPECT_EQ("default", plugin->EntityName()); + EXPECT_EQ("world", plugin->EntityType()); + EXPECT_EQ("TestWorldSystem", plugin->Filename()); + EXPECT_EQ("ignition::gazebo::TestWorldSystem", plugin->Name()); + + plugin = std::next(plugin, 1); + + EXPECT_EQ("box", plugin->EntityName()); + EXPECT_EQ("model", plugin->EntityType()); + EXPECT_EQ("TestModelSystem", plugin->Filename()); + EXPECT_EQ("ignition::gazebo::TestModelSystem", plugin->Name()); + + plugin = std::next(plugin, 1); + + EXPECT_EQ("default::box::link_1::camera", plugin->EntityName()); + EXPECT_EQ("sensor", plugin->EntityType()); + EXPECT_EQ("TestSensorSystem", plugin->Filename()); + EXPECT_EQ("ignition::gazebo::TestSensorSystem", plugin->Name()); } ////////////////////////////////////////////////// -TEST(parsePluginsFromFile, invalid) +TEST(ParsePluginsFromFile, Invalid) { auto config = common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/server_invalid.config"); + "test", "worlds", "server_invalid.config"); // Valid file without valid content auto plugins = parsePluginsFromFile(config); @@ -116,35 +148,35 @@ TEST(parsePluginsFromFile, invalid) } ////////////////////////////////////////////////// -TEST(parsePluginsFromFile, defaultConfig) +TEST(ParsePluginsFromFile, DefaultConfig) { // Note: This test validates that that the default // configuration always parses. // If more systems are added, then the number needs // to be adjusted below. auto config = common::joinPaths(PROJECT_SOURCE_PATH, - "/include/ignition/gazebo/server.config"); + "include", "ignition", "gazebo", "server.config"); auto plugins = parsePluginsFromFile(config); ASSERT_EQ(3u, plugins.size()); } ////////////////////////////////////////////////// -TEST(parsePluginsFromFile, playbackConfig) +TEST(ParsePluginsFromFile, PlaybackConfig) { // Note: This test validates that that the default // configuration always parses. // If more systems are added, then the number needs // to be adjusted below. auto config = common::joinPaths(PROJECT_SOURCE_PATH, - "/include/ignition/gazebo/playback_server.config"); + "include", "ignition", "gazebo", "playback_server.config"); auto plugins = parsePluginsFromFile(config); ASSERT_EQ(2u, plugins.size()); } ////////////////////////////////////////////////// -TEST(loadPluginInfo, fromEmptyEnv) +TEST(LoadPluginInfo, FromEmptyEnv) { // Set environment to something that doesn't exist ASSERT_TRUE(common::setenv(gazebo::kServerConfigPathEnv, "foo")); @@ -155,26 +187,35 @@ TEST(loadPluginInfo, fromEmptyEnv) } ////////////////////////////////////////////////// -TEST(loadPluginInfo, fromValidEnv) +TEST(LoadPluginInfo, FromValidEnv) { auto validPath = common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/server_valid2.config"); + "test", "worlds", "server_valid2.config"); ASSERT_TRUE(common::setenv(gazebo::kServerConfigPathEnv, validPath)); auto plugins = loadPluginInfo(); ASSERT_EQ(2u, plugins.size()); - EXPECT_EQ("*", plugins.begin()->EntityName()); - EXPECT_EQ("world", plugins.begin()->EntityType()); - EXPECT_EQ("TestWorldSystem", plugins.begin()->Filename()); - EXPECT_EQ("ignition::gazebo::TestWorldSystem", plugins.begin()->Name()); + auto plugin = plugins.begin(); + + EXPECT_EQ("*", plugin->EntityName()); + EXPECT_EQ("world", plugin->EntityType()); + EXPECT_EQ("TestWorldSystem", plugin->Filename()); + EXPECT_EQ("ignition::gazebo::TestWorldSystem", plugin->Name()); + + plugin = std::next(plugin, 1); + + EXPECT_EQ("box", plugin->EntityName()); + EXPECT_EQ("model", plugin->EntityType()); + EXPECT_EQ("TestModelSystem", plugin->Filename()); + EXPECT_EQ("ignition::gazebo::TestModelSystem", plugin->Name()); EXPECT_TRUE(common::unsetenv(gazebo::kServerConfigPathEnv)); } ////////////////////////////////////////////////// -TEST(ServerConfig, generateRecordPlugin) +TEST(ServerConfig, GenerateRecordPlugin) { ServerConfig config; config.SetUseLogRecord(true); diff --git a/src/ServerPrivate.cc b/src/ServerPrivate.cc index 9c75c8a463..4b15c260fa 100644 --- a/src/ServerPrivate.cc +++ b/src/ServerPrivate.cc @@ -328,6 +328,60 @@ void ServerPrivate::AddRecordPlugin(const ServerConfig &_config) pluginElem = pluginElem->GetNextElement(); } } + + // A record plugin is not already specified in SDF. Add one. + sdf::ElementPtr recordElem = worldElem->AddElement("plugin"); + sdf::ParamPtr recordName = recordElem->GetAttribute("name"); + recordName->SetFromString(LoggingPlugin::RecordPluginName()); + sdf::ParamPtr recordFileName = recordElem->GetAttribute("filename"); + recordFileName->SetFromString(LoggingPlugin::LoggingPluginFileName()); + + // Add custom record path + if (!_config.LogRecordPath().empty()) + { + sdf::ElementPtr pathElem = std::make_shared(); + pathElem->SetName("path"); + recordElem->AddElementDescription(pathElem); + pathElem = recordElem->GetElement("path"); + pathElem->AddValue("string", "", false, ""); + pathElem->Set(_config.LogRecordPath()); + } + + // Set whether to record resources + sdf::ElementPtr resourceElem = std::make_shared(); + resourceElem->SetName("record_resources"); + recordElem->AddElementDescription(resourceElem); + resourceElem = recordElem->GetElement("record_resources"); + resourceElem->AddValue("bool", "false", false, ""); + resourceElem->Set(_config.LogRecordResources() ? true : false); + + // Set whether to compress + sdf::ElementPtr compressElem = std::make_shared(); + compressElem->SetName("compress"); + recordElem->AddElementDescription(compressElem); + compressElem = recordElem->GetElement("compress"); + compressElem->AddValue("bool", "false", false, ""); + compressElem->Set(_config.LogRecordCompressPath().empty() ? false : + true); + + // Set compress path + sdf::ElementPtr cPathElem = std::make_shared(); + cPathElem->SetName("compress_path"); + recordElem->AddElementDescription(cPathElem); + cPathElem = recordElem->GetElement("compress_path"); + cPathElem->AddValue("string", "", false, ""); + cPathElem->Set(_config.LogRecordCompressPath()); + + // If record topics specified, add in SDF + for (const std::string &topic : _config.LogRecordTopics()) + { + sdf::ElementPtr topicElem = std::make_shared(); + topicElem->SetName("record_topic"); + recordElem->AddElementDescription(topicElem); + topicElem = recordElem->AddElement("record_topic"); + topicElem->AddValue("string", "false", false, ""); + topicElem->Set(topic); + } } ////////////////////////////////////////////////// diff --git a/src/Server_TEST.cc b/src/Server_TEST.cc index 44b0f48923..0947ca6821 100644 --- a/src/Server_TEST.cc +++ b/src/Server_TEST.cc @@ -59,12 +59,6 @@ class ServerFixture : public ::testing::TestWithParam ///////////////////////////////////////////////// TEST_P(ServerFixture, DefaultServerConfig) { - // Always override to the default config, in case - // a user has customized their installed config - auto validPath = common::joinPaths(PROJECT_SOURCE_PATH, - "/include/ignition/gazebo/server.config"); - ASSERT_TRUE(common::setenv(gazebo::kServerConfigPathEnv, validPath)); - ignition::gazebo::ServerConfig serverConfig; EXPECT_TRUE(serverConfig.SdfFile().empty()); EXPECT_TRUE(serverConfig.SdfString().empty()); @@ -227,8 +221,8 @@ TEST_P(ServerFixture, ServerConfigSensorPlugin) { // Start server ServerConfig serverConfig; - serverConfig.SetSdfFile(std::string(PROJECT_SOURCE_PATH) + - "/test/worlds/air_pressure.sdf"); + serverConfig.SetSdfFile(common::joinPaths(PROJECT_SOURCE_PATH, + "test", "worlds", "air_pressure.sdf")); sdf::ElementPtr sdf(new sdf::Element); sdf->SetName("plugin"); diff --git a/src/SimulationRunner_TEST.cc b/src/SimulationRunner_TEST.cc index f3d37168b5..5d53fac7dd 100644 --- a/src/SimulationRunner_TEST.cc +++ b/src/SimulationRunner_TEST.cc @@ -86,7 +86,7 @@ class SimulationRunnerTest : public ::testing::TestWithParam common::Console::SetVerbosity(4); common::setenv("IGN_GAZEBO_SYSTEM_PLUGIN_PATH", - common::joinPaths(PROJECT_BINARY_PATH, "/lib")); + common::joinPaths(PROJECT_BINARY_PATH, "lib")); } }; @@ -112,7 +112,7 @@ TEST_P(SimulationRunnerTest, CreateEntities) // Load SDF file sdf::Root root; root.Load(common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/shapes.sdf")); + "test", "worlds", "shapes.sdf")); ASSERT_EQ(1u, root.WorldCount()); @@ -525,7 +525,7 @@ TEST_P(SimulationRunnerTest, CreateLights) // Load SDF file sdf::Root root; root.Load(common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/lights.sdf")); + "test", "worlds", "lights.sdf")); ASSERT_EQ(1u, root.WorldCount()); @@ -795,7 +795,7 @@ TEST_P(SimulationRunnerTest, CreateJointEntities) // Load SDF file sdf::Root root; root.Load(common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/demo_joint_types.sdf")); + "test", "worlds", "demo_joint_types.sdf")); ASSERT_EQ(1u, root.WorldCount()); @@ -936,7 +936,7 @@ TEST_P(SimulationRunnerTest, Time) // Load SDF file sdf::Root root; root.Load(common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/shapes.sdf")); + "test", "worlds", "shapes.sdf")); ASSERT_EQ(1u, root.WorldCount()); @@ -1058,7 +1058,7 @@ TEST_P(SimulationRunnerTest, LoadPlugins) // Load SDF file sdf::Root root; root.Load(common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/plugins.sdf")); + "test", "worlds", "plugins.sdf")); ASSERT_EQ(1u, root.WorldCount()); @@ -1144,12 +1144,12 @@ TEST_P(SimulationRunnerTest, LoadServerNoPlugins) { sdf::Root rootWithout; rootWithout.Load(common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/plugins_empty.sdf")); + "test", "worlds", "plugins_empty.sdf")); ASSERT_EQ(1u, rootWithout.WorldCount()); // ServerConfig will fall back to environment variable auto config = common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/server_valid2.config"); + "test", "worlds", "server_valid2.config"); ASSERT_EQ(true, common::setenv(gazebo::kServerConfigPathEnv, config)); ServerConfig serverConfig; @@ -1166,13 +1166,13 @@ TEST_P(SimulationRunnerTest, LoadServerConfigPlugins) { sdf::Root rootWithout; rootWithout.Load(common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/plugins_empty.sdf")); + "test", "worlds", "plugins_empty.sdf")); ASSERT_EQ(1u, rootWithout.WorldCount()); // Create a server configuration with plugins // No fallback expected auto config = common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/server_valid.config"); + "test", "worlds", "server_valid.config"); auto plugins = parsePluginsFromFile(config); ASSERT_EQ(3u, plugins.size()); @@ -1266,11 +1266,13 @@ TEST_P(SimulationRunnerTest, LoadPluginsDefault) { sdf::Root rootWithout; rootWithout.Load(common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/plugins_empty.sdf")); + "test", "worlds", "plugins_empty.sdf")); ASSERT_EQ(1u, rootWithout.WorldCount()); + // Load the default config, but not through the default code path. + // The user may have modified their local config. auto config = common::joinPaths(PROJECT_SOURCE_PATH, - "/include/ignition/gazebo/server.config"); + "include", "ignition", "gazebo", "server.config"); ASSERT_TRUE(common::setenv(gazebo::kServerConfigPathEnv, config)); // Create simulation runner @@ -1286,7 +1288,7 @@ TEST_P(SimulationRunnerTest, LoadPluginsEvent) // Load SDF file without plugins sdf::Root rootWithout; rootWithout.Load(common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/shapes.sdf")); + "test", "worlds", "shapes.sdf")); ASSERT_EQ(1u, rootWithout.WorldCount()); // Create simulation runner @@ -1321,7 +1323,7 @@ TEST_P(SimulationRunnerTest, LoadPluginsEvent) // Load SDF file with plugins sdf::Root rootWith; rootWith.Load(common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/plugins.sdf")); + "test", "worlds", "plugins.sdf")); ASSERT_EQ(1u, rootWith.WorldCount()); // Emit plugin loading event @@ -1379,7 +1381,7 @@ TEST_P(SimulationRunnerTest, GuiInfo) // Load SDF file sdf::Root root; root.Load(common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/shapes.sdf")); + "test", "worlds", "shapes.sdf")); ASSERT_EQ(1u, root.WorldCount()); @@ -1416,7 +1418,7 @@ TEST_P(SimulationRunnerTest, GenerateWorldSdf) // Load SDF file sdf::Root root; root.Load(common::joinPaths(PROJECT_SOURCE_PATH, - "/test/worlds/shapes.sdf")); + "test", "worlds", "shapes.sdf")); ASSERT_EQ(1u, root.WorldCount()); diff --git a/src/cmd/cmdgazebo.rb.in b/src/cmd/cmdgazebo.rb.in index 37c9c24ddd..0baa713665 100755 --- a/src/cmd/cmdgazebo.rb.in +++ b/src/cmd/cmdgazebo.rb.in @@ -156,8 +156,7 @@ COMMANDS = { 'gazebo' => " resources such as worlds and models. \n\n"\ " IGN_GAZEBO_SYSTEM_PLUGIN_PATH Colon separated paths used to \n"\ " locate system plugins. \n\n"\ - " IGN_GAZEBO_SERVER_CONFIG_PATH Colon separated paths used to \n"\ - " locate server configurations. \n\n"\ + " IGN_GAZEBO_SERVER_CONFIG_PATH Path to server configuration file. \n\n"\ " IGN_GUI_PLUGIN_PATH Colon separated paths used to locate GUI \n"\ " plugins. \n\n"\ " IGN_GAZEBO_NETWORK_ROLE Participant role used in a distributed \n"\ diff --git a/src/systems/log/LogRecord.cc b/src/systems/log/LogRecord.cc index 2e0e366513..9cc417088f 100644 --- a/src/systems/log/LogRecord.cc +++ b/src/systems/log/LogRecord.cc @@ -38,7 +38,15 @@ #include #include -#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include "ignition/gazebo/components/Geometry.hh" #include "ignition/gazebo/components/Light.hh" @@ -655,7 +663,7 @@ void LogRecord::PostUpdate(const UpdateInfo &_info, else { auto worldSdfComp = _ecm.Component(worldEntity); - if (worldSdfComp == nullptr) + if (worldSdfComp == nullptr || worldSdfComp->Data().Element() == nullptr) { ignerr << "Could not load world SDF data\n"; } diff --git a/tutorials/server_config.md b/tutorials/server_config.md index 4f89c9b337..42fdec843d 100644 --- a/tutorials/server_config.md +++ b/tutorials/server_config.md @@ -2,7 +2,7 @@ Most functionality on Ignition Gazebo is provided by plugins, which means that users can choose exactly what functionality is available to their simulations. -Even running the physics engine is optional. This giver users great control +Even running the physics engine is optional. This gives users great control and makes sure only what's crucial for a given simulation is loaded. This tutorial will go over how to specify what system plugins to be loaded for