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>

9 files changed, 318 insertions, 28 deletions

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 @@ uint64_t FanPwm::target(uint64_t value)
         std::string empty;
         //Write target out to sysfs
         try {
-            ioAccess.write(
+            ioAccess->write(
                 value,
                 type,
                 id,
@@ -45,7 +45,7 @@ uint64_t FanPwm::target(uint64_t value)
                         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 @@ class FanPwm : public FanPwmObject
         /**
          * @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 @@ class FanPwm : public FanPwmObject
            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 @@ class FanPwm : public FanPwmObject
         /** @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 @@ uint64_t FanSpeed::target(uint64_t value)
         //Write target out to sysfs
         try
         {
-            ioAccess.write(
+            ioAccess->write(
                     value,
                     type,
                     id,
@@ -41,7 +41,7 @@ uint64_t FanSpeed::target(uint64_t value)
                         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 @@ void FanSpeed::enable()
 
         try
         {
-            ioAccess.write(
+            ioAccess->write(
                     val,
                     type::pwm,
                     id,
@@ -85,7 +85,7 @@ void FanSpeed::enable()
                         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 @@ class FanSpeed : public FanSpeedObject
         /**
          * @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 @@ class FanSpeed : public FanSpeedObject
          * @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 @@ class FanSpeed : public FanSpeedObject
                  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 @@ class FanSpeed : public FanSpeedObject
         /** @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 @@ namespace hwmonio {
 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 @@ static constexpr auto delay = std::chrono::milliseconds{100};
  *  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 @@ class HwmonIO
                 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 @@ class HwmonIO
                 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 @@ std::shared_ptr<T> addTarget(const SensorSet::key_type& sensor,
                                 "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 @@ AM_LDFLAGS = $(GTEST_LIBS) \
 	$(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