dbus: dbuswrite: use factory to catch errors
Add factories for creating DbusWrite and DbusWritePercent objects and
use these factories.
Change-Id: Iad222d4bf6c1fdf561daa4ba9d88a6644c0a5965
Signed-off-by: Patrick Venture <venture@google.com>
diff --git a/dbus/dbuswrite.cpp b/dbus/dbuswrite.cpp
index 714580b..b691b66 100644
--- a/dbus/dbuswrite.cpp
+++ b/dbus/dbuswrite.cpp
@@ -17,13 +17,36 @@
#include "dbus/dbuswrite.hpp"
#include <iostream>
+#include <memory>
#include <sdbusplus/bus.hpp>
+#include <string>
+
+constexpr const char* pwmInterface = "xyz.openbmc_project.Control.FanPwm";
// this bus object is treated as a singleton because the class is constructed in
// a different thread than it is used, and as bus objects are relatively
// expensive we'd prefer to only have one
std::unique_ptr<sdbusplus::bus::bus> writeBus = nullptr;
+std::unique_ptr<WriteInterface>
+ DbusWritePercent::createDbusWrite(const std::string& path, int64_t min,
+ int64_t max, DbusHelperInterface& helper)
+{
+ auto tempBus = sdbusplus::bus::new_default();
+ std::string connectionName;
+
+ try
+ {
+ connectionName = helper.getService(tempBus, pwmInterface, path);
+ }
+ catch (const std::exception& e)
+ {
+ return nullptr;
+ }
+
+ return std::make_unique<DbusWritePercent>(path, min, max, connectionName);
+}
+
void initBus()
{
if (writeBus == nullptr)
@@ -61,6 +84,25 @@
return;
}
+std::unique_ptr<WriteInterface>
+ DbusWrite::createDbusWrite(const std::string& path, int64_t min,
+ int64_t max, DbusHelperInterface& helper)
+{
+ auto tempBus = sdbusplus::bus::new_default();
+ std::string connectionName;
+
+ try
+ {
+ connectionName = helper.getService(tempBus, pwmInterface, path);
+ }
+ catch (const std::exception& e)
+ {
+ return nullptr;
+ }
+
+ return std::make_unique<DbusWrite>(path, min, max, connectionName);
+}
+
void DbusWrite::write(double value)
{
if (oldValue == static_cast<int64_t>(value))
diff --git a/dbus/dbuswrite.hpp b/dbus/dbuswrite.hpp
index 577013d..6f5b85b 100644
--- a/dbus/dbuswrite.hpp
+++ b/dbus/dbuswrite.hpp
@@ -19,22 +19,22 @@
#include "dbus/util.hpp"
#include "interfaces.hpp"
+#include <memory>
#include <sdbusplus/bus.hpp>
#include <string>
-constexpr const char* pwmInterface = "xyz.openbmc_project.Control.FanPwm";
-
class DbusWritePercent : public WriteInterface
{
public:
+ static std::unique_ptr<WriteInterface>
+ createDbusWrite(const std::string& path, int64_t min, int64_t max,
+ DbusHelperInterface& helper);
+
DbusWritePercent(const std::string& path, int64_t min, int64_t max,
- DbusHelperInterface& helper) :
+ const std::string& connectionName) :
WriteInterface(min, max),
- path(path)
+ path(path), connectionName(connectionName)
{
- auto tempBus = sdbusplus::bus::new_default();
- // getService can except, does it make more sense to use a factory?
- connectionName = helper.getService(tempBus, pwmInterface, path);
}
void write(double value) override;
@@ -48,14 +48,15 @@
class DbusWrite : public WriteInterface
{
public:
+ static std::unique_ptr<WriteInterface>
+ createDbusWrite(const std::string& path, int64_t min, int64_t max,
+ DbusHelperInterface& helper);
+
DbusWrite(const std::string& path, int64_t min, int64_t max,
- DbusHelperInterface& helper) :
+ const std::string& connectionName) :
WriteInterface(min, max),
- path(path)
+ path(path), connectionName(connectionName)
{
- auto tempBus = sdbusplus::bus::new_default();
- // getService can except, does it make more sense to use a factory?
- connectionName = helper.getService(tempBus, pwmInterface, path);
}
void write(double value) override;
diff --git a/sensors/builder.cpp b/sensors/builder.cpp
index 15d479c..0f2f0c7 100644
--- a/sensors/builder.cpp
+++ b/sensors/builder.cpp
@@ -101,13 +101,15 @@
case IOInterfaceType::DBUSACTIVE:
if (info->max > 0)
{
- wi = std::make_unique<DbusWritePercent>(
+ wi = DbusWritePercent::createDbusWrite(
info->writepath, info->min, info->max, helper);
+ /* TODO: handle this being nullptr. */
}
else
{
- wi = std::make_unique<DbusWrite>(
+ wi = DbusWrite::createDbusWrite(
info->writepath, info->min, info->max, helper);
+ /* TODO: handle this being nullptr. */
}
break;