diff --git a/Makefile.am b/Makefile.am index 1010533..052a978 100644 --- a/Makefile.am +++ b/Makefile.am @@ -7,7 +7,6 @@ noinst_LTLIBRARIES = libtimemanager.la libtimemanager_la_SOURCES = \ epoch_base.cpp \ bmc_epoch.cpp \ - host_epoch.cpp \ manager.cpp \ utils.cpp \ settings.cpp \ diff --git a/README.md b/README.md index a3fea5e..5b1d5cd 100644 --- a/README.md +++ b/README.md @@ -1,12 +1,11 @@ # Introduction `phosphor-time-manager` is the time manager service that implements D-Bus interface `xyz/openbmc_project/Time/EpochTime.interface.yaml`. -The user can get or set the BMC's or HOST's time via this interface. +The user can get or set the BMC's time via this interface. ### General usage -The service `xyz.openbmc_project.Time.Manager` provides two objects on D-Bus: +The service `xyz.openbmc_project.Time.Manager` provides an object on D-Bus: * /xyz/openbmc_project/time/bmc -* /xyz/openbmc_project/time/host where each object implements interface `xyz.openbmc_project.Time.EpochTime`. @@ -22,44 +21,33 @@ the time. For example on an authenticated session: ### With REST API on remote host curl -b cjar -k https://${BMC_IP}/xyz/openbmc_project/time/bmc ``` -* To set HOST's time: +* To set BMC's time: ``` ### With busctl on BMC busctl set-property xyz.openbmc_project.Time.Manager \ - /xyz/openbmc_project/time/host xyz.openbmc_project.Time.EpochTime \ + /xyz/openbmc_project/time/bmc xyz.openbmc_project.Time.EpochTime \ Elapsed t ### With REST API on remote host curl -b cjar -k -H "Content-Type: application/json" -X PUT \ -d '{"data": 1487304700000000}' \ - https://${BMC_IP}/xyz/openbmc_project/time/host/attr/Elapsed + https://${BMC_IP}/xyz/openbmc_project/time/bmc/attr/Elapsed ``` ### Time settings -Getting BMC or HOST time is always allowed, but setting the time may not be +Getting BMC time is always allowed, but setting the time may not be allowed depending on the below two settings in the settings manager. * TimeSyncMethod * NTP: The time is set via NTP server. * MANUAL: The time is set manually. -* TimeOwner - * BMC: BMC owns the time and can set the time. - * HOST: Host owns the time and can set the time. - * SPLIT: BMC and Host own separate time. - * BOTH: Both BMC and Host can set the time. A summary of which cases the time can be set on BMC or HOST: -Mode | Owner | Set BMC Time | Set Host Time ---------- | ----- | ------------- | ------------------- -NTP | BMC | Fail to set | Not allowed -NTP | HOST | Not allowed | Not allowed -NTP | SPLIT | Fail to set | OK -NTP | BOTH | Fail to set | Not allowed -MANUAL | BMC | OK | Not allowed -MANUAL | HOST | Not allowed | OK -MANUAL | SPLIT | OK | OK -MANUAL | BOTH | OK | OK +Mode | Set BMC Time +--------- | ------------- +NTP | Fail to set +MANUAL | OK * To set an NTP [server](https://tf.nist.gov/tf-cgi/servers.cgi): ``` @@ -88,19 +76,6 @@ MANUAL | BOTH | OK | OK https://${BMC_IP}/xyz/openbmc_project/time/sync_method/attr/TimeSyncMethod ``` -* To change owner - ``` - ### With busctl on BMC - busctl set-property xyz.openbmc_project.Settings \ - /xyz/openbmc_project/time/owner xyz.openbmc_project.Time.Owner \ - TimeOwner s xyz.openbmc_project.Time.Owner.Owners.BMC - - ### With REST API on remote host - curl -c cjar -b cjar -k -H "Content-Type: application/json" -X PUT -d \ - '{"data": "xyz.openbmc_project.Time.Owner.Owners.BMC" }' \ - https://${BMC_IP}/xyz/openbmc_project/time/owner/attr/TimeOwner - ``` - ### Special note on changing NTP setting Starting from OpenBMC 2.6 (with systemd v239), systemd's timedated introduces a new beahvior that it checks the NTP services' status during setting time, @@ -115,13 +90,13 @@ This results in [openbmc/openbmc#3459][1], and the related test cases are updated to cooperate with this behavior change. ### Special note on host on -When the host is on, the changes of the above time mode/owner are not applied but -deferred. The changes of the mode/owner are saved to persistent storage. +When the host is on, the changes of the above time mode are not applied but +deferred. The changes of the mode are saved to persistent storage. -When the host is off, the saved mode/owner are read from persistent storage and are +When the host is off, the saved mode are read from persistent storage and are applied. -Note: A user can set the time mode and owner in the settings daemon at any time, +Note: A user can set the time mode in the settings daemon at any time, but the time manager applying them is governed by the above condition. diff --git a/bmc_epoch.cpp b/bmc_epoch.cpp index d000a71..9c8affc 100644 --- a/bmc_epoch.cpp +++ b/bmc_epoch.cpp @@ -9,7 +9,6 @@ #include #include #include -#include // Need to do this since its not exported outside of the kernel. // Refer : https://gist.github.com/lethean/446cea944b7441228298 @@ -27,9 +26,6 @@ namespace time namespace server = sdbusplus::xyz::openbmc_project::Time::server; using namespace phosphor::logging; -using NotAllowedError = - sdbusplus::xyz::openbmc_project::Time::Error::NotAllowed; - BmcEpoch::BmcEpoch(sdbusplus::bus::bus& bus, const char* objPath) : EpochBase(bus, objPath), bus(bus) { @@ -84,63 +80,27 @@ BmcEpoch::~BmcEpoch() uint64_t BmcEpoch::elapsed() const { - // It does not needs to check owner when getting time return getTime().count(); } uint64_t BmcEpoch::elapsed(uint64_t value) { /* - Mode | Owner | Set BMC Time - ----- | ----- | ------------- - NTP | BMC | Fail to set - NTP | HOST | Not allowed - NTP | SPLIT | Fail to set - NTP | BOTH | Fail to set - MANUAL| BMC | OK - MANUAL| HOST | Not allowed - MANUAL| SPLIT | OK - MANUAL| BOTH | OK + Mode | Set BMC Time + ----- | ------------- + NTP | Fail to set + MANUAL| OK */ - if (timeOwner == Owner::Host) - { - using namespace xyz::openbmc_project::Time; - elog( - NotAllowed::OWNER(utils::ownerToStr(timeOwner).c_str()), - NotAllowed::SYNC_METHOD(utils::modeToStr(timeMode).c_str()), - NotAllowed::REASON( - "Setting BmcTime with HOST owner is not allowed")); - } - auto time = microseconds(value); - if (setTime(time)) - { - notifyBmcTimeChange(time); - } + setTime(time); server::EpochTime::elapsed(value); return value; } -void BmcEpoch::setBmcTimeChangeListener(BmcTimeChangeListener* listener) -{ - timeChangeListener = listener; -} - -void BmcEpoch::notifyBmcTimeChange(const microseconds& time) -{ - // Notify listener if it exists - if (timeChangeListener) - { - timeChangeListener->onBmcTimeChanged(time); - } -} - int BmcEpoch::onTimeChange(sd_event_source* es, int fd, uint32_t /* revents */, void* userdata) { - auto bmcEpoch = static_cast(userdata); - std::array time{}; // We are not interested in the data here. @@ -148,9 +108,6 @@ int BmcEpoch::onTimeChange(sd_event_source* es, int fd, uint32_t /* revents */, while (read(fd, time.data(), time.max_size()) > 0) ; - log("BMC system time is changed"); - bmcEpoch->notifyBmcTimeChange(bmcEpoch->getTime()); - return 0; } diff --git a/bmc_epoch.hpp b/bmc_epoch.hpp index 96c14b7..21cf540 100644 --- a/bmc_epoch.hpp +++ b/bmc_epoch.hpp @@ -1,6 +1,5 @@ #pragma once -#include "bmc_time_change_listener.hpp" #include "epoch_base.hpp" #include @@ -39,12 +38,6 @@ class BmcEpoch : public EpochBase **/ uint64_t elapsed(uint64_t value) override; - /** @brief Set the listner for bmc time change - * - * @param[in] listener - The pointer to the listener - */ - void setBmcTimeChangeListener(BmcTimeChangeListener* listener); - private: /** @brief The fd for time change event */ int timeFd = -1; @@ -52,12 +45,6 @@ class BmcEpoch : public EpochBase /** @brief Initialize timerFd related resource */ void initialize(); - /** @brief Notify the listeners that bmc time is changed - * - * @param[in] time - The epoch time in microseconds to notify - */ - void notifyBmcTimeChange(const microseconds& time); - /** @brief The callback function on system time change * * @param[in] es - Source of the event @@ -84,9 +71,6 @@ class BmcEpoch : public EpochBase /** @brief The event source on system time change */ SdEventSource timeChangeEventSource{nullptr, sdEventSourceDeleter}; - - /** @brief The listener for bmc time change */ - BmcTimeChangeListener* timeChangeListener = nullptr; }; } // namespace time diff --git a/bmc_time_change_listener.hpp b/bmc_time_change_listener.hpp deleted file mode 100644 index 2c382f6..0000000 --- a/bmc_time_change_listener.hpp +++ /dev/null @@ -1,23 +0,0 @@ -#pragma once - -#include - -namespace phosphor -{ -namespace time -{ - -class BmcTimeChangeListener -{ - public: - virtual ~BmcTimeChangeListener() = default; - - /** @brief Notified on bmc time is changed - * - * @param[in] bmcTime - The epoch time in microseconds - */ - virtual void onBmcTimeChanged(const std::chrono::microseconds& bmcTime) = 0; -}; - -} // namespace time -} // namespace phosphor diff --git a/configure.ac b/configure.ac index 4e6eb92..df0d310 100644 --- a/configure.ac +++ b/configure.ac @@ -65,21 +65,9 @@ AC_ARG_VAR(OBJPATH_BMC, [The bmc epoch Dbus root]) AS_IF([test "x$OBJPATH_BMC" == "x"], [OBJPATH_BMC="/xyz/openbmc_project/time/bmc"]) AC_DEFINE_UNQUOTED([OBJPATH_BMC], ["$OBJPATH_BMC"], [The bmc epoch Dbus root]) -AC_ARG_VAR(OBJPATH_HOST, [The host epoch Dbus root]) -AS_IF([test "x$OBJPATH_HOST" == "x"], [OBJPATH_HOST="/xyz/openbmc_project/time/host"]) -AC_DEFINE_UNQUOTED([OBJPATH_HOST], ["$OBJPATH_HOST"], [The host epoch Dbus root]) - -AC_ARG_VAR(HOST_OFFSET_FILE, [The file to save host time offset]) -AS_IF([test "x$HOST_OFFSET_FILE" == "x"], [HOST_OFFSET_FILE="/var/lib/obmc/saved_host_offset"]) -AC_DEFINE_UNQUOTED([HOST_OFFSET_FILE], ["$HOST_OFFSET_FILE"], [The file to save host time offset]) - AC_ARG_VAR(DEFAULT_TIME_MODE, [The default time mode]) AS_IF([test "x$DEFAULT_TIME_MODE" == "x"], [DEFAULT_TIME_MODE=Mode::Manual]) AC_DEFINE_UNQUOTED([DEFAULT_TIME_MODE], [$DEFAULT_TIME_MODE], [The default time mode]) -AC_ARG_VAR(DEFAULT_TIME_OWNER, [The default time owner]) -AS_IF([test "x$DEFAULT_TIME_OWNER" == "x"], [DEFAULT_TIME_OWNER=Owner::Both]) -AC_DEFINE_UNQUOTED([DEFAULT_TIME_OWNER], [$DEFAULT_TIME_OWNER], [The default time owner]) - AC_CONFIG_FILES([Makefile test/Makefile]) AC_OUTPUT diff --git a/epoch_base.cpp b/epoch_base.cpp index 9346391..507c5ff 100644 --- a/epoch_base.cpp +++ b/epoch_base.cpp @@ -33,11 +33,6 @@ void EpochBase::onModeChanged(Mode mode) timeMode = mode; } -void EpochBase::onOwnerChanged(Owner owner) -{ - timeOwner = owner; -} - using namespace std::chrono; bool EpochBase::setTime(const microseconds& usec) { diff --git a/epoch_base.hpp b/epoch_base.hpp index 80fa3f4..40025c4 100644 --- a/epoch_base.hpp +++ b/epoch_base.hpp @@ -30,9 +30,6 @@ class EpochBase : public sdbusplus::server::object::object< /** @brief Notified on time mode changed */ void onModeChanged(Mode mode) override; - /** @brief Notified on time owner changed */ - void onOwnerChanged(Owner owner) override; - protected: /** @brief Persistent sdbusplus DBus connection */ sdbusplus::bus::bus& bus; @@ -40,9 +37,6 @@ class EpochBase : public sdbusplus::server::object::object< /** @brief The current time mode */ Mode timeMode = DEFAULT_TIME_MODE; - /** @brief The current time owner */ - Owner timeOwner = DEFAULT_TIME_OWNER; - /** @brief Set current time to system * * This function set the time to system by invoking systemd diff --git a/host_epoch.cpp b/host_epoch.cpp deleted file mode 100644 index 96cbfa9..0000000 --- a/host_epoch.cpp +++ /dev/null @@ -1,129 +0,0 @@ -#include "host_epoch.hpp" - -#include "utils.hpp" - -#include -#include -#include - -namespace phosphor -{ -namespace time -{ -using namespace sdbusplus::xyz::openbmc_project::Time; -using namespace phosphor::logging; -using namespace std::chrono; - -using NotAllowedError = - sdbusplus::xyz::openbmc_project::Time::Error::NotAllowed; - -HostEpoch::HostEpoch(sdbusplus::bus::bus& bus, const char* objPath) : - EpochBase(bus, objPath), - offset(utils::readData(offsetFile)) -{ - // Initialize the diffToSteadyClock - auto steadyTime = - duration_cast(steady_clock::now().time_since_epoch()); - diffToSteadyClock = getTime() + offset - steadyTime; -} - -uint64_t HostEpoch::elapsed() const -{ - auto ret = getTime(); - if (timeOwner == Owner::Split) - { - ret += offset; - } - return ret.count(); -} - -uint64_t HostEpoch::elapsed(uint64_t value) -{ - /* - Mode | Owner | Set Host Time - ----- | ----- | ------------- - NTP | BMC | Not allowed - NTP | HOST | Not allowed - NTP | SPLIT | OK, and just save offset - NTP | BOTH | Not allowed - MANUAL| BMC | Not allowed - MANUAL| HOST | OK, and set time to BMC - MANUAL| SPLIT | OK, and just save offset - MANUAL| BOTH | OK, and set time to BMC - */ - if (timeOwner == Owner::BMC || - (timeMode == Mode::NTP && - (timeOwner == Owner::Host || timeOwner == Owner::Both))) - { - using namespace xyz::openbmc_project::Time; - elog( - NotAllowed::OWNER(utils::ownerToStr(timeOwner).c_str()), - NotAllowed::SYNC_METHOD(utils::modeToStr(timeMode).c_str()), - NotAllowed::REASON("Setting HostTime is not allowed")); - } - - auto time = microseconds(value); - if (timeOwner == Owner::Split) - { - // Calculate the offset between host and bmc time - offset = time - getTime(); - saveOffset(); - - // Calculate the diff between host and steady time - auto steadyTime = - duration_cast(steady_clock::now().time_since_epoch()); - diffToSteadyClock = time - steadyTime; - } - else - { - // Set time to BMC - setTime(time); - } - - server::EpochTime::elapsed(value); - return value; -} - -void HostEpoch::onOwnerChanged(Owner owner) -{ - // If timeOwner is changed to SPLIT, the offset shall be preserved - // Otherwise it shall be cleared; - timeOwner = owner; - if (timeOwner != Owner::Split) - { - offset = microseconds(0); - saveOffset(); - } - else - { - // In SPLIT, need to re-calculate the diff between - // host and steady time - auto steadyTime = - duration_cast(steady_clock::now().time_since_epoch()); - diffToSteadyClock = getTime() - steadyTime; - } -} - -void HostEpoch::saveOffset() -{ - // Store the offset to file - utils::writeData(offsetFile, offset.count()); -} - -void HostEpoch::onBmcTimeChanged(const microseconds& bmcTime) -{ - // If owner is split and BMC time is changed, - // the offset shall be adjusted - if (timeOwner == Owner::Split) - { - auto steadyTime = - duration_cast(steady_clock::now().time_since_epoch()); - auto hostTime = steadyTime + diffToSteadyClock; - offset = hostTime - bmcTime; - - saveOffset(); - } -} - -} // namespace time -} // namespace phosphor diff --git a/host_epoch.hpp b/host_epoch.hpp deleted file mode 100644 index bbf9d22..0000000 --- a/host_epoch.hpp +++ /dev/null @@ -1,73 +0,0 @@ -#pragma once - -#include "config.h" - -#include "bmc_time_change_listener.hpp" -#include "epoch_base.hpp" - -#include - -namespace phosphor -{ -namespace time -{ - -/** @class HostEpoch - * @brief OpenBMC HOST EpochTime implementation. - * @details A concrete implementation for xyz.openbmc_project.Time.EpochTime - * DBus API for HOST's epoch time. - */ -class HostEpoch : public EpochBase, public BmcTimeChangeListener -{ - public: - friend class TestHostEpoch; - HostEpoch(sdbusplus::bus::bus& bus, const char* objPath); - - /** - * @brief Get value of Elapsed property - * - * @return The elapsed microseconds since UTC - **/ - uint64_t elapsed() const override; - - /** - * @brief Set value of Elapsed property - * - * @param[in] value - The microseconds since UTC to set - * - * @return The updated elapsed microseconds since UTC - **/ - uint64_t elapsed(uint64_t value) override; - - /** @brief Notified on time owner changed */ - void onOwnerChanged(Owner owner) override; - - /** @brief Notified on bmc time is changed - * - * @param[in] bmcTime - The epoch time in microseconds - */ - void onBmcTimeChanged(const std::chrono::microseconds& bmcTime) override; - - private: - /** @brief The diff between BMC and Host time */ - std::chrono::microseconds offset; - - /** - * @brief The diff between host time and steady clock - * @details This diff is used to calculate the host time if BMC time - * is changed and the owner is SPLIT. - * Without this the host time is lost if BMC time is changed. - */ - std::chrono::microseconds diffToSteadyClock; - - /** @brief Save the offset value into offsetFile */ - void saveOffset(); - - /** @brief The file to store the offset in File System. - * Read back when starts - **/ - static constexpr auto offsetFile = HOST_OFFSET_FILE; -}; - -} // namespace time -} // namespace phosphor diff --git a/main.cpp b/main.cpp index 8344b38..1b94439 100644 --- a/main.cpp +++ b/main.cpp @@ -1,7 +1,6 @@ #include "config.h" #include "bmc_epoch.hpp" -#include "host_epoch.hpp" #include "manager.hpp" #include @@ -24,15 +23,11 @@ int main() // Add sdbusplus ObjectManager sdbusplus::server::manager::manager bmcEpochObjManager(bus, OBJPATH_BMC); - sdbusplus::server::manager::manager hostEpochObjManager(bus, OBJPATH_HOST); phosphor::time::Manager manager(bus); phosphor::time::BmcEpoch bmc(bus, OBJPATH_BMC); - phosphor::time::HostEpoch host(bus, OBJPATH_HOST); manager.addListener(&bmc); - manager.addListener(&host); - bmc.setBmcTimeChangeListener(&host); bus.request_name(BUSNAME); diff --git a/manager.cpp b/manager.cpp index 51bd5bf..92a7a3c 100644 --- a/manager.cpp +++ b/manager.cpp @@ -27,8 +27,7 @@ namespace time using namespace phosphor::logging; -const std::set Manager::managedProperties = {PROPERTY_TIME_MODE, - PROPERTY_TIME_OWNER}; +const std::set Manager::managedProperties = {PROPERTY_TIME_MODE}; Manager::Manager(sdbusplus::bus::bus& bus) : bus(bus), settings(bus) { @@ -38,10 +37,6 @@ Manager::Manager(sdbusplus::bus::bus& bus) : bus(bus), settings(bus) bus, propertiesChanged(settings.hostState, settings::hostStateIntf), std::bind(std::mem_fn(&Manager::onHostStateChanged), this, std::placeholders::_1)); - settingsMatches.emplace_back( - bus, propertiesChanged(settings.timeOwner, settings::timeOwnerIntf), - std::bind(std::mem_fn(&Manager::onSettingsChanged), this, - std::placeholders::_1)); settingsMatches.emplace_back( bus, propertiesChanged(settings.timeSyncMethod, settings::timeSyncIntf), std::bind(std::mem_fn(&Manager::onSettingsChanged), this, @@ -55,18 +50,14 @@ Manager::Manager(sdbusplus::bus::bus& bus) : bus(bus), settings(bus) // Check the settings daemon to process the new settings auto mode = getSetting(settings.timeSyncMethod.c_str(), settings::timeSyncIntf, PROPERTY_TIME_MODE); - auto owner = getSetting(settings.timeOwner.c_str(), settings::timeOwnerIntf, - PROPERTY_TIME_OWNER); onPropertyChanged(PROPERTY_TIME_MODE, mode); - onPropertyChanged(PROPERTY_TIME_OWNER, owner); } void Manager::addListener(PropertyChangeListner* listener) { // Notify listener about the initial value listener->onModeChanged(timeMode); - listener->onOwnerChanged(timeOwner); listeners.insert(listener); } @@ -78,11 +69,6 @@ void Manager::restoreSettings() { timeMode = utils::strToMode(mode); } - auto owner = utils::readData(ownerFile); - if (!owner.empty()) - { - timeOwner = utils::strToOwner(owner); - } } void Manager::checkHostOn() @@ -102,7 +88,7 @@ void Manager::onPropertyChanged(const std::string& key, { if (hostOn) { - // If host is on, set the values as requested time mode/owner. + // If host is on, set the values as requested time mode. // And when host becomes off, notify the listeners. setPropertyAsRequested(key, value); } @@ -114,11 +100,6 @@ void Manager::onPropertyChanged(const std::string& key, setCurrentTimeMode(value); onTimeModeChanged(value); } - else if (key == PROPERTY_TIME_OWNER) - { - setCurrentTimeOwner(value); - onTimeOwnerChanged(); - } } } @@ -149,10 +130,6 @@ void Manager::setPropertyAsRequested(const std::string& key, { setRequestedMode(value); } - else if (key == PROPERTY_TIME_OWNER) - { - setRequestedOwner(value); - } else { // The key shall be already the supported one @@ -170,11 +147,6 @@ void Manager::setRequestedMode(const std::string& mode) requestedMode = mode; } -void Manager::setRequestedOwner(const std::string& owner) -{ - requestedOwner = owner; -} - void Manager::updateNtpSetting(const std::string& value) { bool isNtp = @@ -238,14 +210,6 @@ void Manager::onHostState(bool on) } setRequestedMode({}); // Clear requested mode } - if (!requestedOwner.empty()) - { - if (setCurrentTimeOwner(requestedOwner)) - { - onTimeOwnerChanged(); - } - setRequestedOwner({}); // Clear requested owner - } } bool Manager::setCurrentTimeMode(const std::string& mode) @@ -265,23 +229,6 @@ bool Manager::setCurrentTimeMode(const std::string& mode) } } -bool Manager::setCurrentTimeOwner(const std::string& owner) -{ - auto newOwner = utils::strToOwner(owner); - if (newOwner != timeOwner) - { - log("Time owner is changed", - entry("OWNER=%s", owner.c_str())); - timeOwner = newOwner; - utils::writeData(ownerFile, owner); - return true; - } - else - { - return false; - } -} - void Manager::onTimeModeChanged(const std::string& mode) { for (const auto listener : listeners) @@ -292,14 +239,6 @@ void Manager::onTimeModeChanged(const std::string& mode) updateNtpSetting(mode); } -void Manager::onTimeOwnerChanged() -{ - for (const auto& listener : listeners) - { - listener->onOwnerChanged(timeOwner); - } -} - std::string Manager::getSetting(const char* path, const char* interface, const char* setting) const { diff --git a/manager.hpp b/manager.hpp index 0c672fa..2d5d131 100644 --- a/manager.hpp +++ b/manager.hpp @@ -61,15 +61,9 @@ class Manager /** @brief The requested time mode when host is on*/ std::string requestedMode; - /** @brief The requested time owner when host is on*/ - std::string requestedOwner; - /** @brief The current time mode */ Mode timeMode = DEFAULT_TIME_MODE; - /** @brief The current time owner */ - Owner timeOwner = DEFAULT_TIME_OWNER; - /** @brief Restore saved settings */ void restoreSettings(); @@ -96,15 +90,6 @@ class Manager */ bool setCurrentTimeMode(const std::string& mode); - /** @brief Set current time owner from the time owner string - * - * @param[in] owner - The string of time owner - * - * @return - true if the owner is updated - * false if it's the same as before - */ - bool setCurrentTimeOwner(const std::string& owner); - /** @brief Called on time mode is changed * * Notify listeners that time mode is changed and update ntp setting @@ -113,12 +98,6 @@ class Manager */ void onTimeModeChanged(const std::string& mode); - /** @brief Called on time owner is changed - * - * Notify listeners that time owner is changed - */ - void onTimeOwnerChanged(); - /** @brief Callback to handle change in a setting * * @param[in] msg - sdbusplus dbusmessage @@ -146,7 +125,7 @@ class Manager */ void onHostState(bool on); - /** @brief Set the property as requested time mode/owner + /** @brief Set the property as requested time mode * * @param[in] key - The property name * @param[in] value - The property value @@ -161,13 +140,6 @@ class Manager */ void setRequestedMode(const std::string& mode); - /** @brief Set the current owner to user requested one - * if conditions allow it - * - * @param[in] owner - The string of time owner - */ - void setRequestedOwner(const std::string& owner); - /** @brief Update the NTP setting to systemd time service * * @param[in] value - The time mode value, e.g. "NTP" or "MANUAL" @@ -186,9 +158,6 @@ class Manager /** @brief The string of time mode property */ static constexpr auto PROPERTY_TIME_MODE = "TimeSyncMethod"; - /** @brief The string of time owner property */ - static constexpr auto PROPERTY_TIME_OWNER = "TimeOwner"; - using Updater = std::function; /** @brief Map the property string to functions that shall @@ -196,23 +165,15 @@ class Manager */ const std::map propertyUpdaters = { {PROPERTY_TIME_MODE, - std::bind(&Manager::setCurrentTimeMode, this, std::placeholders::_1)}, - {PROPERTY_TIME_OWNER, std::bind(&Manager::setCurrentTimeOwner, this, - std::placeholders::_1)}}; + std::bind(&Manager::setCurrentTimeMode, this, std::placeholders::_1)}}; /** @brief The properties that manager shall notify the * listeners when changed */ static const std::set managedProperties; - /** @brief The map that maps the string to Owners */ - static const std::map ownerMap; - /** @brief The file name of saved time mode */ static constexpr auto modeFile = "/var/lib/obmc/saved_time_mode"; - - /** @brief The file name of saved time owner */ - static constexpr auto ownerFile = "/var/lib/obmc/saved_time_owner"; }; } // namespace time diff --git a/property_change_listener.hpp b/property_change_listener.hpp index a19a228..d34fcf0 100644 --- a/property_change_listener.hpp +++ b/property_change_listener.hpp @@ -16,9 +16,6 @@ class PropertyChangeListner /** @brief Notified on time mode is changed */ virtual void onModeChanged(Mode mode) = 0; - - /** @brief Notified on time owner is changed */ - virtual void onOwnerChanged(Owner owner) = 0; }; } // namespace time diff --git a/settings.cpp b/settings.cpp index 9add59d..61a575b 100644 --- a/settings.cpp +++ b/settings.cpp @@ -17,8 +17,7 @@ constexpr auto mapperIntf = "xyz.openbmc_project.ObjectMapper"; Objects::Objects(sdbusplus::bus::bus& bus) : bus(bus) { - std::vector settingsIntfs = {timeOwnerIntf, timeSyncIntf, - hostStateIntf}; + std::vector settingsIntfs = {timeSyncIntf, hostStateIntf}; auto depth = 0; auto mapperCall = bus.new_method_call(mapperService, mapperPath, mapperIntf, @@ -51,11 +50,7 @@ Objects::Objects(sdbusplus::bus::bus& bus) : bus(bus) { for (const Interface& interface : service_iter.second) { - if (timeOwnerIntf == interface) - { - timeOwner = path; - } - else if (timeSyncIntf == interface) + if (timeSyncIntf == interface) { timeSyncMethod = path; } diff --git a/settings.hpp b/settings.hpp index b4893cb..12209c0 100644 --- a/settings.hpp +++ b/settings.hpp @@ -11,7 +11,6 @@ using Service = std::string; using Interface = std::string; constexpr auto root = "/"; -constexpr auto timeOwnerIntf = "xyz.openbmc_project.Time.Owner"; constexpr auto timeSyncIntf = "xyz.openbmc_project.Time.Synchronization"; constexpr auto hostStateIntf = "xyz.openbmc_project.State.Host"; @@ -44,9 +43,6 @@ struct Objects */ Service service(const Path& path, const Interface& interface) const; - /** @brief time owner settings object */ - Path timeOwner; - /** @brief time sync method settings object */ Path timeSyncMethod; diff --git a/test/Makefile.am b/test/Makefile.am index a8cd3b7..e0457cb 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -8,7 +8,6 @@ check_PROGRAMS += test test_SOURCES = \ TestEpochBase.cpp \ TestBmcEpoch.cpp \ - TestHostEpoch.cpp \ TestManager.cpp \ TestUtils.cpp diff --git a/test/TestBmcEpoch.cpp b/test/TestBmcEpoch.cpp index eb116e5..db43854 100644 --- a/test/TestBmcEpoch.cpp +++ b/test/TestBmcEpoch.cpp @@ -1,12 +1,10 @@ #include "config.h" #include "bmc_epoch.hpp" -#include "mocked_bmc_time_change_listener.hpp" #include "types.hpp" #include #include -#include #include @@ -15,16 +13,13 @@ namespace phosphor namespace time { -using ::testing::_; using namespace std::chrono; -using NotAllowed = sdbusplus::xyz::openbmc_project::Time::Error::NotAllowed; class TestBmcEpoch : public testing::Test { public: sdbusplus::bus::bus bus; sd_event* event; - MockBmcTimeChangeListener listener; std::unique_ptr bmcEpoch; TestBmcEpoch() : bus(sdbusplus::bus::new_default()) @@ -33,7 +28,6 @@ class TestBmcEpoch : public testing::Test sd_event_default(&event); bus.attach_event(event, SD_EVENT_PRIORITY_NORMAL); bmcEpoch = std::make_unique(bus, OBJPATH_BMC); - bmcEpoch->setBmcTimeChangeListener(&listener); } ~TestBmcEpoch() @@ -47,14 +41,6 @@ class TestBmcEpoch : public testing::Test { return bmcEpoch->timeMode; } - Owner getTimeOwner() - { - return bmcEpoch->timeOwner; - } - void setTimeOwner(Owner owner) - { - bmcEpoch->timeOwner = owner; - } void setTimeMode(Mode mode) { bmcEpoch->timeMode = mode; @@ -67,9 +53,8 @@ class TestBmcEpoch : public testing::Test TEST_F(TestBmcEpoch, empty) { - // Default mode/owner is MANUAL/BOTH + // Default mode is MANUAL EXPECT_EQ(Mode::Manual, getTimeMode()); - EXPECT_EQ(Owner::Both, getTimeOwner()); } TEST_F(TestBmcEpoch, getElapsed) @@ -80,18 +65,6 @@ TEST_F(TestBmcEpoch, getElapsed) EXPECT_GE(t2, t1); } -TEST_F(TestBmcEpoch, setElapsedNotAllowed) -{ - auto epochNow = - duration_cast(system_clock::now().time_since_epoch()) - .count(); - - // In Host owner, setting time is not allowed - setTimeMode(Mode::Manual); - setTimeOwner(Owner::Host); - EXPECT_THROW(bmcEpoch->elapsed(epochNow), NotAllowed); -} - TEST_F(TestBmcEpoch, setElapsedOK) { // TODO: setting time will call sd-bus functions and it will fail on host @@ -99,12 +72,5 @@ TEST_F(TestBmcEpoch, setElapsedOK) // But for now we can not test it } -TEST_F(TestBmcEpoch, onTimeChange) -{ - // On BMC time change, the listner is expected to be notified - EXPECT_CALL(listener, onBmcTimeChanged(_)).Times(1); - triggerTimeChange(); -} - } // namespace time } // namespace phosphor diff --git a/test/TestEpochBase.cpp b/test/TestEpochBase.cpp index b193969..d3d1b69 100644 --- a/test/TestEpochBase.cpp +++ b/test/TestEpochBase.cpp @@ -25,10 +25,6 @@ class TestEpochBase : public testing::Test { return epochBase.timeMode; } - Owner getOwner() - { - return epochBase.timeOwner; - } }; TEST_F(TestEpochBase, onModeChange) @@ -40,20 +36,5 @@ TEST_F(TestEpochBase, onModeChange) EXPECT_EQ(Mode::Manual, getMode()); } -TEST_F(TestEpochBase, onOwnerChange) -{ - epochBase.onOwnerChanged(Owner::BMC); - EXPECT_EQ(Owner::BMC, getOwner()); - - epochBase.onOwnerChanged(Owner::Host); - EXPECT_EQ(Owner::Host, getOwner()); - - epochBase.onOwnerChanged(Owner::Split); - EXPECT_EQ(Owner::Split, getOwner()); - - epochBase.onOwnerChanged(Owner::Both); - EXPECT_EQ(Owner::Both, getOwner()); -} - } // namespace time } // namespace phosphor diff --git a/test/TestHostEpoch.cpp b/test/TestHostEpoch.cpp deleted file mode 100644 index 7ddf92f..0000000 --- a/test/TestHostEpoch.cpp +++ /dev/null @@ -1,298 +0,0 @@ -#include "config.h" - -#include "host_epoch.hpp" -#include "types.hpp" -#include "utils.hpp" - -#include -#include - -#include - -namespace phosphor -{ -namespace time -{ - -using namespace std::chrono; -using namespace std::chrono_literals; -using NotAllowed = sdbusplus::xyz::openbmc_project::Time::Error::NotAllowed; - -const constexpr microseconds USEC_ZERO{0}; - -class TestHostEpoch : public testing::Test -{ - public: - sdbusplus::bus::bus bus; - HostEpoch hostEpoch; - - static constexpr auto FILE_NOT_EXIST = "path/to/file-not-exist"; - static constexpr auto FILE_OFFSET = "saved_host_offset"; - const microseconds delta = 2s; - - TestHostEpoch() : - bus(sdbusplus::bus::new_default()), hostEpoch(bus, OBJPATH_HOST) - { - // Make sure the file does not exist - std::remove(FILE_NOT_EXIST); - } - ~TestHostEpoch() - { - // Cleanup test file - std::remove(FILE_OFFSET); - } - - // Proxies for HostEpoch's private members and functions - Mode getTimeMode() - { - return hostEpoch.timeMode; - } - Owner getTimeOwner() - { - return hostEpoch.timeOwner; - } - microseconds getOffset() - { - return hostEpoch.offset; - } - void setOffset(microseconds us) - { - hostEpoch.offset = us; - } - void setTimeOwner(Owner owner) - { - hostEpoch.onOwnerChanged(owner); - } - void setTimeMode(Mode mode) - { - hostEpoch.onModeChanged(mode); - } - - void checkSettingTimeNotAllowed() - { - // By default offset shall be 0 - EXPECT_EQ(0, getOffset().count()); - - // Set time is not allowed, - // so verify offset is still 0 after set time - microseconds diff = 1min; - EXPECT_THROW(hostEpoch.elapsed(hostEpoch.elapsed() + diff.count()), - NotAllowed); - EXPECT_EQ(0, getOffset().count()); - } - - void checkSetSplitTimeInFuture() - { - // Get current time, and set future +1min time - auto t1 = hostEpoch.elapsed(); - EXPECT_NE(0, t1); - microseconds diff = 1min; - auto t2 = t1 + diff.count(); - hostEpoch.elapsed(t2); - - // Verify that the offset shall be positive, - // and less or equal to diff, and shall be not too less. - auto offset = getOffset(); - EXPECT_GT(offset, USEC_ZERO); - EXPECT_LE(offset, diff); - diff -= delta; - EXPECT_GE(offset, diff); - - // Now get time shall be around future +1min time - auto epochNow = - duration_cast(system_clock::now().time_since_epoch()) - .count(); - auto elapsedGot = hostEpoch.elapsed(); - EXPECT_LT(epochNow, elapsedGot); - auto epochDiff = elapsedGot - epochNow; - diff = 1min; - EXPECT_GT(epochDiff, (diff - delta).count()); - EXPECT_LT(epochDiff, (diff + delta).count()); - } - void checkSetSplitTimeInPast() - { - // Get current time, and set past -1min time - auto t1 = hostEpoch.elapsed(); - EXPECT_NE(0, t1); - microseconds diff = 1min; - auto t2 = t1 - diff.count(); - hostEpoch.elapsed(t2); - - // Verify that the offset shall be negative, and the absolute value - // shall be equal or greater than diff, and shall not be too greater - auto offset = getOffset(); - EXPECT_LT(offset, USEC_ZERO); - offset = -offset; - EXPECT_GE(offset, diff); - diff += 10s; - EXPECT_LE(offset, diff); - - // Now get time shall be around past -1min time - auto epochNow = - duration_cast(system_clock::now().time_since_epoch()) - .count(); - auto elapsedGot = hostEpoch.elapsed(); - EXPECT_LT(elapsedGot, epochNow); - auto epochDiff = epochNow - elapsedGot; - diff = 1min; - EXPECT_GT(epochDiff, (diff - delta).count()); - EXPECT_LT(epochDiff, (diff + delta).count()); - } -}; - -TEST_F(TestHostEpoch, empty) -{ - // Default mode/owner is MANUAL/BOTH - EXPECT_EQ(Mode::Manual, getTimeMode()); - EXPECT_EQ(Owner::Both, getTimeOwner()); -} - -TEST_F(TestHostEpoch, readDataFileNotExist) -{ - // When file does not exist, the default offset shall be 0 - microseconds offset(0); - auto value = utils::readData(FILE_NOT_EXIST); - EXPECT_EQ(0, value); -} - -TEST_F(TestHostEpoch, writeAndReadData) -{ - // Write offset to file - microseconds offsetToWrite(1234567); - utils::writeData(FILE_OFFSET, - offsetToWrite.count()); - - // Read it back - microseconds offsetToRead; - offsetToRead = - microseconds(utils::readData(FILE_OFFSET)); - EXPECT_EQ(offsetToWrite, offsetToRead); -} - -TEST_F(TestHostEpoch, setElapsedInNtpBmc) -{ - // Set time in NTP/BMC is not allowed - setTimeMode(Mode::NTP); - setTimeOwner(Owner::BMC); - checkSettingTimeNotAllowed(); -} - -TEST_F(TestHostEpoch, setElapsedInNtpHost) -{ - // Set time in NTP/HOST is not allowed - setTimeMode(Mode::NTP); - setTimeOwner(Owner::Host); - checkSettingTimeNotAllowed(); -} - -TEST_F(TestHostEpoch, setElapsedInNtpSplit) -{ - // Set time in NTP/SPLIT, offset will be set - setTimeMode(Mode::NTP); - setTimeOwner(Owner::Split); - - checkSetSplitTimeInFuture(); - - // Reset offset - setOffset(USEC_ZERO); - checkSetSplitTimeInPast(); -} - -TEST_F(TestHostEpoch, setElapsedInNtpBoth) -{ - // Set time in NTP/BOTH is not allowed - setTimeMode(Mode::NTP); - setTimeOwner(Owner::Both); - checkSettingTimeNotAllowed(); -} - -TEST_F(TestHostEpoch, setElapsedInManualBmc) -{ - // Set time in MANUAL/BMC is not allowed - setTimeMode(Mode::Manual); - setTimeOwner(Owner::BMC); - checkSettingTimeNotAllowed(); -} - -TEST_F(TestHostEpoch, setElapsedInManualHost) -{ - // Set time in MANUAL/HOST, time will be set to BMC - // However it requies gmock to test this case - // TODO: when gmock is ready, test this case. - setTimeMode(Mode::Manual); - setTimeOwner(Owner::Host); -} - -TEST_F(TestHostEpoch, setElapsedInManualSplit) -{ - // Set to SPLIT owner so that offset will be set - setTimeMode(Mode::Manual); - setTimeOwner(Owner::Split); - - checkSetSplitTimeInFuture(); - - // Reset offset - setOffset(USEC_ZERO); - checkSetSplitTimeInPast(); -} - -TEST_F(TestHostEpoch, setElapsedInManualBoth) -{ - // Set time in MANUAL/BOTH, time will be set to BMC - // However it requies gmock to test this case - // TODO: when gmock is ready, test this case. - setTimeMode(Mode::Manual); - setTimeOwner(Owner::Both); -} - -TEST_F(TestHostEpoch, setElapsedInSplitAndBmcTimeIsChanged) -{ - // Set to SPLIT owner so that offset will be set - setTimeOwner(Owner::Split); - - // Get current time, and set future +1min time - auto t1 = hostEpoch.elapsed(); - EXPECT_NE(0, t1); - microseconds diff = 1min; - auto t2 = t1 + diff.count(); - hostEpoch.elapsed(t2); - - // Verify that the offset shall be positive, - // and less or equal to diff, and shall be not too less. - auto offset = getOffset(); - EXPECT_GT(offset, USEC_ZERO); - EXPECT_LE(offset, diff); - diff -= delta; - EXPECT_GE(offset, diff); - - // Now BMC time is changed to future +1min - hostEpoch.onBmcTimeChanged(microseconds(t2)); - - // Verify that the offset shall be around zero since it's almost - // the same as BMC time - offset = getOffset(); - if (offset.count() < 0) - { - offset = microseconds(-offset.count()); - } - EXPECT_LE(offset, delta); -} - -TEST_F(TestHostEpoch, clearOffsetOnOwnerChange) -{ - EXPECT_EQ(USEC_ZERO, getOffset()); - - setTimeOwner(Owner::Split); - hostEpoch.onBmcTimeChanged(microseconds(hostEpoch.elapsed()) + 1min); - - // Now offset shall be non zero - EXPECT_NE(USEC_ZERO, getOffset()); - - setTimeOwner(Owner::Both); - - // Now owner is BOTH, the offset shall be cleared - EXPECT_EQ(USEC_ZERO, getOffset()); -} - -} // namespace time -} // namespace phosphor diff --git a/test/TestManager.cpp b/test/TestManager.cpp index c0763c8..95f5734 100644 --- a/test/TestManager.cpp +++ b/test/TestManager.cpp @@ -34,10 +34,6 @@ class TestManager : public testing::Test { return manager.timeMode; } - Owner getTimeOwner() - { - return manager.timeOwner; - } bool hostOn() { return manager.hostOn; @@ -46,10 +42,6 @@ class TestManager : public testing::Test { return manager.requestedMode; } - std::string getRequestedOwner() - { - return manager.requestedOwner; - } void notifyPropertyChanged(const std::string& key, const std::string& value) { manager.onPropertyChanged(key, value); @@ -64,11 +56,9 @@ TEST_F(TestManager, DISABLED_empty) { EXPECT_FALSE(hostOn()); EXPECT_EQ("", getRequestedMode()); - EXPECT_EQ("", getRequestedOwner()); - // Default mode/owner is MANUAL/BOTH + // Default mode is MANUAL EXPECT_EQ(Mode::Manual, getTimeMode()); - EXPECT_EQ(Owner::Both, getTimeOwner()); } TEST_F(TestManager, DISABLED_hostStateChange) @@ -86,50 +76,36 @@ TEST_F(TestManager, DISABLED_propertyChanged) // Check mocked listeners shall receive notifications on property changed EXPECT_CALL(listener1, onModeChanged(Mode::Manual)).Times(1); - EXPECT_CALL(listener1, onOwnerChanged(Owner::Host)).Times(1); EXPECT_CALL(listener2, onModeChanged(Mode::Manual)).Times(1); - EXPECT_CALL(listener2, onOwnerChanged(Owner::Host)).Times(1); notifyPropertyChanged( "TimeSyncMethod", "xyz.openbmc_project.Time.Synchronization.Method.Manual"); - notifyPropertyChanged("TimeOwner", - "xyz.openbmc_project.Time.Owner.Owners.Host"); EXPECT_EQ("", getRequestedMode()); - EXPECT_EQ("", getRequestedOwner()); // When host is on, property changes are saved as requested ones notifyOnHostState(true); // Check mocked listeners shall not receive notifications EXPECT_CALL(listener1, onModeChanged(Mode::Manual)).Times(0); - EXPECT_CALL(listener1, onOwnerChanged(Owner::Host)).Times(0); EXPECT_CALL(listener2, onModeChanged(Mode::Manual)).Times(0); - EXPECT_CALL(listener2, onOwnerChanged(Owner::Host)).Times(0); notifyPropertyChanged( "TimeSyncMethod", "xyz.openbmc_project.Time.Synchronization.Method.NTP"); - notifyPropertyChanged("TimeOwner", - "xyz.openbmc_project.Time.Owner.Owners.Split"); EXPECT_EQ("xyz.openbmc_project.Time.Synchronization.Method.NTP", getRequestedMode()); - EXPECT_EQ("xyz.openbmc_project.Time.Owner.Owners.Split", - getRequestedOwner()); - // When host becomes off, the requested mode/owner shall be notified + // When host becomes off, the requested mode shall be notified // to listeners, and be cleared EXPECT_CALL(listener1, onModeChanged(Mode::NTP)).Times(1); - EXPECT_CALL(listener1, onOwnerChanged(Owner::Split)).Times(1); EXPECT_CALL(listener2, onModeChanged(Mode::NTP)).Times(1); - EXPECT_CALL(listener2, onOwnerChanged(Owner::Split)).Times(1); notifyOnHostState(false); EXPECT_EQ("", getRequestedMode()); - EXPECT_EQ("", getRequestedOwner()); // When host is on, and invalid property is changed, // verify the code asserts because it shall never occur @@ -143,55 +119,40 @@ TEST_F(TestManager, DISABLED_propertyChangedAndChangedbackWhenHostOn) notifyPropertyChanged( "TimeSyncMethod", "xyz.openbmc_project.Time.Synchronization.Method.Manual"); - notifyPropertyChanged("TimeOwner", - "xyz.openbmc_project.Time.Owner.Owners.Host"); // Set host on notifyOnHostState(true); // Check mocked listeners shall not receive notifications EXPECT_CALL(listener1, onModeChanged(_)).Times(0); - EXPECT_CALL(listener1, onOwnerChanged(_)).Times(0); EXPECT_CALL(listener2, onModeChanged(_)).Times(0); - EXPECT_CALL(listener2, onOwnerChanged(_)).Times(0); notifyPropertyChanged( "TimeSyncMethod", "xyz.openbmc_project.Time.Synchronization.Method.NTP"); - notifyPropertyChanged("TimeOwner", - "xyz.openbmc_project.Time.Owner.Owners.Split"); - // Saved as requested mode/owner + // Saved as requested mode EXPECT_EQ("xyz.openbmc_project.Time.Synchronization.Method.NTP", getRequestedMode()); - EXPECT_EQ("xyz.openbmc_project.Time.Owner.Owners.Split", - getRequestedOwner()); // Property changed back to MANUAL/HOST notifyPropertyChanged( "TimeSyncMethod", "xyz.openbmc_project.Time.Synchronization.Method.Manual"); - notifyPropertyChanged("TimeOwner", - "xyz.openbmc_project.Time.Owner.Owners.Host"); - // Requested mode/owner shall be updated + // Requested mode shall be updated EXPECT_EQ("xyz.openbmc_project.Time.Synchronization.Method.Manual", getRequestedMode()); - EXPECT_EQ("xyz.openbmc_project.Time.Owner.Owners.Host", - getRequestedOwner()); - // Because the latest mode/owner is the same as when host is off, - // The listeners shall not be notified, and requested mode/owner + // Because the latest mode is the same as when host is off, + // The listeners shall not be notified, and requested mode // shall be cleared EXPECT_CALL(listener1, onModeChanged(_)).Times(0); - EXPECT_CALL(listener1, onOwnerChanged(_)).Times(0); EXPECT_CALL(listener2, onModeChanged(_)).Times(0); - EXPECT_CALL(listener2, onOwnerChanged(_)).Times(0); notifyOnHostState(false); EXPECT_EQ("", getRequestedMode()); - EXPECT_EQ("", getRequestedOwner()); } // TODO: if gmock is ready, add case to test diff --git a/test/TestUtils.cpp b/test/TestUtils.cpp index 1cbc852..3d38171 100644 --- a/test/TestUtils.cpp +++ b/test/TestUtils.cpp @@ -29,23 +29,6 @@ TEST(TestUtil, strToMode) EXPECT_THROW(strToMode("whatever"), InvalidEnumString); } -TEST(TestUtil, strToOwner) -{ - EXPECT_EQ(Owner::BMC, - strToOwner("xyz.openbmc_project.Time.Owner.Owners.BMC")); - EXPECT_EQ(Owner::Host, - strToOwner("xyz.openbmc_project.Time.Owner.Owners.Host")); - EXPECT_EQ(Owner::Split, - strToOwner("xyz.openbmc_project.Time.Owner.Owners.Split")); - EXPECT_EQ(Owner::Both, - strToOwner("xyz.openbmc_project.Time.Owner.Owners.Both")); - - // All unrecognized strings result in InvalidEnumString exception - EXPECT_THROW(strToOwner(""), InvalidEnumString); - EXPECT_THROW(strToOwner("Split"), InvalidEnumString); - EXPECT_THROW(strToOwner("xyz"), InvalidEnumString); -} - TEST(TestUtil, modeToStr) { EXPECT_EQ("xyz.openbmc_project.Time.Synchronization.Method.NTP", @@ -57,21 +40,6 @@ TEST(TestUtil, modeToStr) EXPECT_ANY_THROW(modeToStr(static_cast(100))); } -TEST(TestUtil, ownerToStr) -{ - EXPECT_EQ("xyz.openbmc_project.Time.Owner.Owners.BMC", - ownerToStr(Owner::BMC)); - EXPECT_EQ("xyz.openbmc_project.Time.Owner.Owners.Host", - ownerToStr(Owner::Host)); - EXPECT_EQ("xyz.openbmc_project.Time.Owner.Owners.Split", - ownerToStr(Owner::Split)); - EXPECT_EQ("xyz.openbmc_project.Time.Owner.Owners.Both", - ownerToStr(Owner::Both)); - - // All unrecognized strings result in exception - EXPECT_ANY_THROW(ownerToStr(static_cast(100))); -} - } // namespace utils } // namespace time } // namespace phosphor diff --git a/test/mocked_bmc_time_change_listener.hpp b/test/mocked_bmc_time_change_listener.hpp deleted file mode 100644 index 364b333..0000000 --- a/test/mocked_bmc_time_change_listener.hpp +++ /dev/null @@ -1,18 +0,0 @@ -#pragma once -#include "bmc_time_change_listener.hpp" - -#include - -namespace phosphor -{ -namespace time -{ - -class MockBmcTimeChangeListener : public BmcTimeChangeListener -{ - public: - MOCK_METHOD1(onBmcTimeChanged, void(const std::chrono::microseconds&)); -}; - -} // namespace time -} // namespace phosphor diff --git a/test/mocked_property_change_listener.hpp b/test/mocked_property_change_listener.hpp index 5d7a3b5..b5d6511 100644 --- a/test/mocked_property_change_listener.hpp +++ b/test/mocked_property_change_listener.hpp @@ -12,7 +12,6 @@ class MockPropertyChangeListner : public PropertyChangeListner { public: MOCK_METHOD1(onModeChanged, void(Mode mode)); - MOCK_METHOD1(onOwnerChanged, void(Owner owner)); }; } // namespace time diff --git a/types.hpp b/types.hpp index 0e78432..0f9f57c 100644 --- a/types.hpp +++ b/types.hpp @@ -1,6 +1,5 @@ #pragma once -#include #include namespace phosphor @@ -11,38 +10,10 @@ namespace time using ModeSetting = sdbusplus::xyz::openbmc_project::Time::server::Synchronization; -/** @brief Alias to time owner class */ -using OwnerSetting = sdbusplus::xyz::openbmc_project::Time::server::Owner; - /** @brief Supported time sync modes * NTP Time sourced by Network Time Server * Manual User of the system need to set the time */ using Mode = ModeSetting::Method; - -/** @brief Supported time owners - * BMC Time source may be NTP or Manual but it has to be set natively - * on the BMC. Meaning, host can not set the time. What it also - * means is that when BMC gets IPMI_SET_SEL_TIME, then its ignored. - * similarly, when BMC gets IPMI_GET_SEL_TIME, then the BMC's time - * is returned. - * - * Host Its only IPMI_SEL_SEL_TIME that will set the time on BMC. - * Meaning, IPMI_GET_SEL_TIME and request to get BMC time will - * result in same value. - * - * Split Both BMC and Host will maintain their individual clocks but then - * the time information is stored in BMC. BMC can have either NTP - * or Manual as it's source of time and will set the time directly - * on the BMC. When IPMI_SET_SEL_TIME is received, then the delta - * between that and BMC's time is calculated and is stored. - * When BMC reads the time, the current time is returned. - * When IPMI_GET_SEL_TIME is received, BMC's time is retrieved and - * then the delta offset is factored in prior to returning. - * - * Both: BMC's time is set with whoever that sets the time. Similarly, - * BMC's time is returned to whoever that asks the time. - */ -using Owner = OwnerSetting::Owners; } // namespace time } // namespace phosphor diff --git a/utils.cpp b/utils.cpp index 4df1910..3ea05a7 100644 --- a/utils.cpp +++ b/utils.cpp @@ -58,23 +58,12 @@ Mode strToMode(const std::string& mode) return ModeSetting::convertMethodFromString(mode); } -Owner strToOwner(const std::string& owner) -{ - return OwnerSetting::convertOwnersFromString(owner); -} - std::string modeToStr(Mode mode) { return sdbusplus::xyz::openbmc_project::Time::server::convertForMessage( mode); } -std::string ownerToStr(Owner owner) -{ - return sdbusplus::xyz::openbmc_project::Time::server::convertForMessage( - owner); -} - } // namespace utils } // namespace time } // namespace phosphor diff --git a/utils.hpp b/utils.hpp index 0e30fa2..0ec55f3 100644 --- a/utils.hpp +++ b/utils.hpp @@ -107,23 +107,6 @@ std::string getService(sdbusplus::bus::bus& bus, const char* path, */ Mode strToMode(const std::string& mode); -/** @brief Convert a string to enum Owner - * - * Convert the time owner string to enum. - * Valid strings are - * "xyz.openbmc_project.Time.Owner.Owners.BMC" - * "xyz.openbmc_project.Time.Owner.Owners.Host" - * "xyz.openbmc_project.Time.Owner.Owners.Both" - * "xyz.openbmc_project.Time.Owner.Owners.Split" - * If it's not a valid time owner string, it means something - * goes wrong so raise exception. - * - * @param[in] owner - The string of time owner - * - * @return The Owner enum - */ -Owner strToOwner(const std::string& owner); - /** @brief Convert a mode enum to mode string * * @param[in] mode - The Mode enum @@ -132,14 +115,6 @@ Owner strToOwner(const std::string& owner); */ std::string modeToStr(Mode mode); -/** @brief Convert a owner enum to owner string - * - * @param[in] owner - The Owner enum - * - * @return The string of the owner - */ -std::string ownerToStr(Owner owner); - } // namespace utils } // namespace time } // namespace phosphor