timer: Replace with sdeventplus/timer
This is aimed at replacing the ad-hoc timer implementation contained in
each of our openbmc daemons, with a single well-tested timer
implementation.
Tested:
Compiled
Change-Id: I3e562ab72820442aa137a2d517e476192ea6c1bd
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/Makefile.am b/Makefile.am
index 5aff810..dfbd2cf 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -14,8 +14,7 @@
phosphor_chassis_state_manager_SOURCES = \
chassis_state_manager.cpp \
- chassis_state_manager_main.cpp \
- timer.cpp
+ chassis_state_manager_main.cpp
phosphor_bmc_state_manager_SOURCES = \
bmc_state_manager.cpp \
@@ -43,8 +42,13 @@
phosphor_host_state_manager_CXXFLAGS = $(generic_cxxflags)
phosphor_host_state_manager_LDFLAGS = $(generic_ldflags) -lstdc++fs
-phosphor_chassis_state_manager_CXXFLAGS = $(generic_cxxflags)
-phosphor_chassis_state_manager_LDFLAGS = $(generic_ldflags) -lstdc++fs
+phosphor_chassis_state_manager_CXXFLAGS = \
+ $(generic_cxxflags) \
+ $(SDEVENTPLUS_CFLAGS)
+phosphor_chassis_state_manager_LDFLAGS = \
+ $(generic_ldflags) \
+ $(SDEVENTPLUS_LIBS) \
+ -lstdc++fs
phosphor_bmc_state_manager_CXXFLAGS = $(generic_cxxflags)
phosphor_bmc_state_manager_LDFLAGS = $(generic_ldflags)
diff --git a/chassis_state_manager.cpp b/chassis_state_manager.cpp
index 4e8e1e7..d721460 100644
--- a/chassis_state_manager.cpp
+++ b/chassis_state_manager.cpp
@@ -1,5 +1,7 @@
#include <sdbusplus/bus.hpp>
#include <sdbusplus/exception.hpp>
+#include <sdeventplus/event.hpp>
+#include <sdeventplus/exception.hpp>
#include <phosphor-logging/log.hpp>
#include <phosphor-logging/elog-errors.hpp>
#include "xyz/openbmc_project/Common/error.hpp"
@@ -21,6 +23,7 @@
using namespace phosphor::logging;
using sdbusplus::exception::SdBusError;
+using sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
constexpr auto CHASSIS_STATE_POWEROFF_TGT = "obmc-chassis-poweroff@0.target";
constexpr auto CHASSIS_STATE_HARD_POWEROFF_TGT =
@@ -268,14 +271,7 @@
convertForMessage(value).c_str()));
chassisPowerState = server::Chassis::currentPowerState(value);
- if (chassisPowerState == PowerState::On)
- {
- timer->state(timer::ON);
- }
- else
- {
- timer->state(timer::OFF);
- }
+ pOHTimer.setEnabled(chassisPowerState == PowerState::On);
return chassisPowerState;
}
@@ -289,6 +285,14 @@
return pOHCounter();
}
+void Chassis::pOHCallback()
+{
+ if (ChassisInherit::currentPowerState() == PowerState::On)
+ {
+ pOHCounter(pOHCounter() + 1);
+ }
+}
+
void Chassis::restorePOHCounter()
{
uint32_t counter;
@@ -340,47 +344,19 @@
void Chassis::startPOHCounter()
{
- using namespace std::chrono_literals;
- using namespace phosphor::logging;
- using namespace sdbusplus::xyz::openbmc_project::Common::Error;
-
auto dir = fs::path(POH_COUNTER_PERSIST_PATH).parent_path();
fs::create_directories(dir);
- sd_event* event = nullptr;
- auto r = sd_event_default(&event);
- if (r < 0)
- {
- log<level::ERR>("Error creating a default sd_event handler");
- throw;
- }
-
- phosphor::state::manager::EventPtr eventP{event};
- event = nullptr;
-
- auto callback = [&]() {
- if (ChassisInherit::currentPowerState() == PowerState::On)
- {
- pOHCounter(pOHCounter() + 1);
- }
- };
-
try
{
- timer = std::make_unique<phosphor::state::manager::Timer>(
- eventP, callback, std::chrono::seconds(POH::hour),
- phosphor::state::manager::timer::ON);
- bus.attach_event(eventP.get(), SD_EVENT_PRIORITY_NORMAL);
- r = sd_event_loop(eventP.get());
- if (r < 0)
- {
- log<level::ERR>("Error occurred during the sd_event_loop",
- entry("RC=%d", r));
- elog<InternalFailure>();
- }
+ auto event = sdeventplus::Event::get_default();
+ bus.attach_event(event.get(), SD_EVENT_PRIORITY_NORMAL);
+ event.loop();
}
- catch (InternalFailure& e)
+ catch (const sdeventplus::SdEventError& e)
{
+ log<level::ERR>("Error occurred during the sdeventplus loop",
+ entry("ERROR=%s", e.what()));
phosphor::logging::commit<InternalFailure>();
}
}
diff --git a/chassis_state_manager.hpp b/chassis_state_manager.hpp
index 9a6b647..4780e0f 100644
--- a/chassis_state_manager.hpp
+++ b/chassis_state_manager.hpp
@@ -1,13 +1,16 @@
#pragma once
+#include <chrono>
#include <functional>
#include <experimental/filesystem>
#include <cereal/cereal.hpp>
#include <sdbusplus/bus.hpp>
+#include <sdeventplus/clock.hpp>
+#include <sdeventplus/event.hpp>
+#include <sdeventplus/utility/timer.hpp>
#include "xyz/openbmc_project/State/Chassis/server.hpp"
#include "xyz/openbmc_project/State/PowerOnHours/server.hpp"
#include "config.h"
-#include "timer.hpp"
namespace phosphor
{
@@ -15,13 +18,6 @@
{
namespace manager
{
-namespace POH
-{
-
-using namespace std::chrono_literals;
-constexpr auto hour = 3600s; // seconds Per Hour
-
-} // namespace POH
using ChassisInherit = sdbusplus::server::object::object<
sdbusplus::xyz::openbmc_project::State::server::Chassis,
@@ -57,7 +53,10 @@
sdbusRule::path("/org/freedesktop/systemd1") +
sdbusRule::interface("org.freedesktop.systemd1.Manager"),
std::bind(std::mem_fn(&Chassis::sysStateChange), this,
- std::placeholders::_1))
+ std::placeholders::_1)),
+ pOHTimer(sdeventplus::Event::get_default(),
+ std::bind(&Chassis::pOHCallback, this), std::chrono::hours{1},
+ std::chrono::minutes{1})
{
subscribeToSystemdSignals();
@@ -136,6 +135,9 @@
/** @brief Used to Set value of POHCounter */
uint32_t pOHCounter(uint32_t value) override;
+ /** @brief Used by the timer to update the POHCounter */
+ void pOHCallback();
+
/** @brief Used to restore POHCounter value from persisted file */
void restorePOHCounter();
@@ -188,8 +190,8 @@
*/
void restoreChassisStateChangeTime();
- /** @brief Timer */
- std::unique_ptr<phosphor::state::manager::Timer> timer;
+ /** @brief Timer used for tracking power on hours */
+ sdeventplus::utility::Timer<sdeventplus::ClockId::Monotonic> pOHTimer;
};
} // namespace manager
diff --git a/configure.ac b/configure.ac
index bd3ecac..e985a95 100644
--- a/configure.ac
+++ b/configure.ac
@@ -16,6 +16,8 @@
AC_MSG_ERROR(["Requires phosphor-dbus-interfaces package."]))
PKG_CHECK_MODULES([SDBUSPLUS], [sdbusplus],,
AC_MSG_ERROR(["Requires sdbusplus package."]))
+PKG_CHECK_MODULES([SDEVENTPLUS], [sdeventplus],,
+ AC_MSG_ERROR(["Requires sdeventplus package."]))
PKG_CHECK_MODULES([PHOSPHOR_LOGGING], [phosphor-logging],,\
AC_MSG_ERROR(["Requires phosphor-logging package."]))
diff --git a/timer.cpp b/timer.cpp
deleted file mode 100644
index ceb4c29..0000000
--- a/timer.cpp
+++ /dev/null
@@ -1,89 +0,0 @@
-#include <chrono>
-#include <system_error>
-#include <string.h>
-#include "timer.hpp"
-
-namespace phosphor
-{
-namespace state
-{
-namespace manager
-{
-static std::chrono::microseconds getTime()
-{
- using namespace std::chrono;
- auto usec = steady_clock::now().time_since_epoch();
- return duration_cast<microseconds>(usec);
-}
-
-int Timer::state(timer::Action value)
-{
- action_ = value;
- return sd_event_source_set_enabled(eventSource_.get(), action_);
-}
-
-timer::Action Timer::getAction() const
-{
- return action_;
-}
-
-std::chrono::microseconds Timer::getDuration() const
-{
- return duration_;
-}
-
-Timer::Timer(EventPtr& event, std::function<void()> callback,
- std::chrono::microseconds usec, timer::Action action) :
- event_(event),
- callback_(callback), duration_(usec), action_(action)
-{
- // Add infinite expiration time
- sd_event_source* sourcePtr = nullptr;
-
- auto r = sd_event_add_time(event_.get(), &sourcePtr,
- CLOCK_MONOTONIC, // Time base
- (getTime() + usec).count(), // When to fire
- 0, // Use default event accuracy
- timeoutHandler, // Callback handler on timeout
- this); // User data
-
- if (r < 0)
- {
- throw std::system_error(r, std::generic_category(), strerror(-r));
- }
-
- eventSource_.reset(sourcePtr);
-}
-
-int Timer::timeoutHandler(sd_event_source* eventSrc, uint64_t usec,
- void* userData)
-{
- auto timer = static_cast<Timer*>(userData);
-
- if (timer->getAction() == timer::ON)
- {
- auto r = sd_event_source_set_time(
- eventSrc, (getTime() + timer->getDuration()).count());
- if (r < 0)
- {
- throw std::system_error(r, std::generic_category(), strerror(-r));
- }
-
- r = sd_event_source_set_enabled(eventSrc, timer::ON);
- if (r < 0)
- {
- throw std::system_error(r, std::generic_category(), strerror(-r));
- }
- }
-
- if (timer->callback_)
- {
- timer->callback_();
- }
-
- return 0;
-}
-
-} // namespace manager
-} // namespace state
-} // namespace phosphor
diff --git a/timer.hpp b/timer.hpp
deleted file mode 100644
index f5f9ada..0000000
--- a/timer.hpp
+++ /dev/null
@@ -1,112 +0,0 @@
-#pragma once
-
-#include <memory>
-#include <chrono>
-#include <functional>
-#include <systemd/sd-event.h>
-
-namespace phosphor
-{
-namespace state
-{
-namespace manager
-{
-namespace timer
-{
-
-enum Action
-{
- OFF = SD_EVENT_OFF,
- ON = SD_EVENT_ON,
- ONESHOT = SD_EVENT_ONESHOT
-};
-} // namespace timer
-
-/* Need a custom deleter for freeing up sd_event */
-struct EventDeleter
-{
- void operator()(sd_event* event) const
- {
- event = sd_event_unref(event);
- }
-};
-using EventPtr = std::unique_ptr<sd_event, EventDeleter>;
-
-/* Need a custom deleter for freeing up sd_event_source */
-struct EventSourceDeleter
-{
- void operator()(sd_event_source* eventSource) const
- {
- eventSource = sd_event_source_unref(eventSource);
- }
-};
-using EventSourcePtr = std::unique_ptr<sd_event_source, EventSourceDeleter>;
-
-/** @class Timer
- * @brief Provides a timer source and a mechanism to callback when the timer
- * expires.
- *
- * The timer is armed upon construction. The constructor requires a timeout
- * handler function, the timer expiry duration, and the timer state (one-shot,
- * reptitive, disabled).
- * It's possible to change the state of the timer after it's been armed via the
- * state() API.
- */
-class Timer
-{
- public:
- Timer() = delete;
- Timer(const Timer&) = delete;
- Timer& operator=(const Timer&) = delete;
- Timer(Timer&&) = delete;
- Timer& operator=(Timer&&) = delete;
-
- /** @brief Constructs timer object
- *
- * @param[in] events - sd_event pointer
- * @param[in] callback - function callback for timer expiry
- * @param[in] usec - timer duration, in micro seconds
- * @param[in] action - controls the timer's lifetime
- */
- Timer(EventPtr& event, std::function<void()> userCallback,
- std::chrono::microseconds usec, timer::Action action);
-
- /** @brief Enables / disables the timer
- * @param[in] action - controls the timer's lifetime
- */
- int state(timer::Action value);
-
- timer::Action getAction() const;
-
- std::chrono::microseconds getDuration() const;
-
- private:
- /** @brief Reference to sd_event unique pointer */
- EventPtr& event_;
-
- /** @brief Source of events */
- EventSourcePtr eventSource_;
-
- /** @brief Optional function to call on timer expiration */
- std::function<void()> callback_{};
-
- /** @brief Duration of the timer */
- std::chrono::microseconds duration_{};
-
- /** @brief whether the timer is enabled/disabled/one-shot */
- timer::Action action_ = timer::OFF;
-
- /** @brief Timer expiry handler - invokes callback
- *
- * @param[in] eventSource - Source of the event
- * @param[in] usec - time in micro seconds
- * @param[in] userData - User data pointer
- *
- * @return zero on success, non-zero otherwise
- */
- static int timeoutHandler(sd_event_source* eventSource, uint64_t usec,
- void* userData);
-};
-} // namespace manager
-} // namespace state
-} // namespace phosphor