Towards Green CI (gazebosim#1771)

Apply fixes to get CI closer to green on all platforms.

Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
Co-authored-by: Nate Koenig <[email protected]>

.gitignore

@@ -15,3 +15,8 @@ build_*
 
 # clangd index
 .cache
+
+
+# Python cache
+__pycache__
+*.pyc

python/CMakeLists.txt

@@ -28,6 +28,7 @@ else()
   set(GZ_PYTHON_INSTALL_PATH ${GZ_LIB_INSTALL_DIR}/python)
 endif()
 
+set(GZ_PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
 set(GZ_PYTHON_INSTALL_PATH "${GZ_PYTHON_INSTALL_PATH}/gz")
 
 # Set the build location and install location for a CPython extension
@@ -88,7 +89,7 @@ if (BUILD_TESTING)
     testFixture_TEST
   )
 
-  execute_process(COMMAND "${PYTHON_EXECUTABLE}" -m pytest --version
+  execute_process(COMMAND "${GZ_PYTHON_EXECUTABLE}" -m pytest --version
     OUTPUT_VARIABLE PYTEST_output
     ERROR_VARIABLE  PYTEST_error
     RESULT_VARIABLE PYTEST_result)
@@ -101,17 +102,17 @@ if (BUILD_TESTING)
 
   foreach (test ${python_tests})
     if (pytest_FOUND)
-      add_test(NAME ${test}.py COMMAND
-        "${PYTHON_EXECUTABLE}" -m pytest "${CMAKE_SOURCE_DIR}/python/test/${test}.py" --junitxml "${CMAKE_BINARY_DIR}/test_results/UNIT_${test}.xml")
+      add_test(NAME ${test} COMMAND
+        "${GZ_PYTHON_EXECUTABLE}" -m pytest "${CMAKE_SOURCE_DIR}/python/test/${test}.py" --junitxml "${CMAKE_BINARY_DIR}/test_results/UNIT_${test}.xml")
     else()
-      add_test(NAME ${test}.py COMMAND
-        "${PYTHON_EXECUTABLE}" "${CMAKE_SOURCE_DIR}/python/test/${test}.py")
+      add_test(NAME ${test} COMMAND
+        "${GZ_PYTHON_EXECUTABLE}" "${CMAKE_SOURCE_DIR}/python/test/${test}.py")
     endif()
 
     set(_env_vars)
     list(APPEND _env_vars "PYTHONPATH=${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/python/")
     list(APPEND _env_vars "LD_LIBRARY_PATH=${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}:$ENV{LD_LIBRARY_PATH}")
-    set_tests_properties(${test}.py PROPERTIES
+    set_tests_properties(${test} PROPERTIES
       ENVIRONMENT "${_env_vars}")
   endforeach()
 endif()

src/Conversions_TEST.cc

@@ -1032,9 +1032,10 @@ TEST(Conversions, ParticleEmitter)
   EXPECT_NEAR(0.2, emitterMsg.max_velocity().data(), 1e-3);
   EXPECT_EQ(math::Vector3d(1, 2, 3), msgs::Convert(emitterMsg.size()));
   EXPECT_EQ(math::Vector3d(4, 5, 6), msgs::Convert(emitterMsg.particle_size()));
-  EXPECT_EQ(math::Color(0.1, 0.2, 0.3),
+  EXPECT_EQ(math::Color(0.1f, 0.2f, 0.3f),
       msgs::Convert(emitterMsg.color_start()));
-  EXPECT_EQ(math::Color(0.4, 0.5, 0.6), msgs::Convert(emitterMsg.color_end()));
+  EXPECT_EQ(math::Color(0.4f, 0.5f, 0.6f),
+      msgs::Convert(emitterMsg.color_end()));
   EXPECT_EQ("range_image", emitterMsg.color_range_image().data());
 
   auto header = emitterMsg.header().data(0);

src/SdfGenerator_TEST.cc

@@ -348,7 +348,7 @@ class ModelElementFixture : public ElementUpdateFixture
 /////////////////////////////////////////////////
 TEST_F(ModelElementFixture, ModelsInline)
 {
-  this->LoadWorld("test/worlds/shapes.sdf");
+  this->LoadWorld(common::joinPaths("test", "worlds", "shapes.sdf"));
   this->TestModel("box");
   this->TestModel("capsule");
   this->TestModel("cylinder");
@@ -359,15 +359,15 @@ TEST_F(ModelElementFixture, ModelsInline)
 /////////////////////////////////////////////////
 TEST_F(ModelElementFixture, ModelIncluded)
 {
-  this->LoadWorld("test/worlds/save_world.sdf");
+  this->LoadWorld(common::joinPaths("test", "worlds", "save_world.sdf"));
   this->TestModel("backpack1");
   this->TestModel("backpack2");
 }
 
 /////////////////////////////////////////////////
 TEST_F(ModelElementFixture, ModelComponentUpdate)
 {
-  this->LoadWorld("test/worlds/shapes.sdf");
+  this->LoadWorld(common::joinPaths("test", "worlds", "shapes.sdf"));
   std::string modelName = "box";
   Entity modelEntity = this->ecm.EntityByComponents(
       components::Model(), components::Name(modelName));
@@ -461,8 +461,7 @@ TEST_F(ElementUpdateFixture, ConfigOverrideCopyOrMerge)
 /////////////////////////////////////////////////
 TEST_F(ElementUpdateFixture, ConfigOverride)
 {
-  const std::string worldFile{"test/worlds/save_world.sdf"};
-  this->LoadWorld(worldFile);
+  this->LoadWorld(common::joinPaths("test", "worlds", "save_world.sdf"));
   Entity worldEntity = this->ecm.EntityByComponents(components::World());
   {
     this->sdfGenConfig.mutable_global_entity_gen_config()
@@ -522,8 +521,7 @@ TEST_F(ElementUpdateFixture, ConfigOverride)
 /////////////////////////////////////////////////
 TEST_F(ElementUpdateFixture, WorldWithModelsInline)
 {
-  const std::string worldFile{"test/worlds/shapes.sdf"};
-  this->LoadWorld(worldFile);
+  this->LoadWorld(common::joinPaths("test", "worlds", "shapes.sdf"));
   Entity worldEntity = this->ecm.EntityByComponents(components::World());
   auto elem = std::make_shared<sdf::Element>();
   sdf::initFile("world.sdf", elem);
@@ -535,7 +533,7 @@ TEST_F(ElementUpdateFixture, WorldWithModelsInline)
 /////////////////////////////////////////////////
 TEST_F(ElementUpdateFixture, WorldWithModelsIncludedExpanded)
 {
-  this->LoadWorld("test/worlds/save_world.sdf");
+  this->LoadWorld(common::joinPaths("test", "worlds", "save_world.sdf"));
   Entity worldEntity = this->ecm.EntityByComponents(components::World());
   auto elem = std::make_shared<sdf::Element>();
   sdf::initFile("world.sdf", elem);
@@ -586,7 +584,7 @@ TEST_F(ElementUpdateFixture, WorldComponentUpdate)
 /////////////////////////////////////////////////
 TEST_F(ElementUpdateFixture, WorldWithModelsIncludedNotExpanded)
 {
-  const std::string worldFile{"test/worlds/save_world.sdf"};
+  const auto worldFile = common::joinPaths("test", "worlds", "save_world.sdf");
   this->LoadWorld(worldFile);
   Entity worldEntity = this->ecm.EntityByComponents(components::World());
   auto elem = std::make_shared<sdf::Element>();
@@ -644,17 +642,17 @@ TEST_F(ElementUpdateFixture, WorldWithModelsIncludedNotExpanded)
 TEST_F(ElementUpdateFixture, WorldWithModelsIncludedWithInvalidUris)
 {
   const std::string goodUri =
-      "https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Backpack/2";
+      "https://fuel.gazebosim.org/1.0/openroboticstest/models/backpack/2";
 
   // These are URIs that are potentially problematic.
   const std::vector<std::string> fuelUris = {
       // Thes following two URIs are valid, but have a trailing '/'
-      "https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Backpack/",
-      "https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Backpack/2/",
+      "https://fuel.gazebosim.org/1.0/openroboticstest/models/backpack/",
+      "https://fuel.gazebosim.org/1.0/openroboticstest/models/backpack/2/",
       // Thes following two URIs are invalid, and will not be saved
-      "https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Backpack/"
+      "https://fuel.gazebosim.org/1.0/openroboticstest/models/backpack/"
       "notInt",
-      "https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Backpack/"
+      "https://fuel.gazebosim.org/1.0/openroboticstest/models/backpack/"
       "notInt/",
   };
 
@@ -709,7 +707,7 @@ TEST_F(ElementUpdateFixture, WorldWithModelsIncludedWithInvalidUris)
 TEST_F(ElementUpdateFixture, WorldWithModelsIncludedWithNonFuelUris)
 {
   const std::vector<std::string> includeUris = {
-      "https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Backpack",
+      "https://fuel.gazebosim.org/1.0/openroboticstest/models/backpack",
       std::string("file://") + PROJECT_SOURCE_PATH +
           "/test/worlds/models/sphere"};
 
@@ -768,7 +766,7 @@ TEST_F(ElementUpdateFixture, WorldWithModelsIncludedWithNonFuelUris)
 /////////////////////////////////////////////////
 TEST_F(ElementUpdateFixture, WorldWithModelsIncludedWithOneExpanded)
 {
-  const std::string worldFile{"test/worlds/save_world.sdf"};
+  const auto worldFile = common::joinPaths("test", "worlds", "save_world.sdf");
   this->LoadWorld(worldFile);
   Entity worldEntity = this->ecm.EntityByComponents(components::World());
   auto elem = std::make_shared<sdf::Element>();
@@ -947,8 +945,7 @@ using GenerateWorldFixture = ElementUpdateFixture;
 /////////////////////////////////////////////////
 TEST_F(GenerateWorldFixture, ModelsInline)
 {
-  const std::string worldFile{"test/worlds/save_world.sdf"};
-  this->LoadWorld(worldFile);
+  this->LoadWorld(common::joinPaths("test", "worlds", "save_world.sdf"));
   Entity worldEntity = this->ecm.EntityByComponents(components::World());
   // Check with expandIncludeTags = true
   {

src/Server.cc

@@ -202,7 +202,7 @@ bool Server::Run(const bool _blocking, const uint64_t _iterations,
     // running variable gets updated before this function returns. With
     // a small number of iterations it is possible that the run thread
     // successfuly completes before this function returns.
-    cond.wait(lock);
+    cond.wait(lock, [this]() -> bool {return this->dataPtr->running;});
     return true;
   }
 

src/ServerPrivate.cc

@@ -227,9 +227,8 @@ void ServerPrivate::AddRecordPlugin(const ServerConfig &_config)
           {
             auto topic = recordTopicElem->Get<std::string>();
             sdfRecordTopics.push_back(topic);
+            recordTopicElem = recordTopicElem->GetNextElement();
           }
-
-          recordTopicElem = recordTopicElem->GetNextElement();
         }
 
         // Remove the plugin, which will be added back in by ServerConfig.

src/Server_TEST.cc

@@ -1217,14 +1217,31 @@ TEST_P(ServerFixture, Stop)
 
   sim::Server server(serverConfig);
 
+  std::mutex testMutex;
+  std::condition_variable testCv;
+  test::Relay testSystem;
+  unsigned int preUpdates{0};
+  testSystem.OnPreUpdate(
+    [&](const sim::UpdateInfo &,
+    sim::EntityComponentManager &)
+    {
+      std::scoped_lock localLock(testMutex);
+      ++preUpdates;
+      testCv.notify_one();
+      return true;
+    });
+
+  server.AddSystem(testSystem.systemPtr);
   // The simulation runner should not be running.
   EXPECT_FALSE(*server.Running(0));
   EXPECT_FALSE(server.Running());
 
   // Run the server.
+  std::unique_lock<std::mutex> testLock(testMutex);
   EXPECT_TRUE(server.Run(false, 0, false));
-  EXPECT_TRUE(*server.Running(0));
   EXPECT_TRUE(server.Running());
+  testCv.wait(testLock, [&]() -> bool {return preUpdates > 0;});
+  EXPECT_TRUE(*server.Running(0));
 
   // Stop the server
   server.Stop();

src/SimulationRunner.cc

@@ -502,7 +502,8 @@ void SimulationRunner::ProcessSystemQueue()
 
   this->systemMgr->ActivatePendingSystems();
 
-  auto threadCount = this->systemMgr->SystemsPostUpdate().size() + 1u;
+  unsigned int threadCount =
+    static_cast<unsigned int>(this->systemMgr->SystemsPostUpdate().size() + 1u);
 
   gzdbg << "Creating PostUpdate worker threads: "
     << threadCount << std::endl;

src/gz_TEST.cc

@@ -101,7 +101,7 @@ TEST(CmdLine, GZ_UTILS_TEST_DISABLED_ON_WIN32(CachedFuelWorld))
   std::string projectPath = std::string(PROJECT_SOURCE_PATH) + "/test/worlds";
   gz::common::setenv("GZ_FUEL_CACHE_PATH", projectPath.c_str());
   std::string cmd = kGzCommand + " -r -v 4 --iterations 5" +
-    " https://fuel.ignitionrobotics.org/1.0/OpenRobotics/worlds/Test%20world";
+    " https://fuel.gazebosim.org/1.0/openroboticstest/worlds/test%20world";
   std::cout << "Running command [" << cmd << "]" << std::endl;
 
   std::string output = customExecStr(cmd);

src/systems/acoustic_comms/AcousticComms.cc

@@ -57,10 +57,10 @@ AcousticComms::AcousticComms()
 
 //////////////////////////////////////////////////
 void AcousticComms::Load(
-    const Entity &_entity,
+    const Entity &/*_entity*/,
     std::shared_ptr<const sdf::Element> _sdf,
-    EntityComponentManager &_ecm,
-    EventManager &_eventMgr)
+    EntityComponentManager &/*_ecm*/,
+    EventManager &/*_eventMgr*/)
 {
   if (_sdf->HasElement("max_range"))
   {

src/systems/optical_tactile_plugin/OpticalTactilePlugin.cc

@@ -767,8 +767,7 @@ void OpticalTactilePluginPrivate::ComputeNormalForces(
   normalsMsg.set_step(3 * sizeof(float) * _msg.width());
   normalsMsg.set_pixel_format_type(gz::msgs::PixelFormatType::R_FLOAT32);
 
-  uint32_t bufferSize = 3 * sizeof(float) * _msg.width() * _msg.height();
-  std::shared_ptr<char> normalForcesBuffer(new char[bufferSize]);
+  std::vector<float> normalForcesBuffer(3 * _msg.width() * _msg.height());
   uint32_t bufferIndex;
 
   // Marker messages representing the normal forces
@@ -804,11 +803,10 @@ void OpticalTactilePluginPrivate::ComputeNormalForces(
       // Add force to buffer
       // Forces buffer is composed of XYZ coordinates, while _msg buffer is
       // made up of XYZRGB values
-      bufferIndex = j * (_msg.row_step() / 2) + i * (_msg.point_step() / 2);
-      normalForcesBuffer.get()[bufferIndex] = normalForce.X();
-      normalForcesBuffer.get()[bufferIndex + sizeof(float)] = normalForce.Y();
-      normalForcesBuffer.get()[bufferIndex + 2 * sizeof(float)] =
-        normalForce.Z();
+      bufferIndex = j * (_msg.width() * 3) + i * 3;
+      normalForcesBuffer[bufferIndex] = normalForce.X();
+      normalForcesBuffer[bufferIndex+1] = normalForce.Y();
+      normalForcesBuffer[bufferIndex+2] = normalForce.Z();
 
       if (!_visualizeForces)
         continue;
@@ -820,9 +818,10 @@ void OpticalTactilePluginPrivate::ComputeNormalForces(
     }
   }
 
+  std::string *dataStr = normalsMsg.mutable_data();
+  dataStr->resize(sizeof(float) * normalForcesBuffer.size());
+  memcpy(&((*dataStr)[0]), normalForcesBuffer.data(), dataStr->size());
   // Publish message
-  normalsMsg.set_data(normalForcesBuffer.get(),
-    3 * sizeof(float) * _msg.width() * _msg.height());
   this->normalForcesPub.Publish(normalsMsg);
 
   if (_visualizeForces)

src/systems/rf_comms/RFComms.cc

@@ -257,7 +257,8 @@ std::tuple<bool, double> RFComms::Implementation::AttemptSend(
 
   // Compute prospective accumulated bits along with time window
   // (including this packet).
-  double bitsSent = (_txState.bytesSentThisEpoch + _numBytes) * 8;
+  auto bitsSent =
+    static_cast<double>((_txState.bytesSentThisEpoch + _numBytes) * 8);
 
   // Check current epoch bitrate vs capacity and fail to send accordingly
   if (bitsSent > this->radioConfig.capacity * this->epochDuration)
@@ -286,8 +287,9 @@ std::tuple<bool, double> RFComms::Implementation::AttemptSend(
   // error rate (BER).
   double ber = this->QPSKPowerToBER(
     this->DbmToPow(rxPower), this->DbmToPow(this->radioConfig.noiseFloor));
+  double packetDropProb =
 
-  double packetDropProb = 1.0 - exp(_numBytes * log(1 - ber));
+    1.0 - exp(static_cast<double>(_numBytes) * log(1 - ber));
 
   // gzdbg << "TX power (dBm): " << this->radioConfig.txPower << "\n" <<
   //           "RX power (dBm): " << rxPower << "\n" <<
@@ -315,7 +317,8 @@ std::tuple<bool, double> RFComms::Implementation::AttemptSend(
 
   // Compute prospective accumulated bits along with time window
   // (including this packet).
-  double bitsReceived = (_rxState.bytesReceivedThisEpoch + _numBytes) * 8;
+  auto bitsReceived =
+    static_cast<double>((_rxState.bytesReceivedThisEpoch + _numBytes) * 8);
 
   // Check current epoch bitrate vs capacity and fail to send accordingly.
   if (bitsReceived > this->radioConfig.capacity * this->epochDuration)

src/systems/shader_param/ShaderParam.cc

@@ -441,7 +441,7 @@ void ShaderParamPrivate::OnUpdate()
         if (!spv.type.empty() && spv.type == "int_array")
         {
           for (const auto &v : values)
-            floatArrayValue.push_back(std::stoi(v));
+            floatArrayValue.push_back(std::stof(v));
           paramType = rendering::ShaderParam::PARAM_INT_BUFFER;
         }
         // treat everything else as float_array

src/systems/track_controller/TrackController.cc

@@ -533,7 +533,7 @@ void TrackControllerPrivate::ComputeSurfaceProperties(
     gz::msgs::Set(this->debugMarker.mutable_pose(), math::Pose3d(
       p.X(), p.Y(), p.Z(), rot.Roll(), rot.Pitch(), rot.Yaw()));
     this->debugMarker.mutable_material()->mutable_diffuse()->set_r(
-      surfaceMotion >= 0 ? 0 : 1);
+      surfaceMotion >= 0 ? 0.0f : 1.0f);
 
     this->node.Request("/marker", this->debugMarker);
   }

src/systems/trajectory_follower/TrajectoryFollower.cc

@@ -421,7 +421,7 @@ void TrajectoryFollower::PreUpdate(
       }
     }
 
-    int sign = std::abs(bearing.Degree()) / bearing.Degree();
+    int sign = static_cast<int>(std::abs(bearing.Degree()) / bearing.Degree());
     torqueWorld = (*comPose).Rot().RotateVector(
        gz::math::Vector3d(0, 0, sign * this->dataPtr->torqueToApply));
   }