From 1181af741589db873676f177ed85d6bc04884aa1 Mon Sep 17 00:00:00 2001 From: Vernon Mauery Date: Mon, 8 Oct 2018 12:05:00 -0700 Subject: Use the common timer class The common timer class from sdbusplus offers all the timer goodness that we currently use. The unit test is also no longer needed (and has been added to sdbusplus's version of the timer.hpp implementation). Change-Id: I278817489433a29ca739f70fdacd8bb897797d66 Signed-off-by: Vernon Mauery --- .gitignore | 3 - Makefile.am | 1 - chassishandler.cpp | 22 ++-- configure.ac | 2 +- host-cmd-manager.cpp | 8 +- host-cmd-manager.hpp | 4 +- ipmid.cpp | 4 +- softoff/Makefile.am | 2 - softoff/mainapp.cpp | 2 +- softoff/softoff.cpp | 4 +- softoff/softoff.hpp | 3 +- softoff/test/Makefile.am | 12 --- softoff/test/utest.cpp | 263 ----------------------------------------------- timer.cpp | 111 -------------------- timer.hpp | 101 ------------------ transporthandler.cpp | 9 +- 16 files changed, 27 insertions(+), 524 deletions(-) delete mode 100644 softoff/test/Makefile.am delete mode 100644 softoff/test/utest.cpp delete mode 100644 timer.cpp delete mode 100644 timer.hpp diff --git a/.gitignore b/.gitignore index 53f37f1..c368285 100644 --- a/.gitignore +++ b/.gitignore @@ -50,9 +50,6 @@ Makefile /ipmid .project /softoff/phosphor-softpoweroff -/softoff/test/utest -/softoff/test/*.log -/softoff/test/*.trs /softoff/xyz/ /test/sample_unittest /test/*.log diff --git a/Makefile.am b/Makefile.am index 28ae7ed..a5dfdab 100644 --- a/Makefile.am +++ b/Makefile.am @@ -7,7 +7,6 @@ ipmid_SOURCES = \ ipmid.cpp \ settings.cpp \ host-cmd-manager.cpp \ - timer.cpp \ utils.cpp \ oemrouter.cpp nodist_ipmid_SOURCES = ipmiwhitelist.cpp diff --git a/chassishandler.cpp b/chassishandler.cpp index dab62c4..d75b3b2 100644 --- a/chassishandler.cpp +++ b/chassishandler.cpp @@ -4,7 +4,6 @@ #include "ipmid.hpp" #include "settings.hpp" -#include "timer.hpp" #include "types.hpp" #include "utils.hpp" @@ -25,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -49,13 +49,11 @@ namespace filesystem = std::experimental::filesystem; // Defines #define SET_PARM_VERSION 0x01 -#define SET_PARM_BOOT_FLAGS_PERMANENT 0x40 // boot flags data1 7th bit on -#define SET_PARM_BOOT_FLAGS_VALID_ONE_TIME 0x80 // boot flags data1 8th bit on -#define SET_PARM_BOOT_FLAGS_VALID_PERMANENT \ - 0xC0 // boot flags data1 7 & 8 bit - // on +#define SET_PARM_BOOT_FLAGS_PERMANENT 0x40 +#define SET_PARM_BOOT_FLAGS_VALID_ONE_TIME 0x80 +#define SET_PARM_BOOT_FLAGS_VALID_PERMANENT 0xC0 -std::unique_ptr identifyTimer = nullptr; +std::unique_ptr identifyTimer = nullptr; constexpr size_t SIZE_MAC = 18; constexpr size_t SIZE_BOOT_OPTION = (uint8_t) @@ -1177,8 +1175,8 @@ void createIdentifyTimer() { if (!identifyTimer) { - identifyTimer = std::make_unique( - ipmid_get_sd_event_connection(), enclosureIdentifyLedOff); + identifyTimer = + std::make_unique(enclosureIdentifyLedOff); } } @@ -1205,7 +1203,7 @@ ipmi_ret_t ipmi_chassis_identify(ipmi_netfn_t netfn, ipmi_cmd_t cmd, { // stop the timer if already started, for force identify we should // not turn off LED - identifyTimer->setTimer(SD_EVENT_OFF); + identifyTimer->stop(); try { enclosureIdentifyLed(true); @@ -1223,11 +1221,11 @@ ipmi_ret_t ipmi_chassis_identify(ipmi_netfn_t netfn, ipmi_cmd_t cmd, // start the timer auto time = std::chrono::duration_cast( std::chrono::seconds(identifyInterval)); - identifyTimer->startTimer(time); + identifyTimer->start(time); } else if (!identifyInterval) { - identifyTimer->setTimer(SD_EVENT_OFF); + identifyTimer->stop(); enclosureIdentifyLedOff(); } return IPMI_CC_OK; diff --git a/configure.ac b/configure.ac index 73d63f6..bd8c454 100644 --- a/configure.ac +++ b/configure.ac @@ -160,5 +160,5 @@ AS_IF([test "x$POWER_READING_SENSOR" == "x"],[POWER_READING_SENSOR="/usr/share/i AC_DEFINE_UNQUOTED([POWER_READING_SENSOR], ["$POWER_READING_SENSOR"], [Power reading sensor configuration file]) # Create configured output -AC_CONFIG_FILES([Makefile test/Makefile softoff/Makefile softoff/test/Makefile]) +AC_CONFIG_FILES([Makefile test/Makefile softoff/Makefile]) AC_OUTPUT diff --git a/host-cmd-manager.cpp b/host-cmd-manager.cpp index 4084910..2f96661 100644 --- a/host-cmd-manager.cpp +++ b/host-cmd-manager.cpp @@ -3,12 +3,12 @@ #include "host-cmd-manager.hpp" #include "systemintfcmds.hpp" -#include "timer.hpp" -#include "utils.hpp" #include #include #include +#include +#include #include #include @@ -47,7 +47,7 @@ Manager::Manager(sdbusplus::bus::bus& bus, sd_event* event) : IpmiCmdData Manager::getNextCommand() { // Stop the timer. Don't have to Err failure doing so. - auto r = timer.setTimer(SD_EVENT_OFF); + auto r = timer.stop(); if (r < 0) { log("Failure to STOP the timer", @@ -123,7 +123,7 @@ void Manager::checkQueueAndAlertHost() auto time = std::chrono::duration_cast( std::chrono::seconds(IPMI_SMS_ATN_ACK_TIMEOUT_SECS)); - auto r = timer.startTimer(time); + auto r = timer.start(time); if (r < 0) { log("Error starting timer for control host"); diff --git a/host-cmd-manager.hpp b/host-cmd-manager.hpp index 949e798..9a5891c 100644 --- a/host-cmd-manager.hpp +++ b/host-cmd-manager.hpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include namespace phosphor @@ -101,7 +101,7 @@ class Manager std::queue workQueue{}; /** @brief Timer for commands to host */ - phosphor::ipmi::Timer timer; + phosphor::Timer timer; /** @brief Match handler for the requested host state */ sdbusplus::bus::match_t hostTransitionMatch; diff --git a/ipmid.cpp b/ipmid.cpp index be6b4d4..e19eda0 100644 --- a/ipmid.cpp +++ b/ipmid.cpp @@ -4,7 +4,6 @@ #include "ipmiwhitelist.hpp" #include "sensorhandler.hpp" #include "settings.hpp" -#include "timer.hpp" #include #include @@ -26,6 +25,7 @@ #include #include #include +#include #include #include @@ -44,7 +44,7 @@ using cmdManagerPtr = std::unique_ptr; cmdManagerPtr cmdManager; // Global timer for network changes -std::unique_ptr networkTimer = nullptr; +std::unique_ptr networkTimer = nullptr; // Command and handler tuple. Used when clients ask the command to be put // into host message queue diff --git a/softoff/Makefile.am b/softoff/Makefile.am index cf50cdb..ed0c226 100644 --- a/softoff/Makefile.am +++ b/softoff/Makefile.am @@ -6,7 +6,6 @@ sbin_PROGRAMS = phosphor-softpoweroff # https://debbugs.gnu.org/cgi/bugreport.cgi?bug=13928 phosphor_softpoweroff_SOURCES = \ softoff.cpp \ - ../timer.cpp \ mainapp.cpp \ xyz/openbmc_project/Ipmi/Internal/SoftPowerOff/server.cpp \ ../utils.cpp @@ -35,4 +34,3 @@ xyz/openbmc_project/Ipmi/Internal/SoftPowerOff/server.hpp: ${top_srcdir}/xyz/ope @mkdir -p `dirname $@` $(SDBUSPLUSPLUS) -r $(top_srcdir) interface server-header xyz.openbmc_project.Ipmi.Internal.SoftPowerOff > $@ -SUBDIRS = test diff --git a/softoff/mainapp.cpp b/softoff/mainapp.cpp index 3b153a8..f6a52ac 100644 --- a/softoff/mainapp.cpp +++ b/softoff/mainapp.cpp @@ -16,12 +16,12 @@ #include "config.h" #include "softoff.hpp" -#include "timer.hpp" #include #include #include +#include #include // Return -1 on any errors to ensure we follow the calling targets OnFailure= diff --git a/softoff/softoff.cpp b/softoff/softoff.cpp index 921536f..803efc0 100644 --- a/softoff/softoff.cpp +++ b/softoff/softoff.cpp @@ -103,7 +103,7 @@ void SoftPowerOff::hostControlEvent(sdbusplus::message::message& msg) // Starts a timer int SoftPowerOff::startTimer(const std::chrono::microseconds& usec) { - return timer.startTimer(usec); + return timer.start(usec); } // Host Response handler @@ -115,7 +115,7 @@ auto SoftPowerOff::responseReceived(HostResponse response) -> HostResponse { // Disable the timer since Host has quiesced and we are // done with soft power off part - auto r = timer.setTimer(SD_EVENT_OFF); + auto r = timer.stop(); if (r < 0) { log("Failure to STOP the timer", diff --git a/softoff/softoff.hpp b/softoff/softoff.hpp index b7555e3..1c9341b 100644 --- a/softoff/softoff.hpp +++ b/softoff/softoff.hpp @@ -2,11 +2,10 @@ #include "config.h" -#include "timer.hpp" - #include #include #include +#include #include #include namespace phosphor diff --git a/softoff/test/Makefile.am b/softoff/test/Makefile.am deleted file mode 100644 index afdb620..0000000 --- a/softoff/test/Makefile.am +++ /dev/null @@ -1,12 +0,0 @@ -AM_CPPFLAGS = -I$(top_srcdir)/softoff - -# Run all 'check' test programs -TESTS = $(check_PROGRAMS) - -# # Build/add utest to test suite -check_PROGRAMS = utest -utest_CPPFLAGS = -Igtest $(GTEST_CPPFLAGS) $(AM_CPPFLAGS) -utest_CXXFLAGS = $(PTHREAD_CFLAGS) -utest_LDFLAGS = -lgtest_main -lgtest $(PTHREAD_LIBS) $(OESDK_TESTCASE_FLAGS) $(SYSTEMD_LIBS) ${SDBUSPLUS_LIBS} -utest_SOURCES = utest.cpp -utest_LDADD = $(top_builddir)/timer.o diff --git a/softoff/test/utest.cpp b/softoff/test/utest.cpp deleted file mode 100644 index 0dd99ae..0000000 --- a/softoff/test/utest.cpp +++ /dev/null @@ -1,263 +0,0 @@ -#include "timer.hpp" - -#include -#include - -#include - -using namespace phosphor::ipmi; - -class TimerTest : public ::testing::Test -{ - public: - // systemd event handler - sd_event* events; - - // Need this so that events can be initialized. - int rc; - - // Source of event - sd_event_source* eventSource = nullptr; - - // Add a Timer Object - Timer timer; - - // Gets called as part of each TEST_F construction - TimerTest() : rc(sd_event_default(&events)), timer(events) - { - // Check for successful creation of - // event handler and timer object. - EXPECT_GE(rc, 0); - } - - // Gets called as part of each TEST_F destruction - ~TimerTest() - { - events = sd_event_unref(events); - } -}; - -class TimerTestCallBack : public ::testing::Test -{ - public: - // systemd event handler - sd_event* events; - - // Need this so that events can be initialized. - int rc; - - // Source of event - sd_event_source* eventSource = nullptr; - - // Add a Timer Object - std::unique_ptr timer = nullptr; - - // Indicates optional call back fun was called - bool callBackDone = false; - - void callBack() - { - callBackDone = true; - } - - // Gets called as part of each TEST_F construction - TimerTestCallBack() : rc(sd_event_default(&events)) - - { - // Check for successful creation of - // event handler and timer object. - EXPECT_GE(rc, 0); - - std::function func( - std::bind(&TimerTestCallBack::callBack, this)); - timer = std::make_unique(events, func); - } - - // Gets called as part of each TEST_F destruction - ~TimerTestCallBack() - { - events = sd_event_unref(events); - } -}; - -/** @brief Makes sure that timer is expired and the - * callback handler gets invoked post 2 seconds - */ -TEST_F(TimerTest, timerExpiresAfter2seconds) -{ - using namespace std::chrono; - - auto time = duration_cast(seconds(2)); - EXPECT_GE(timer.startTimer(time), 0); - - // Waiting 2 seconds is enough here since we have - // already spent some usec now - int count = 0; - while (count < 2 && !timer.isExpired()) - { - // Returns -0- on timeout and positive number on dispatch - auto sleepTime = duration_cast(seconds(1)); - if (!sd_event_run(events, sleepTime.count())) - { - count++; - } - } - EXPECT_EQ(true, timer.isExpired()); - EXPECT_EQ(1, count); -} - -/** @brief Makes sure that timer is not expired - */ -TEST_F(TimerTest, timerNotExpiredAfter2Seconds) -{ - using namespace std::chrono; - - auto time = duration_cast(seconds(2)); - EXPECT_GE(timer.startTimer(time), 0); - - // Now turn off the timer post a 1 second sleep - sleep(1); - EXPECT_GE(timer.setTimer(SD_EVENT_OFF), 0); - - // Wait 2 seconds and see that timer is not expired - int count = 0; - while (count < 2) - { - // Returns -0- on timeout - auto sleepTime = duration_cast(seconds(1)); - if (!sd_event_run(events, sleepTime.count())) - { - count++; - } - } - EXPECT_EQ(false, timer.isExpired()); - - // 2 because of one more count that happens prior to exiting - EXPECT_EQ(2, count); -} - -/** @brief Makes sure that timer value is changed in between - * and that the new timer expires - */ -TEST_F(TimerTest, updateTimerAndExpectExpire) -{ - using namespace std::chrono; - - auto time = duration_cast(seconds(2)); - EXPECT_GE(timer.startTimer(time), 0); - - // Now sleep for a second and then set the new timeout value - sleep(1); - - // New timeout is 3 seconds from THIS point. - time = duration_cast(seconds(3)); - EXPECT_GE(timer.startTimer(time), 0); - - // Wait 3 seconds and see that timer is expired - int count = 0; - while (count < 3 && !timer.isExpired()) - { - // Returns -0- on timeout - auto sleepTime = duration_cast(seconds(1)); - if (!sd_event_run(events, sleepTime.count())) - { - count++; - } - } - EXPECT_EQ(true, timer.isExpired()); - EXPECT_EQ(2, count); -} - -/** @brief Makes sure that timer value is changed in between - * and turn off and make sure that timer does not expire - */ -TEST_F(TimerTest, updateTimerAndNeverExpire) -{ - using namespace std::chrono; - - auto time = duration_cast(seconds(2)); - EXPECT_GE(timer.startTimer(time), 0); - - // Now sleep for a second and then set the new timeout value - sleep(1); - - // New timeout is 2 seconds from THIS point. - time = duration_cast(seconds(2)); - EXPECT_GE(timer.startTimer(time), 0); - - // Now turn off the timer post a 1 second sleep - sleep(1); - EXPECT_GE(timer.setTimer(SD_EVENT_OFF), 0); - - // Wait 2 seconds and see that timer is expired - int count = 0; - while (count < 2) - { - // Returns -0- on timeout - auto sleepTime = duration_cast(seconds(1)); - if (!sd_event_run(events, sleepTime.count())) - { - count++; - } - } - EXPECT_EQ(false, timer.isExpired()); - - // 2 because of one more count that happens prior to exiting - EXPECT_EQ(2, count); -} - -/** @brief Makes sure that optional callback is called */ -TEST_F(TimerTestCallBack, optionalFuncCallBackDone) -{ - using namespace std::chrono; - - auto time = duration_cast(seconds(2)); - EXPECT_GE(timer->startTimer(time), 0); - - // Waiting 2 seconds is enough here since we have - // already spent some usec now - int count = 0; - while (count < 2 && !timer->isExpired()) - { - // Returns -0- on timeout and positive number on dispatch - auto sleepTime = duration_cast(seconds(1)); - if (!sd_event_run(events, sleepTime.count())) - { - count++; - } - } - EXPECT_EQ(true, timer->isExpired()); - EXPECT_EQ(true, callBackDone); - EXPECT_EQ(1, count); -} - -/** @brief Makes sure that timer is not expired - */ -TEST_F(TimerTestCallBack, timerNotExpiredAfter2SecondsNoOptionalCallBack) -{ - using namespace std::chrono; - - auto time = duration_cast(seconds(2)); - EXPECT_GE(timer->startTimer(time), 0); - - // Now turn off the timer post a 1 second sleep - sleep(1); - EXPECT_GE(timer->setTimer(SD_EVENT_OFF), 0); - - // Wait 2 seconds and see that timer is not expired - int count = 0; - while (count < 2) - { - // Returns -0- on timeout - auto sleepTime = duration_cast(seconds(1)); - if (!sd_event_run(events, sleepTime.count())) - { - count++; - } - } - EXPECT_EQ(false, timer->isExpired()); - EXPECT_EQ(false, callBackDone); - - // 2 because of one more count that happens prior to exiting - EXPECT_EQ(2, count); -} diff --git a/timer.cpp b/timer.cpp deleted file mode 100644 index f1887fd..0000000 --- a/timer.cpp +++ /dev/null @@ -1,111 +0,0 @@ -#include "timer.hpp" - -#include -#include -namespace phosphor -{ -namespace ipmi -{ - -using namespace phosphor::logging; - -// Initializes the timer object -void Timer::initialize() -{ - // This can not be called more than once. - if (eventSource) - { - throw std::runtime_error("Timer already initialized"); - } - - // Add infinite expiration time - auto r = sd_event_add_time(timeEvent, &eventSource, - CLOCK_MONOTONIC, // Time base - UINT64_MAX, // Expire time - way long time - 0, // Use default event accuracy - timeoutHandler, // Callback handler on timeout - this); // User data - if (r < 0) - { - log("Failure to set initial expiration time value", - entry("ERROR=%s", strerror(-r))); - - throw std::runtime_error("Timer initialization failed"); - } - - // Disable the timer for now - r = setTimer(SD_EVENT_OFF); - if (r < 0) - { - log("Failure to disable timer", - entry("ERROR=%s", strerror(-r))); - - throw std::runtime_error("Disabling the timer failed"); - } - return; -} - -/** @brief callback handler on timeout */ -int Timer::timeoutHandler(sd_event_source* eventSource, uint64_t usec, - void* userData) -{ - auto timer = static_cast(userData); - timer->expired = true; - - log("Timer expired"); - // Call optional user call back function if available - if (timer->userCallBack) - { - timer->userCallBack(); - } - - sd_event_source_set_enabled(eventSource, SD_EVENT_OFF); - return 0; -} - -// Gets the time from steady_clock -std::chrono::microseconds Timer::getTime() -{ - using namespace std::chrono; - auto usec = steady_clock::now().time_since_epoch(); - return duration_cast(usec); -} - -// Enables or disables the timer -int Timer::setTimer(int action) -{ - return sd_event_source_set_enabled(eventSource, action); -} - -// Sets the time and arms the timer -int Timer::startTimer(std::chrono::microseconds timeValue) -{ - // Disable the timer - setTimer(SD_EVENT_OFF); - expired = false; - - // Get the current MONOTONIC time and add the delta - auto expireTime = getTime() + timeValue; - - // Set the time - auto r = sd_event_source_set_time(eventSource, expireTime.count()); - if (r < 0) - { - log("Failure to set timer", - entry("ERROR=%s", strerror(-r))); - return r; - } - - // A ONESHOT timer means that when the timer goes off, - // its moves to disabled state. - r = setTimer(SD_EVENT_ONESHOT); - if (r < 0) - { - log("Failure to start timer", - entry("ERROR=%s", strerror(-r))); - } - return r; -} - -} // namespace ipmi -} // namespace phosphor diff --git a/timer.hpp b/timer.hpp deleted file mode 100644 index ec432ba..0000000 --- a/timer.hpp +++ /dev/null @@ -1,101 +0,0 @@ -#pragma once - -#include - -#include -#include -namespace phosphor -{ -namespace ipmi -{ - -/** @class Timer - * @brief Manages starting watchdog timers and handling timeouts - */ -class Timer -{ - public: - /** @brief Only need the default Timer */ - 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] funcCallBack - optional function callback for timer - * expirations - */ - Timer(sd_event* events, std::function userCallBack = nullptr) : - timeEvent(events), userCallBack(userCallBack) - { - // Initialize the timer - initialize(); - } - - ~Timer() - { - if (eventSource) - { - eventSource = sd_event_source_unref(eventSource); - } - } - - inline auto isExpired() const - { - return expired; - } - - /** @brief Starts the timer with specified expiration value. - * input is an offset from the current steady_clock - */ - int startTimer(std::chrono::microseconds usec); - - /** @brief Enables / disables the timer */ - int setTimer(int action); - - private: - /** @brief the sd_event structure */ - sd_event* timeEvent = nullptr; - - /** @brief Source of events */ - sd_event_source* eventSource = nullptr; - - /** @brief Returns if the associated timer is expired - * - * This is set to true when the timeoutHandler is called into - */ - bool expired = true; - - /** @brief Initializes the timer object with infinite - * expiration time and sets up the callback handler - * - * @return None. - * - * @error std::runtime exception thrown - */ - void initialize(); - - /** @brief Callback function when timer goes off - * - * On getting the signal, initiate the hard power off request - * - * @param[in] eventSource - Source of the event - * @param[in] usec - time in micro seconds - * @param[in] userData - User data pointer - * - */ - static int timeoutHandler(sd_event_source* eventSource, uint64_t usec, - void* userData); - - /** @brief Gets the current time from steady clock */ - static std::chrono::microseconds getTime(); - - /** @brief Optional function to call on timer expiration */ - std::function userCallBack; -}; - -} // namespace ipmi -} // namespace phosphor diff --git a/transporthandler.cpp b/transporthandler.cpp index 43c88bb..9c42887 100644 --- a/transporthandler.cpp +++ b/transporthandler.cpp @@ -3,7 +3,6 @@ #include "app/channel.hpp" #include "ipmid.hpp" #include "net.hpp" -#include "timer.hpp" #include "utils.hpp" #include @@ -13,6 +12,7 @@ #include #include #include +#include #include #include @@ -36,7 +36,7 @@ namespace filesystem = std::experimental::filesystem; #error filesystem not available #endif -extern std::unique_ptr networkTimer; +extern std::unique_ptr networkTimer; const int SIZE_MAC = 18; // xx:xx:xx:xx:xx:xx constexpr auto ipv4Protocol = "xyz.openbmc_project.Network.IP.Protocol.IPv4"; @@ -509,7 +509,7 @@ ipmi_ret_t ipmi_transport_set_lan(ipmi_netfn_t netfn, ipmi_cmd_t cmd, } // start/restart the timer - networkTimer->startTimer(networkTimeout); + networkTimer->start(networkTimeout); } else if (reqptr->data[0] == SET_IN_PROGRESS) // Set In Progress { @@ -955,8 +955,7 @@ void createNetworkTimer() std::function networkTimerCallback( std::bind(&commitNetworkChanges)); - networkTimer = std::make_unique( - ipmid_get_sd_event_connection(), networkTimerCallback); + networkTimer = std::make_unique(networkTimerCallback); } } -- cgit v1.2.1