sensormanager: use bus reference_wrapper over pointer
Switch the SensorManager class to use a `reference_wrapper` to the
sdbusplus connection rather than a raw pointer for code safety.
This has a side-effect of disabling the default constructor for the
class, which implicates using wrappers like `std::optional`.
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I799d4564f0dbd9698951f0ec8e86a7272299ebf6
diff --git a/main.cpp b/main.cpp
index d823966..4539d83 100644
--- a/main.cpp
+++ b/main.cpp
@@ -45,6 +45,7 @@
#include <list>
#include <map>
#include <memory>
+#include <optional>
#include <unordered_map>
#include <utility>
#include <vector>
@@ -68,7 +69,7 @@
/* The timers used by the PID loop */
static std::vector<std::shared_ptr<boost::asio::steady_timer>> timers;
/* The sensors build from configuration */
-static SensorManager mgmr;
+static std::optional<SensorManager> mgmr;
} // namespace state
} // namespace pid_control
@@ -162,7 +163,7 @@
state::mgmr = buildSensors(sensorConfig, passiveBus, hostBus);
state::zones =
- buildZones(zoneConfig, zoneDetailsConfig, state::mgmr, modeControlBus);
+ buildZones(zoneConfig, zoneDetailsConfig, *state::mgmr, modeControlBus);
// Set `logMaxCountPerSecond` to 20 will limit the number of logs output per
// second in each zone. Using 20 here would limit the output rate to be no
// larger than 100 per sec for most platforms as the number of zones are
diff --git a/sensors/builder.cpp b/sensors/builder.cpp
index 166977b..1c63623 100644
--- a/sensors/builder.cpp
+++ b/sensors/builder.cpp
@@ -46,7 +46,7 @@
const std::map<std::string, conf::SensorConfig>& config,
sdbusplus::bus_t& passive, sdbusplus::bus_t& host)
{
- SensorManager mgmr(passive, host);
+ SensorManager mgmr{passive, host};
auto& hostSensorBus = mgmr.getHostBus();
auto& passiveListeningBus = mgmr.getPassiveBus();
diff --git a/sensors/manager.hpp b/sensors/manager.hpp
index e02e1c9..6ae39fd 100644
--- a/sensors/manager.hpp
+++ b/sensors/manager.hpp
@@ -20,13 +20,13 @@
{
public:
SensorManager(sdbusplus::bus_t& pass, sdbusplus::bus_t& host) :
- _passiveListeningBus(&pass), _hostSensorBus(&host)
+ _passiveListeningBus(pass), _hostSensorBus(host)
{
// manager gets its interface from the bus. :D
- sdbusplus::server::manager_t(*_hostSensorBus, SensorRoot);
+ sdbusplus::server::manager_t(_hostSensorBus, SensorRoot);
}
- SensorManager() = default;
+ SensorManager() = delete;
~SensorManager() = default;
SensorManager(const SensorManager&) = delete;
SensorManager& operator=(const SensorManager&) = delete;
@@ -47,20 +47,20 @@
sdbusplus::bus_t& getPassiveBus(void)
{
- return *_passiveListeningBus;
+ return _passiveListeningBus;
}
sdbusplus::bus_t& getHostBus(void)
{
- return *_hostSensorBus;
+ return _hostSensorBus;
}
private:
std::map<std::string, std::unique_ptr<Sensor>> _sensorMap;
std::map<std::string, std::vector<std::string>> _sensorTypeList;
- sdbusplus::bus_t* _passiveListeningBus;
- sdbusplus::bus_t* _hostSensorBus;
+ std::reference_wrapper<sdbusplus::bus_t> _passiveListeningBus;
+ std::reference_wrapper<sdbusplus::bus_t> _hostSensorBus;
static constexpr auto SensorRoot = "/xyz/openbmc_project/extsensors";
};
diff --git a/test/pid_zone_unittest.cpp b/test/pid_zone_unittest.cpp
index ea8ac91..e4e7a26 100644
--- a/test/pid_zone_unittest.cpp
+++ b/test/pid_zone_unittest.cpp
@@ -13,6 +13,7 @@
#include <chrono>
#include <cstring>
+#include <optional>
#include <unordered_map>
#include <vector>
@@ -104,9 +105,7 @@
auto bus_mock_mode = sdbusplus::get_mocked_new(&sdbus_mock_mode);
auto bus_mock_enable = sdbusplus::get_mocked_new(&sdbus_mock_enable);
- // Compiler weirdly not happy about just instantiating mgr(...);
- SensorManager m(bus_mock_passive, bus_mock_host);
- mgr = std::move(m);
+ mgr = SensorManager(bus_mock_passive, bus_mock_host);
SetupDbusObject(&sdbus_mock_mode, defer, objPath, modeInterface,
properties, &property_index);
@@ -118,7 +117,7 @@
&propertyenable_index);
zone = std::make_unique<DbusPidZone>(
- zoneId, minThermalOutput, failSafePercent, cycleTime, mgr,
+ zoneId, minThermalOutput, failSafePercent, cycleTime, *mgr,
bus_mock_mode, objPath, defer, accSetPoint);
}
@@ -139,7 +138,7 @@
bool defer = true;
bool accSetPoint = false;
const char* objPath = "/path/";
- SensorManager mgr;
+ std::optional<SensorManager> mgr;
conf::CycleTime cycleTime;
std::string sensorname = "temp1";
@@ -381,10 +380,10 @@
SensorMock* sensor_ptr2 = reinterpret_cast<SensorMock*>(sensor2.get());
std::string type = "unchecked";
- mgr.addSensor(type, name1, std::move(sensor1));
- EXPECT_EQ(mgr.getSensor(name1), sensor_ptr1);
- mgr.addSensor(type, name2, std::move(sensor2));
- EXPECT_EQ(mgr.getSensor(name2), sensor_ptr2);
+ mgr->addSensor(type, name1, std::move(sensor1));
+ EXPECT_EQ(mgr->getSensor(name1), sensor_ptr1);
+ mgr->addSensor(type, name2, std::move(sensor2));
+ EXPECT_EQ(mgr->getSensor(name2), sensor_ptr2);
// Now that the sensors exist, add them to the zone.
zone->addThermalInput(name1, false);
@@ -437,10 +436,10 @@
SensorMock* sensor_ptr2 = reinterpret_cast<SensorMock*>(sensor2.get());
std::string type = "unchecked";
- mgr.addSensor(type, name1, std::move(sensor1));
- EXPECT_EQ(mgr.getSensor(name1), sensor_ptr1);
- mgr.addSensor(type, name2, std::move(sensor2));
- EXPECT_EQ(mgr.getSensor(name2), sensor_ptr2);
+ mgr->addSensor(type, name1, std::move(sensor1));
+ EXPECT_EQ(mgr->getSensor(name1), sensor_ptr1);
+ mgr->addSensor(type, name2, std::move(sensor2));
+ EXPECT_EQ(mgr->getSensor(name2), sensor_ptr2);
// Now that the sensors exist, add them to the zone.
zone->addFanInput(name1, false);
@@ -489,10 +488,10 @@
SensorMock* sensor_ptr2 = reinterpret_cast<SensorMock*>(sensor2.get());
std::string type = "unchecked";
- mgr.addSensor(type, name1, std::move(sensor1));
- EXPECT_EQ(mgr.getSensor(name1), sensor_ptr1);
- mgr.addSensor(type, name2, std::move(sensor2));
- EXPECT_EQ(mgr.getSensor(name2), sensor_ptr2);
+ mgr->addSensor(type, name1, std::move(sensor1));
+ EXPECT_EQ(mgr->getSensor(name1), sensor_ptr1);
+ mgr->addSensor(type, name2, std::move(sensor2));
+ EXPECT_EQ(mgr->getSensor(name2), sensor_ptr2);
zone->addThermalInput(name1, false);
zone->addThermalInput(name2, false);
@@ -555,10 +554,10 @@
SensorMock* sensor_ptr2 = reinterpret_cast<SensorMock*>(sensor2.get());
std::string type = "unchecked";
- mgr.addSensor(type, name1, std::move(sensor1));
- EXPECT_EQ(mgr.getSensor(name1), sensor_ptr1);
- mgr.addSensor(type, name2, std::move(sensor2));
- EXPECT_EQ(mgr.getSensor(name2), sensor_ptr2);
+ mgr->addSensor(type, name1, std::move(sensor1));
+ EXPECT_EQ(mgr->getSensor(name1), sensor_ptr1);
+ mgr->addSensor(type, name2, std::move(sensor2));
+ EXPECT_EQ(mgr->getSensor(name2), sensor_ptr2);
// Only sensor1 has MissingIsAcceptable enabled for it
zone->addThermalInput(name1, true);
@@ -655,10 +654,10 @@
SensorMock* sensor_ptr2 = reinterpret_cast<SensorMock*>(sensor2.get());
std::string type = "unchecked";
- mgr.addSensor(type, name1, std::move(sensor1));
- EXPECT_EQ(mgr.getSensor(name1), sensor_ptr1);
- mgr.addSensor(type, name2, std::move(sensor2));
- EXPECT_EQ(mgr.getSensor(name2), sensor_ptr2);
+ mgr->addSensor(type, name1, std::move(sensor1));
+ EXPECT_EQ(mgr->getSensor(name1), sensor_ptr1);
+ mgr->addSensor(type, name2, std::move(sensor2));
+ EXPECT_EQ(mgr->getSensor(name2), sensor_ptr2);
// Now that the sensors exist, add them to the zone.
zone->addFanInput(name1, false);
@@ -712,10 +711,10 @@
SensorMock* sensor_ptr2 = reinterpret_cast<SensorMock*>(sensor2.get());
std::string type = "unchecked";
- mgr.addSensor(type, name1, std::move(sensor1));
- EXPECT_EQ(mgr.getSensor(name1), sensor_ptr1);
- mgr.addSensor(type, name2, std::move(sensor2));
- EXPECT_EQ(mgr.getSensor(name2), sensor_ptr2);
+ mgr->addSensor(type, name1, std::move(sensor1));
+ EXPECT_EQ(mgr->getSensor(name1), sensor_ptr1);
+ mgr->addSensor(type, name2, std::move(sensor2));
+ EXPECT_EQ(mgr->getSensor(name2), sensor_ptr2);
// Now that the sensors exist, add them to the zone.
zone->addFanInput(name1, false);
@@ -770,13 +769,13 @@
SensorMock* sensor_ptr1 = reinterpret_cast<SensorMock*>(sensor1.get());
std::string type = "unchecked";
- mgr.addSensor(type, name1, std::move(sensor1));
- EXPECT_EQ(mgr.getSensor(name1), sensor_ptr1);
+ mgr->addSensor(type, name1, std::move(sensor1));
+ EXPECT_EQ(mgr->getSensor(name1), sensor_ptr1);
zone->addThermalInput(name1, false);
// Verify method under test returns the pointer we expect.
- EXPECT_EQ(mgr.getSensor(name1), zone->getSensor(name1));
+ EXPECT_EQ(mgr->getSensor(name1), zone->getSensor(name1));
}
TEST_F(PidZoneTest, AddThermalPIDTest_VerifiesThermalPIDsProcessed)