sensor: Replace GpioLock with stdplus RAII helper
The old RAII helper was not move safe, although it was never moved in
the current code so it didn't have any effect on runtime safety.
Change-Id: Ica19ed7e60d699d86d0166b356cedb82e4a28b61
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/Makefile.am b/Makefile.am
index 98b73bd..2efbe77 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -47,13 +47,14 @@
$(PHOSPHOR_DBUS_INTERFACES_LIBS) \
$(PHOSPHOR_LOGGING_LIBS) \
$(GPIOPLUS_LIBS) \
+ $(STDPLUS_LIBS) \
$(CODE_COVERAGE_LIBS)
libhwmon_la_CXXFLAGS = \
$(SDBUSPLUS_CFLAGS) \
$(SDEVENTPLUS_CFLAGS) \
$(PHOSPHOR_DBUS_INTERFACES_CFLAGS) \
$(PHOSPHOR_LOGGING_CFLAGS) \
- $(GPIOPLUS_CFLAGS) \
+ $(STDPLUS_CFLAGS) \
$(CODE_COVERAGE_CXXFLAGS)
libhwmon_la_SOURCES = \
diff --git a/configure.ac b/configure.ac
index 289f34a..977594d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -31,6 +31,7 @@
PKG_CHECK_MODULES([PHOSPHOR_DBUS_INTERFACES], [phosphor-dbus-interfaces])
PKG_CHECK_MODULES([PHOSPHOR_LOGGING], [phosphor-logging])
PKG_CHECK_MODULES([GPIOPLUS], [gpioplus])
+PKG_CHECK_MODULES([STDPLUS], [stdplus])
# We need the header only CLI library
AC_CHECK_HEADERS(
[CLI/CLI.hpp],
diff --git a/mainloop.cpp b/mainloop.cpp
index 82cf3aa..05ac623 100644
--- a/mainloop.cpp
+++ b/mainloop.cpp
@@ -427,7 +427,7 @@
{
// RAII object for GPIO unlock / lock
- sensor::GpioLock gpioLock(sensor->getGpio());
+ auto locker = sensor::gpioUnlock(sensor->getGpio());
// Retry for up to a second if device is busy
// or has a transient error.
diff --git a/sensor.cpp b/sensor.cpp
index 6e5e841..145ba6c 100644
--- a/sensor.cpp
+++ b/sensor.cpp
@@ -9,6 +9,7 @@
#include "sysfs.hpp"
#include <cassert>
+#include <chrono>
#include <cmath>
#include <cstring>
#include <filesystem>
@@ -151,7 +152,7 @@
#endif
{
// RAII object for GPIO unlock / lock
- GpioLock gpioLock(getGpio());
+ auto locker = gpioUnlock(getGpio());
// Retry for up to a second if device is busy
// or has a transient error.
@@ -259,31 +260,22 @@
return iface;
}
-GpioLock::GpioLock(const gpioplus::HandleInterface* handle) : _handle(handle)
+void gpioLock(const gpioplus::HandleInterface*&& handle)
{
- unlockGpio();
+ handle->setValues({0});
}
-GpioLock::~GpioLock()
+std::optional<GpioLocker> gpioUnlock(const gpioplus::HandleInterface* handle)
{
- lockGpio();
-}
-
-void GpioLock::unlockGpio()
-{
- if (_handle)
+ if (handle == nullptr)
{
- _handle->setValues({1});
- std::this_thread::sleep_for(_pause);
+ return std::nullopt;
}
-}
-void GpioLock::lockGpio()
-{
- if (_handle)
- {
- _handle->setValues({0});
- }
+ handle->setValues({1});
+ // Default pause needed to guarantee sensors are ready
+ std::this_thread::sleep_for(std::chrono::milliseconds(500));
+ return GpioLocker(std::move(handle));
}
} // namespace sensor
diff --git a/sensor.hpp b/sensor.hpp
index 3ec3dda..4b2d281 100644
--- a/sensor.hpp
+++ b/sensor.hpp
@@ -4,9 +4,10 @@
#include "sensorset.hpp"
#include "types.hpp"
-#include <chrono>
#include <gpioplus/handle.hpp>
#include <memory>
+#include <optional>
+#include <stdplus/handle/managed.hpp>
#include <unordered_set>
namespace sensor
@@ -157,48 +158,23 @@
bool _hasFaultFile;
};
-/** @class GpioLock
- * @brief RAII class for GPIO unlock and lock
- * @details Create this object in the stack to unlock the GPIO. GPIO will
- * be locked by the destructor when we go out of scope.
+/**
+ * @brief Locks the gpio represented by the handle
+ *
+ * @param[in] handle - The gpio handle to lock
*/
-class GpioLock
-{
- public:
- GpioLock() = delete;
- GpioLock(const GpioLock&) = delete;
- GpioLock(GpioLock&&) = default;
- GpioLock& operator=(const GpioLock&) = delete;
- GpioLock& operator=(GpioLock&&) = default;
+void gpioLock(const gpioplus::HandleInterface*&& handle);
- /**
- * @brief Constructs GpioLock RAII object. Unlocks the GPIO.
- *
- * @param[in] handle - Pointer to a GPIO handle interface, can be nullptr.
- */
- explicit GpioLock(const gpioplus::HandleInterface* handle);
+/** @brief The type which is responsible for managing the lock */
+using GpioLocker =
+ stdplus::Managed<const gpioplus::HandleInterface*>::Handle<gpioLock>;
- /**
- * @brief Destructs GpioLock RAII object. Locks the GPIO.
- */
- ~GpioLock();
-
- /**
- * @brief Unlock the gpio, set to high if relevant.
- */
- void unlockGpio();
-
- /**
- * @brief Lock the gpio, set to low if relevant.
- */
- void lockGpio();
-
- private:
- /** @brief Pointer to GPIO handle. */
- const gpioplus::HandleInterface* _handle;
-
- /** @brief default pause after unlocking gpio. */
- static constexpr std::chrono::milliseconds _pause{500};
-};
+/**
+ * @brief Unlocks the gpio and creates a lock object to ensure
+ * the gpio is locked again.
+ *
+ * @param[in] handle - The gpio handle to unlock and wrap
+ */
+std::optional<GpioLocker> gpioUnlock(const gpioplus::HandleInterface* handle);
} // namespace sensor
diff --git a/test/Makefile.am b/test/Makefile.am
index 016409f..a40391a 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -34,10 +34,12 @@
sensor_unittest_CXXFLAGS = \
$(PHOSPHOR_LOGGING_CFLAGS) \
$(GPIOPLUS_CFLAGS)
+ $(STDPLUS_CFLAGS)
sensor_unittest_LDADD = \
-lstdc++fs \
$(PHOSPHOR_LOGGING_LIBS) \
$(GPIOPLUS_LIBS) \
+ $(STDPLUS_LIBS) \
$(top_builddir)/sensor.o \
$(top_builddir)/hwmon.o \
env.o \