Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the symbols if they are already loaded #304

Closed
wants to merge 11 commits into from
6 changes: 5 additions & 1 deletion src/SystemLoader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,11 @@ class ignition::gazebo::SystemLoaderPrivate
return false;
}

auto pluginName = *pluginNames.begin();
std::string pluginName = "";
for (auto name : pluginNames)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (auto name : pluginNames)
for (const auto &name : pluginNames)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I applied this change in 2f674e6

{
pluginName = name;
}
if (pluginName.empty())
{
ignerr << "Failed to load system plugin [" << _filename <<
Expand Down
4 changes: 2 additions & 2 deletions src/systems/air_pressure/AirPressure.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ void AirPressurePrivate::CreateAirPressureEntities(EntityComponentManager &_ecm)
data.SetTopic(topic);
}
std::unique_ptr<sensors::AirPressureSensor> sensor =
this->sensorFactory.CreateSensor<
sensors::AirPressureSensor>(data);
std::make_unique<sensors::AirPressureSensor>();
sensor->Load(data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at the CreateSensor function, it also makes a sensor->Init() call in addition to sensor->Load

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added init() method 3f42c00

// set sensor parent
std::string parentName = _ecm.Component<components::Name>(
_parent->Data())->Data();
Expand Down
4 changes: 2 additions & 2 deletions src/systems/altimeter/Altimeter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ void AltimeterPrivate::CreateAltimeterEntities(EntityComponentManager &_ecm)
data.SetTopic(topic);
}
std::unique_ptr<sensors::AltimeterSensor> sensor =
this->sensorFactory.CreateSensor<
sensors::AltimeterSensor>(data);
std::make_unique<sensors::AltimeterSensor>();
sensor->Load(data);
// set sensor parent
std::string parentName = _ecm.Component<components::Name>(
_parent->Data())->Data();
Expand Down
4 changes: 2 additions & 2 deletions src/systems/imu/Imu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ void ImuPrivate::CreateImuEntities(EntityComponentManager &_ecm)
data.SetTopic(topic);
}
std::unique_ptr<sensors::ImuSensor> sensor =
this->sensorFactory.CreateSensor<
sensors::ImuSensor>(data);
std::make_unique<sensors::ImuSensor>();
sensor->Load(data);
// set sensor parent
std::string parentName = _ecm.Component<components::Name>(
_parent->Data())->Data();
Expand Down
4 changes: 2 additions & 2 deletions src/systems/logical_camera/LogicalCamera.cc
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ void LogicalCameraPrivate::CreateLogicalCameraEntities(
data->GetElement("topic")->Set(topic);
}
std::unique_ptr<sensors::LogicalCameraSensor> sensor =
this->sensorFactory.CreateSensor<
sensors::LogicalCameraSensor>(data);
std::make_unique<sensors::LogicalCameraSensor>();
sensor->Load(data);
// set sensor parent
std::string parentName = _ecm.Component<components::Name>(
_parent->Data())->Data();
Expand Down
4 changes: 2 additions & 2 deletions src/systems/magnetometer/Magnetometer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ void MagnetometerPrivate::CreateMagnetometerEntities(
data.SetTopic(topic);
}
std::unique_ptr<sensors::MagnetometerSensor> sensor =
this->sensorFactory.CreateSensor<
sensors::MagnetometerSensor>(data);
std::make_unique<sensors::MagnetometerSensor>();
sensor->Load(data);
// set sensor parent
std::string parentName = _ecm.Component<components::Name>(
_parent->Data())->Data();
Expand Down
6 changes: 2 additions & 4 deletions src/systems/sensors/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ gz_add_system(sensors
PUBLIC_LINK_LIBS
ignition-common${IGN_COMMON_VER}::ignition-common${IGN_COMMON_VER}
ignition-sensors${IGN_SENSORS_VER}::ignition-sensors${IGN_SENSORS_VER}
${PROJECT_LIBRARY_TARGET_NAME}-rendering
PRIVATE_LINK_LIBS
ignition-sensors${IGN_SENSORS_VER}::camera
ignition-sensors${IGN_SENSORS_VER}::gpu_lidar
ignition-sensors${IGN_SENSORS_VER}::depth_camera
ignition-sensors${IGN_SENSORS_VER}::thermal_camera
${PROJECT_LIBRARY_TARGET_NAME}-rendering
)

21 changes: 20 additions & 1 deletion src/systems/sensors/Sensors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,26 @@ std::string Sensors::CreateSensor(const Entity &_entity,
}

// Create within ign-sensors
auto sensorId = this->dataPtr->sensorManager.CreateSensor(_sdf);
ignition::sensors::SensorId sensorId;
if (_sdf.Type() == sdf::SensorType::CAMERA)
{
std::unique_ptr<sensors::CameraSensor> cameraSensor =
std::make_unique<sensors::CameraSensor>();
cameraSensor->Load(_sdf);
sensorId = this->dataPtr->sensorManager.AddSensor(std::move(cameraSensor));
}
else if (_sdf.Type() == sdf::SensorType::THERMAL_CAMERA)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you also need to do the same for depth and rgbd camera? or do they work with CreateSensor?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

depth and rgbd camera classes are not being used, we don't need to link against them, we can use createSensor. If need to modify some of the specific parameters, then we need to instanciate a class and create a specific case for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can try sensors_demos.sdf which is creating: camera, depth_camera, gpu_lidar, rgbd_camera and thermal_camera

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is now targeting Edifice, I think lines 485 - 504 can be replaced with:

ignition::sensors::SensorId sensorId = this->dataPtr->sensorManager.CreateSensor(_sdf);

At least, this worked for me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liberty of applying this change in 2f674e6. Feel free to revert.

{
std::unique_ptr<sensors::ThermalCameraSensor> thermalCameraSensor =
std::make_unique<sensors::ThermalCameraSensor>();
thermalCameraSensor->Load(_sdf);
sensorId = this->dataPtr->sensorManager.AddSensor(
std::move(thermalCameraSensor));
}
else
{
sensorId = this->dataPtr->sensorManager.CreateSensor(_sdf);
}
auto sensor = this->dataPtr->sensorManager.Sensor(sensorId);

// Add to sensorID -> entity map
Expand Down