Save properties to persistent storage when host is on
1. When host is on, set properties as requested properties instead
of notify listeners;
2. When host becomes off, and requested properties are not empty, notify
the listners and reset the requested properties.
Add unit tests.
Change-Id: I9359c801c698df0c6e5eab43e12427bb5a6da611
Signed-off-by: Lei YU <mine260309@gmail.com>
diff --git a/host_epoch.cpp b/host_epoch.cpp
index f45581d..3b4c00e 100644
--- a/host_epoch.cpp
+++ b/host_epoch.cpp
@@ -1,9 +1,8 @@
#include "host_epoch.hpp"
+#include "utils.hpp"
#include <phosphor-logging/log.hpp>
-#include <fstream>
-
namespace phosphor
{
namespace time
@@ -14,7 +13,7 @@
HostEpoch::HostEpoch(sdbusplus::bus::bus& bus,
const char* objPath)
: EpochBase(bus, objPath),
- offset(readData<decltype(offset)::rep>(offsetFile))
+ offset(utils::readData<decltype(offset)::rep>(offsetFile))
{
// Empty
}
@@ -41,34 +40,12 @@
offset = time - getTime();
// Store the offset to file
- writeData(offsetFile, offset.count());
+ utils::writeData(offsetFile, offset.count());
server::EpochTime::elapsed(value);
return value;
}
-template <typename T>
-T HostEpoch::readData(const char* fileName)
-{
- T data{};
- std::ifstream fs(fileName);
- if (fs.is_open())
- {
- fs >> data;
- }
- return data;
-}
-
-template <typename T>
-void HostEpoch::writeData(const char* fileName, T&& data)
-{
- std::ofstream fs(fileName, std::ios::out);
- if (fs.is_open())
- {
- fs << std::forward<T>(data);
- }
-}
-
} // namespace time
} // namespace phosphor
diff --git a/host_epoch.hpp b/host_epoch.hpp
index 1252ff9..3798f8f 100644
--- a/host_epoch.hpp
+++ b/host_epoch.hpp
@@ -46,23 +46,6 @@
* Read back when starts
**/
static constexpr auto offsetFile = HOST_OFFSET_FILE;
-
- /** @brief Read data with type T from file
- *
- * @param[in] fileName - The name of file to read from
- *
- * @return The data with type T
- */
- template <typename T>
- static T readData(const char* fileName);
-
- /** @brief Write data with type T to file
- *
- * @param[in] fileName - The name of file to write to
- * @param[in] data - The data with type T to write to file
- */
- template <typename T>
- static void writeData(const char* fileName, T&& data);
};
} // namespace time
diff --git a/manager.cpp b/manager.cpp
index 5a3d928..9810e14 100644
--- a/manager.cpp
+++ b/manager.cpp
@@ -1,4 +1,5 @@
#include "manager.hpp"
+#include "utils.hpp"
#include <phosphor-logging/log.hpp>
@@ -12,9 +13,6 @@
constexpr auto PROPERTY_INTERFACE = "org.freedesktop.DBus.Properties";
constexpr auto METHOD_GET = "Get";
-constexpr auto PROPERTY_TIME_MODE = "time_mode";
-constexpr auto PROPERTY_TIME_OWNER = "time_owner";
-
// TODO: Use new settings in xyz.openbmc_project
const auto MATCH_PROPERTY_CHANGE =
rules::type::signal() +
@@ -58,9 +56,14 @@
propertyChangeMatch(bus, MATCH_PROPERTY_CHANGE, onPropertyChanged, this),
pgoodChangeMatch(bus, MATCH_PGOOD_CHANGE, onPgoodChanged, this)
{
- setCurrentTimeMode(getSettings(PROPERTY_TIME_MODE));
- setCurrentTimeOwner(getSettings(PROPERTY_TIME_OWNER));
checkHostOn();
+
+ // Restore settings from persistent storage
+ restoreSettings();
+
+ // Check the settings daemon to process the new settings
+ onPropertyChanged(PROPERTY_TIME_MODE, getSettings(PROPERTY_TIME_MODE));
+ onPropertyChanged(PROPERTY_TIME_OWNER, getSettings(PROPERTY_TIME_OWNER));
}
void Manager::addListener(PropertyChangeListner* listener)
@@ -72,6 +75,20 @@
listeners.insert(listener);
}
+void Manager::restoreSettings()
+{
+ auto mode = utils::readData<std::string>(modeFile);
+ if (!mode.empty())
+ {
+ timeMode = convertToMode(mode);
+ }
+ auto owner = utils::readData<std::string>(ownerFile);
+ if (!owner.empty())
+ {
+ timeOwner = convertToOwner(owner);
+ }
+}
+
void Manager::checkHostOn()
{
sdbusplus::message::variant<int> pgood = 0;
@@ -92,28 +109,32 @@
void Manager::onPropertyChanged(const std::string& key,
const std::string& value)
{
- // TODO: Check pgood
- // If it's off, notify listners;
- // If it's on, hold the values and store in persistent storage
- // as requested time mode/owner.
- // And when pgood turns back to off, notify the listners.
-
// TODO: Check dhcp_ntp
- if (key == PROPERTY_TIME_MODE)
+ if (hostOn)
{
- setCurrentTimeMode(value);
- for (const auto& listener : listeners)
- {
- listener->onModeChanged(timeMode);
- }
+ // If host is on, set the values as requested time mode/owner.
+ // And when host becomes off, notify the listners.
+ setPropertyAsRequested(key, value);
}
- else if (key == PROPERTY_TIME_OWNER)
+ else
{
- setCurrentTimeOwner(value);
- for (const auto& listener : listeners)
+ // If host is off, notify listners
+ if (key == PROPERTY_TIME_MODE)
{
- listener->onOwnerChanged(timeOwner);
+ setCurrentTimeMode(value);
+ for (const auto listener : listeners)
+ {
+ listener->onModeChanged(timeMode);
+ }
+ }
+ else if (key == PROPERTY_TIME_OWNER)
+ {
+ setCurrentTimeOwner(value);
+ for (const auto listener : listeners)
+ {
+ listener->onOwnerChanged(timeOwner);
+ }
}
}
}
@@ -133,18 +154,67 @@
{
if (managedProperties.find(item.first) != managedProperties.end())
{
- static_cast<Manager*>(userData)
- ->onPropertyChanged(item.first, item.second.get<std::string>());
+ static_cast<Manager*>(userData)->onPropertyChanged(
+ item.first, item.second.get<std::string>());
}
}
return 0;
}
+void Manager::setPropertyAsRequested(const std::string& key,
+ const std::string& value)
+{
+ if (key == PROPERTY_TIME_MODE)
+ {
+ setRequestedMode(value);
+ }
+ else if (key == PROPERTY_TIME_OWNER)
+ {
+ setRequestedOwner(value);
+ }
+ else
+ {
+ // The key shall be already the supported one
+ // TODO: use elog API
+ assert(false);
+ }
+}
+
+void Manager::setRequestedMode(const std::string& mode)
+{
+ requestedMode = mode;
+}
+
+void Manager::setRequestedOwner(const std::string& owner)
+{
+ requestedOwner = owner;
+}
+
void Manager::onPgoodChanged(bool pgood)
{
hostOn = pgood;
- // TODO: if host is off, check requested time_mode/owner:
- // and notify the listeners if any.
+ if (hostOn)
+ {
+ return;
+ }
+ if (!requestedMode.empty())
+ {
+ setCurrentTimeMode(requestedMode);
+ for (const auto& listener : listeners)
+ {
+ listener->onModeChanged(timeMode);
+ }
+ setRequestedMode({}); // Clear requested mode
+ }
+ if (!requestedOwner.empty())
+ {
+ setCurrentTimeOwner(requestedOwner);
+ for (const auto& listener : listeners)
+ {
+ listener->onOwnerChanged(timeOwner);
+ }
+ setRequestedOwner({}); // Clear requested owner
+ }
}
int Manager::onPgoodChanged(sd_bus_message* msg,
@@ -174,6 +244,7 @@
log<level::INFO>("Time mode is changed",
entry("MODE=%s", mode.c_str()));
timeMode = convertToMode(mode);
+ utils::writeData(modeFile, mode);
}
void Manager::setCurrentTimeOwner(const std::string& owner)
@@ -181,6 +252,7 @@
log<level::INFO>("Time owner is changed",
entry("OWNER=%s", owner.c_str()));
timeOwner = convertToOwner(owner);
+ utils::writeData(ownerFile, owner);
}
std::string Manager::getSettings(const char* value) const
diff --git a/manager.hpp b/manager.hpp
index ef3315f..7edb30f 100644
--- a/manager.hpp
+++ b/manager.hpp
@@ -24,6 +24,7 @@
{
public:
friend class TestManager;
+
explicit Manager(sdbusplus::bus::bus& bus);
Manager(const Manager&) = delete;
Manager& operator=(const Manager&) = delete;
@@ -51,12 +52,21 @@
/** @brief The value to indicate if host is on */
bool hostOn = false;
+ /** @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;
/** @brief The current time owner */
Owner timeOwner;
+ /** @brief Restore saved settings */
+ void restoreSettings();
+
/** @brief Check if host is on and update hostOn variable */
void checkHostOn();
@@ -94,6 +104,28 @@
*/
void onPgoodChanged(bool pgood);
+ /** @brief Set the property as requested time mode/owner
+ *
+ * @param[in] key - The property name
+ * @param[in] value - The property value
+ */
+ void setPropertyAsRequested(const std::string& key,
+ const std::string& value);
+
+ /** @brief Set the current mode to user requested one
+ * if conditions allow it
+ *
+ * @param[in] mode - The string of time mode
+ */
+ 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 The static function called on settings property changed
*
* @param[in] msg - Data associated with subscribed signal
@@ -138,6 +170,12 @@
*/
static Owner convertToOwner(const std::string& owner);
+ /** @brief The string of time mode property */
+ static constexpr auto PROPERTY_TIME_MODE = "time_mode";
+
+ /** @brief The string of time owner property */
+ static constexpr auto PROPERTY_TIME_OWNER = "time_owner";
+
using Updater = std::function<void(const std::string&)>;
/** @brief Map the property string to functions that shall
@@ -145,10 +183,10 @@
*/
const std::map<std::string, Updater> propertyUpdaters =
{
- {"time_mode", std::bind(&Manager::setCurrentTimeMode,
- this, std::placeholders::_1)},
- {"time_owner", std::bind(&Manager::setCurrentTimeOwner,
- this, std::placeholders::_1)}
+ {PROPERTY_TIME_MODE, std::bind(&Manager::setCurrentTimeMode,
+ this, std::placeholders::_1)},
+ {PROPERTY_TIME_OWNER, std::bind(&Manager::setCurrentTimeOwner,
+ this, std::placeholders::_1)}
};
/** @brief The properties that manager shall notify the
@@ -158,6 +196,12 @@
/** @brief The map that maps the string to Owners */
static const std::map<std::string, Owner> 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";
};
}
diff --git a/test/TestHostEpoch.cpp b/test/TestHostEpoch.cpp
index ec8ecf1..0ff8a11 100644
--- a/test/TestHostEpoch.cpp
+++ b/test/TestHostEpoch.cpp
@@ -2,6 +2,7 @@
#include <gtest/gtest.h>
#include "host_epoch.hpp"
+#include "utils.hpp"
#include "config.h"
#include "types.hpp"
@@ -45,16 +46,6 @@
{
return hostEpoch.timeOwner;
}
- template <typename T>
- T readData(const char* fileName)
- {
- return HostEpoch::readData<T>(fileName);
- }
- template <typename T>
- void writeData(const char* fileName, T&& data)
- {
- HostEpoch::writeData<T>(fileName, std::forward<T>(data));
- }
microseconds getOffset()
{
return hostEpoch.offset;
@@ -75,7 +66,7 @@
{
// When file does not exist, the default offset shall be 0
microseconds offset(0);
- auto value = readData<decltype(offset)::rep>(FILE_NOT_EXIST);
+ auto value = utils::readData<decltype(offset)::rep>(FILE_NOT_EXIST);
EXPECT_EQ(0, value);
}
@@ -83,12 +74,13 @@
{
// Write offset to file
microseconds offsetToWrite(1234567);
- writeData<decltype(offsetToWrite)::rep>(FILE_OFFSET, offsetToWrite.count());
+ utils::writeData<decltype(offsetToWrite)::rep>(
+ FILE_OFFSET, offsetToWrite.count());
// Read it back
microseconds offsetToRead;
offsetToRead = microseconds(
- readData<decltype(offsetToRead)::rep>(FILE_OFFSET));
+ utils::readData<decltype(offsetToRead)::rep>(FILE_OFFSET));
EXPECT_EQ(offsetToWrite, offsetToRead);
}
diff --git a/test/TestManager.cpp b/test/TestManager.cpp
index 4d9ae73..935590e 100644
--- a/test/TestManager.cpp
+++ b/test/TestManager.cpp
@@ -39,10 +39,34 @@
{
return Manager::convertToOwner(owner);
}
+ bool hostOn()
+ {
+ return manager.hostOn;
+ }
+ std::string getRequestedMode()
+ {
+ return manager.requestedMode;
+ }
+ std::string getRequestedOwner()
+ {
+ return manager.requestedOwner;
+ }
+ void notifyPropertyChanged(const std::string& key,
+ const std::string& value)
+ {
+ manager.onPropertyChanged(key, value);
+ }
+ void notifyPgoodChanged(bool pgood)
+ {
+ manager.onPgoodChanged(pgood);
+ }
};
TEST_F(TestManager, empty)
{
+ EXPECT_FALSE(hostOn());
+ EXPECT_EQ("", getRequestedMode());
+ EXPECT_EQ("", getRequestedOwner());
EXPECT_EQ(Mode::NTP, getTimeMode());
EXPECT_EQ(Owner::BMC, getTimeOwner());
}
@@ -72,5 +96,44 @@
EXPECT_EQ(Owner::BMC, convertToOwner("xyz"));
}
+TEST_F(TestManager, pgoodChange)
+{
+ notifyPgoodChanged(true);
+ EXPECT_TRUE(hostOn());
+ notifyPgoodChanged(false);
+ EXPECT_FALSE(hostOn());
+}
+
+TEST_F(TestManager, propertyChange)
+{
+ // When host is off, property change will be notified to listners
+ EXPECT_FALSE(hostOn());
+ notifyPropertyChanged("time_mode", "MANUAL");
+ notifyPropertyChanged("time_owner", "HOST");
+ EXPECT_EQ("", getRequestedMode());
+ EXPECT_EQ("", getRequestedOwner());
+ // TODO: if gmock is ready, check mocked listners shall receive notifies
+
+ notifyPgoodChanged(true);
+ // When host is on, property changes are saved as requested ones
+ notifyPropertyChanged("time_mode", "MANUAL");
+ notifyPropertyChanged("time_owner", "HOST");
+ EXPECT_EQ("MANUAL", getRequestedMode());
+ EXPECT_EQ("HOST", getRequestedOwner());
+
+
+ // When host becomes off, the requested mode/owner shall be notified
+ // to listners, and be cleared
+ notifyPgoodChanged(false);
+ // TODO: if gmock is ready, check mocked listners shall receive notifies
+ EXPECT_EQ("", getRequestedMode());
+ EXPECT_EQ("", getRequestedOwner());
+
+ // When host is on, and invalid property is changed,
+ // verify the code asserts because it shall never occur
+ notifyPgoodChanged(true);
+ ASSERT_DEATH(notifyPropertyChanged("invalid property", "whatever"), "");
+}
+
}
}
diff --git a/utils.hpp b/utils.hpp
new file mode 100644
index 0000000..0589613
--- /dev/null
+++ b/utils.hpp
@@ -0,0 +1,47 @@
+#pragma once
+
+#include <fstream>
+
+namespace phosphor
+{
+namespace time
+{
+namespace utils
+{
+
+/** @brief Read data with type T from file
+ *
+ * @param[in] fileName - The name of file to read from
+ *
+ * @return The data with type T
+ */
+template <typename T>
+T readData(const char* fileName)
+{
+ T data{};
+ std::ifstream fs(fileName);
+ if (fs.is_open())
+ {
+ fs >> data;
+ }
+ return data;
+}
+
+/** @brief Write data with type T to file
+ *
+ * @param[in] fileName - The name of file to write to
+ * @param[in] data - The data with type T to write to file
+ */
+template <typename T>
+void writeData(const char* fileName, T&& data)
+{
+ std::ofstream fs(fileName, std::ios::out);
+ if (fs.is_open())
+ {
+ fs << std::forward<T>(data);
+ }
+}
+
+}
+}
+}