hwmonio:: Add Interface base class and tests
Enable injecting hwmonio::HwmonIO mocks for testing.
Tested: Ran on quanta-q71l and saw all sensors exported to dbus as
expected with the expected values.
Change-Id: I35912bf2a733932d9e1e774ff53b0114ae16560b
Signed-off-by: Patrick Venture <venture@google.com>
diff --git a/fan_pwm.cpp b/fan_pwm.cpp
index cf89552..9b43b17 100644
--- a/fan_pwm.cpp
+++ b/fan_pwm.cpp
@@ -26,7 +26,7 @@
std::string empty;
//Write target out to sysfs
try {
- ioAccess.write(
+ ioAccess->write(
value,
type,
id,
@@ -45,7 +45,7 @@
WriteFailure::CALLOUT_DEVICE_PATH(devPath.c_str()));
auto file = sysfs::make_sysfs_path(
- ioAccess.path(),
+ ioAccess->path(),
type,
id,
empty);
diff --git a/fan_pwm.hpp b/fan_pwm.hpp
index 20baf38..61c0bf6 100644
--- a/fan_pwm.hpp
+++ b/fan_pwm.hpp
@@ -1,5 +1,7 @@
#pragma once
+#include <memory>
+
#include "hwmonio.hpp"
#include "interface.hpp"
#include "sysfs.hpp"
@@ -20,15 +22,14 @@
/**
* @brief Constructs FanPwm Object
*
- * @param[in] instancePath - The hwmon instance path
- * (ex. /sys/class/hwmon/hwmon1)
+ * @param[in] io - HwmonIO
* @param[in] devPath - The /sys/devices sysfs path
* @param[in] id - The hwmon id
* @param[in] bus - Dbus bus object
* @param[in] objPath - Dbus object path
* @param[in] defer - Dbus object registration defer
*/
- FanPwm(const std::string& instancePath,
+ FanPwm(std::unique_ptr<hwmonio::HwmonIOInterface> io,
const std::string& devPath,
const std::string& id,
sdbusplus::bus::bus& bus,
@@ -36,7 +37,7 @@
bool defer,
uint64_t target) : FanPwmObject(bus, objPath, defer),
id(id),
- ioAccess(instancePath),
+ ioAccess(std::move(io)),
devPath(devPath)
{
FanPwmObject::target(target);
@@ -55,7 +56,7 @@
/** @brief hwmon id */
std::string id;
/** @brief Hwmon sysfs access. */
- hwmonio::HwmonIO ioAccess;
+ std::unique_ptr<hwmonio::HwmonIOInterface> ioAccess;
/** @brief Physical device path. */
std::string devPath;
};
diff --git a/fan_speed.cpp b/fan_speed.cpp
index f1afd55..9e297bc 100644
--- a/fan_speed.cpp
+++ b/fan_speed.cpp
@@ -21,7 +21,7 @@
//Write target out to sysfs
try
{
- ioAccess.write(
+ ioAccess->write(
value,
type,
id,
@@ -41,7 +41,7 @@
WriteFailure::CALLOUT_DEVICE_PATH(devPath.c_str()));
auto file = sysfs::make_sysfs_path(
- ioAccess.path(),
+ ioAccess->path(),
type,
id,
entry::target);
@@ -66,7 +66,7 @@
try
{
- ioAccess.write(
+ ioAccess->write(
val,
type::pwm,
id,
@@ -85,7 +85,7 @@
WriteFailure::CALLOUT_DEVICE_PATH(devPath.c_str()));
auto fullPath = sysfs::make_sysfs_path(
- ioAccess.path(),
+ ioAccess->path(),
type::pwm,
id,
entry::enable);
diff --git a/fan_speed.hpp b/fan_speed.hpp
index fb97303..9b4ad63 100644
--- a/fan_speed.hpp
+++ b/fan_speed.hpp
@@ -1,5 +1,7 @@
#pragma once
+#include <memory>
+
#include "hwmonio.hpp"
#include "interface.hpp"
#include "sysfs.hpp"
@@ -20,8 +22,7 @@
/**
* @brief Constructs FanSpeed Object
*
- * @param[in] instancePath - The hwmon instance path
- * (ex. /sys/class/hwmon/hwmon1)
+ * @param[in] io - HwmonIO(instance path) (ex /sys/class/hwmon/hwmon1)
* @param[in] devPath - The /sys/devices sysfs path
* @param[in] id - The hwmon id
* @param[in] bus - Dbus bus object
@@ -29,7 +30,7 @@
* @param[in] defer - Dbus object registration defer
* @param[in] target - initial target speed value
*/
- FanSpeed(const std::string& instancePath,
+ FanSpeed(std::unique_ptr<hwmonio::HwmonIOInterface> io,
const std::string& devPath,
const std::string& id,
sdbusplus::bus::bus& bus,
@@ -37,7 +38,7 @@
bool defer,
uint64_t target) : FanSpeedObject(bus, objPath, defer),
id(id),
- ioAccess(instancePath),
+ ioAccess(std::move(io)),
devPath(devPath)
{
FanSpeedObject::target(target);
@@ -62,7 +63,7 @@
/** @brief hwmon id */
std::string id;
/** @brief Hwmon sysfs access. */
- hwmonio::HwmonIO ioAccess;
+ std::unique_ptr<hwmonio::HwmonIOInterface> ioAccess;
/** @brief Physical device path. */
std::string devPath;
diff --git a/hwmonio.hpp b/hwmonio.hpp
index 619d6ad..d49f4db 100644
--- a/hwmonio.hpp
+++ b/hwmonio.hpp
@@ -8,6 +8,35 @@
static constexpr auto retries = 10;
static constexpr auto delay = std::chrono::milliseconds{100};
+/** @class HwmonIOInterface
+ * @brief Abstract base class defining a HwmonIOInterface.
+ *
+ * This is initially to provide easier testing through injection,
+ * however, could in theory support non-sysfs handling of hwmon IO.
+ */
+class HwmonIOInterface
+{
+ public:
+ virtual ~HwmonIOInterface() = default;
+
+ virtual int64_t read(
+ const std::string& type,
+ const std::string& id,
+ const std::string& sensor,
+ size_t retries,
+ std::chrono::milliseconds delay) const = 0;
+
+ virtual void write(
+ uint32_t val,
+ const std::string& type,
+ const std::string& id,
+ const std::string& sensor,
+ size_t retries,
+ std::chrono::milliseconds delay) const = 0;
+
+ virtual std::string path() const = 0;
+};
+
/** @class HwmonIO
* @brief Convenience wrappers for HWMON sysfs attribute IO.
*
@@ -17,7 +46,7 @@
* cannot always be terminated externally before we try to
* do an io.
*/
-class HwmonIO
+class HwmonIO : public HwmonIOInterface
{
public:
HwmonIO() = delete;
@@ -57,7 +86,7 @@
const std::string& id,
const std::string& sensor,
size_t retries,
- std::chrono::milliseconds delay) const;
+ std::chrono::milliseconds delay) const override;
/** @brief Perform formatted hwmon sysfs write.
*
@@ -81,14 +110,14 @@
const std::string& id,
const std::string& sensor,
size_t retries,
- std::chrono::milliseconds delay) const;
+ std::chrono::milliseconds delay) const override;
/** @brief Hwmon instance path access.
*
* @return path - The hwmon instance path.
*/
- std::string path() const;
+ std::string path() const override;
private:
std::string p;
diff --git a/targets.hpp b/targets.hpp
index cb25dee..9867ba9 100644
--- a/targets.hpp
+++ b/targets.hpp
@@ -1,6 +1,7 @@
#pragma once
#include <experimental/filesystem>
+#include <memory>
#include <phosphor-logging/elog-errors.hpp>
#include <phosphor-logging/log.hpp>
#include <xyz/openbmc_project/Sensor/Device/error.hpp>
@@ -165,13 +166,16 @@
"FILE=%s", sysfsFullPath.c_str()));
}
- target = std::make_shared<T>(ioAccess.path(),
- devPath,
- targetId,
- bus,
- objPath.c_str(),
- deferSignals,
- targetSpeed);
+ // ioAccess.path() is a path like: /sys/class/hwmon/hwmon1
+ target = std::make_shared<T>(
+ std::move(std::make_unique<hwmonio::HwmonIO>(
+ ioAccess.path())),
+ devPath,
+ targetId,
+ bus,
+ objPath.c_str(),
+ deferSignals,
+ targetSpeed);
obj[type] = target;
}
}
diff --git a/test/Makefile.am b/test/Makefile.am
index 7f302ca..1b07171 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -12,9 +12,11 @@
$(PHOSPHOR_DBUS_INTERFACES_LIBS)
# Run all 'check' test programs
-check_PROGRAMS = hwmon_unittest
+check_PROGRAMS = hwmon_unittest fanpwm_unittest
TESTS = $(check_PROGRAMS)
hwmon_unittest_SOURCES = hwmon_unittest.cpp
hwmon_unittest_LDADD = $(top_builddir)/hwmon.o
+fanpwm_unittest_SOURCES = fanpwm_unittest.cpp
+fanpwm_unittest_LDADD = $(PHOSPHOR_LOGGING_LIBS) $(top_builddir)/fan_pwm.o
diff --git a/test/fanpwm_unittest.cpp b/test/fanpwm_unittest.cpp
new file mode 100644
index 0000000..63520d3
--- /dev/null
+++ b/test/fanpwm_unittest.cpp
@@ -0,0 +1,221 @@
+#include "fan_pwm.hpp"
+
+#include "hwmonio_mock.hpp"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <sdbusplus/test/sdbus_mock.hpp>
+#include <string>
+
+using ::testing::_;
+using ::testing::Invoke;
+using ::testing::IsNull;
+using ::testing::NotNull;
+using ::testing::Return;
+using ::testing::StrEq;
+
+static auto FanPwmIntf = "xyz.openbmc_project.Control.FanPwm";
+static auto FanPwmProp = "Target";
+
+// Handle basic expectations we'll need for all these tests, if it's found that
+// this is helpful for more tests, it can be promoted in scope.
+void SetupDbusObject(
+ sdbusplus::SdBusMock *sdbus_mock,
+ const std::string& path,
+ const std::string& intf,
+ const std::string property = "")
+{
+ EXPECT_CALL(*sdbus_mock,
+ sd_bus_add_object_vtable(
+ IsNull(),
+ NotNull(),
+ StrEq(path),
+ StrEq(intf),
+ NotNull(),
+ NotNull()))
+ .WillOnce(Return(0));
+
+ if (property.empty())
+ {
+ EXPECT_CALL(*sdbus_mock,
+ sd_bus_emit_properties_changed_strv(
+ IsNull(),
+ StrEq(path),
+ StrEq(intf),
+ NotNull()))
+ .WillOnce(Return(0));
+ }
+ else
+ {
+ EXPECT_CALL(*sdbus_mock,
+ sd_bus_emit_properties_changed_strv(
+ IsNull(),
+ StrEq(path),
+ StrEq(intf),
+ NotNull()))
+ .WillOnce(
+ Invoke([=](sd_bus *bus,
+ const char *path,
+ const char *interface,
+ char **names) {
+ EXPECT_STREQ(property.c_str(), names[0]);
+ return 0;
+ })
+ );
+ }
+
+ return;
+}
+
+TEST(FanPwmTest, BasicConstructorDeferredTest) {
+ // Attempt to just instantiate one.
+
+ // NOTE: This test's goal is to figure out what's minimally required to
+ // mock to instantiate this object.
+ sdbusplus::SdBusMock sdbus_mock;
+ auto bus_mock = sdbusplus::get_mocked_new(&sdbus_mock);
+
+ std::string instancePath = "";
+ std::string devPath = "";
+ std::string id = "";
+ std::string objPath = "asdf";
+ bool defer = true;
+ uint64_t target = 0x01;
+
+ std::unique_ptr<hwmonio::HwmonIOInterface> hwmonio_mock =
+ std::make_unique<hwmonio::HwmonIOMock>();
+
+ SetupDbusObject(&sdbus_mock, objPath, FanPwmIntf, FanPwmProp);
+
+ hwmon::FanPwm f(std::move(hwmonio_mock),
+ devPath,
+ id,
+ bus_mock,
+ objPath.c_str(),
+ defer,
+ target);
+}
+
+TEST(FanPwmTest, BasicConstructorNotDeferredTest) {
+ // Attempt to just instantiate one.
+
+ // NOTE: This test's goal is to figure out what's minimally required to
+ // mock to instantiate this object.
+ sdbusplus::SdBusMock sdbus_mock;
+ auto bus_mock = sdbusplus::get_mocked_new(&sdbus_mock);
+
+ std::string instancePath = "";
+ std::string devPath = "";
+ std::string id = "";
+ std::string objPath = "asdf";
+ bool defer = false;
+ uint64_t target = 0x01;
+
+ std::unique_ptr<hwmonio::HwmonIOInterface> hwmonio_mock =
+ std::make_unique<hwmonio::HwmonIOMock>();
+
+ SetupDbusObject(&sdbus_mock, objPath, FanPwmIntf, FanPwmProp);
+
+ EXPECT_CALL(sdbus_mock,
+ sd_bus_emit_object_added(IsNull(), StrEq("asdf")))
+ .WillOnce(Return(0));
+
+ EXPECT_CALL(sdbus_mock,
+ sd_bus_emit_object_removed(IsNull(), StrEq("asdf")))
+ .WillOnce(Return(0));
+
+ hwmon::FanPwm f(std::move(hwmonio_mock),
+ devPath,
+ id,
+ bus_mock,
+ objPath.c_str(),
+ defer,
+ target);
+}
+
+TEST(FanPwmTest, WriteTargetValue) {
+ // Create a FanPwm and write a value to the object.
+
+ sdbusplus::SdBusMock sdbus_mock;
+ auto bus_mock = sdbusplus::get_mocked_new(&sdbus_mock);
+
+ std::string instancePath = "";
+ std::string devPath = "devp";
+ std::string id = "the_id";
+ std::string objPath = "asdf";
+ bool defer = true;
+ uint64_t target = 0x01;
+
+ std::unique_ptr<hwmonio::HwmonIOInterface> hwmonio_mock =
+ std::make_unique<hwmonio::HwmonIOMock>();
+
+ SetupDbusObject(&sdbus_mock, objPath, FanPwmIntf, FanPwmProp);
+
+ hwmonio::HwmonIOMock *hwmonio =
+ reinterpret_cast<hwmonio::HwmonIOMock *>(hwmonio_mock.get());
+
+ hwmon::FanPwm f(std::move(hwmonio_mock),
+ devPath,
+ id,
+ bus_mock,
+ objPath.c_str(),
+ defer,
+ target);
+
+ target = 0x64;
+
+ EXPECT_CALL(*hwmonio, write(static_cast<uint32_t>(target),
+ StrEq("pwm"),
+ StrEq("the_id"),
+ _,
+ hwmonio::retries,
+ hwmonio::delay));
+
+ EXPECT_CALL(sdbus_mock,
+ sd_bus_emit_properties_changed_strv(
+ IsNull(),
+ StrEq("asdf"),
+ StrEq(FanPwmIntf),
+ NotNull()))
+ .WillOnce(
+ Invoke([&](sd_bus *bus,
+ const char *path,
+ const char *interface,
+ char **names) {
+ EXPECT_EQ(0, strncmp("Target", names[0], 6));
+ return 0;
+ })
+ );
+
+ EXPECT_EQ(target, f.target(target));
+}
+
+TEST(FanPwmTest, WriteTargetValueNoUpdate) {
+ // Create a FanPwm and write a value to the object that was the previous
+ // value.
+
+ sdbusplus::SdBusMock sdbus_mock;
+ auto bus_mock = sdbusplus::get_mocked_new(&sdbus_mock);
+
+ std::string instancePath = "";
+ std::string devPath = "devp";
+ std::string id = "the_id";
+ std::string objPath = "asdf";
+ bool defer = true;
+ uint64_t target = 0x01;
+
+ std::unique_ptr<hwmonio::HwmonIOInterface> hwmonio_mock =
+ std::make_unique<hwmonio::HwmonIOMock>();
+
+ SetupDbusObject(&sdbus_mock, objPath, FanPwmIntf, FanPwmProp);
+
+ hwmon::FanPwm f(std::move(hwmonio_mock),
+ devPath,
+ id,
+ bus_mock,
+ objPath.c_str(),
+ defer,
+ target);
+
+ EXPECT_EQ(target, f.target(target));
+}
diff --git a/test/hwmonio_mock.hpp b/test/hwmonio_mock.hpp
new file mode 100644
index 0000000..4ad944c
--- /dev/null
+++ b/test/hwmonio_mock.hpp
@@ -0,0 +1,32 @@
+#pragma once
+
+#include <gmock/gmock.h>
+
+#include "hwmonio.hpp"
+
+namespace hwmonio {
+
+class HwmonIOMock : public HwmonIOInterface
+{
+ public:
+ virtual ~HwmonIOMock(){};
+
+ MOCK_CONST_METHOD5(read, int64_t(const std::string&,
+ const std::string&,
+ const std::string&,
+ size_t,
+ std::chrono::milliseconds));
+
+ MOCK_CONST_METHOD6(write, void(uint32_t,
+ const std::string&,
+ const std::string&,
+ const std::string&,
+ size_t,
+ std::chrono::milliseconds));
+
+ MOCK_CONST_METHOD0(path, std::string());
+};
+
+} // namespace hwmonio
+
+// vim: tabstop=8 expandtab shiftwidth=4 softtabstop=4