Use host state object to check if host is on
The code was using pgood to determine if host is on or off. Now we have
host state object, which is a more appropriate way to check host on/off.
So change pgood related code to use host state object.
Change-Id: I553c1a40922ca2e8bc6904688c55e85971bd4720
Signed-off-by: Lei YU <mine260309@gmail.com>
diff --git a/README.md b/README.md
index cf7a287..9584ca7 100644
--- a/README.md
+++ b/README.md
@@ -60,13 +60,12 @@
MANUAL | SPLIT | OK | OK
MANUAL | BOTH | OK | OK
-### Pgood
-When host is on (pgood == 1), the changes of the above time mode/owner are not
-applied but deferred. The changes of the mode/owner are saved to persistent
-storage.
+### Special note on host on
+When 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 host is off (pgood == 0), the saved mode/owner are read from
-persistent storage and are applied.
+When host is off, the saved mode/owner are read from persistent storage and are
+applied.
Note: user can set the time mode and owner in settings daemon at any time,
but time manager applying them is governed by the above condition.
diff --git a/manager.cpp b/manager.cpp
index 15f1cec..1c971a3 100644
--- a/manager.cpp
+++ b/manager.cpp
@@ -5,20 +5,13 @@
#include <phosphor-logging/elog-errors.hpp>
#include <phosphor-logging/log.hpp>
#include <xyz/openbmc_project/Common/error.hpp>
+#include <xyz/openbmc_project/State/Host/server.hpp>
namespace rules = sdbusplus::bus::match::rules;
namespace // anonymous
{
-const auto MATCH_PGOOD_CHANGE =
- rules::type::signal() +
- rules::member("PropertiesChanged") +
- rules::path("/org/openbmc/control/power0") +
- rules::interface("org.freedesktop.DBus.Properties");
-
-constexpr auto POWER_PATH = "/org/openbmc/control/power0";
-constexpr auto POWER_INTERFACE = "org.openbmc.control.Power";
-constexpr auto PGOOD_STR = "pgood";
+constexpr auto HOST_CURRENT_STATE = "CurrentHostState";
constexpr auto SYSTEMD_TIME_SERVICE = "org.freedesktop.timedate1";
constexpr auto SYSTEMD_TIME_PATH = "/org/freedesktop/timedate1";
@@ -37,10 +30,15 @@
Manager::managedProperties = {PROPERTY_TIME_MODE, PROPERTY_TIME_OWNER};
Manager::Manager(sdbusplus::bus::bus& bus)
- : bus(bus),
- pgoodChangeMatch(bus, MATCH_PGOOD_CHANGE, onPgoodChanged, this)
+ : bus(bus)
{
using namespace sdbusplus::bus::match::rules;
+ hostStateChangeMatch =
+ std::make_unique<decltype(hostStateChangeMatch)::element_type>(
+ 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),
@@ -94,16 +92,17 @@
void Manager::checkHostOn()
{
- std::string powerService = utils::getService(bus,
- POWER_PATH,
- POWER_INTERFACE);
-
- int pgood = utils::getProperty<int>(bus,
- powerService.c_str(),
- POWER_PATH,
- POWER_INTERFACE,
- PGOOD_STR);
- hostOn = static_cast<bool>(pgood);
+ using Host = sdbusplus::xyz::openbmc_project::State::server::Host;
+ auto hostService = utils::getService(bus,
+ settings.hostState.c_str(),
+ settings::hostStateIntf);
+ auto stateStr = utils::getProperty<std::string>(bus,
+ hostService.c_str(),
+ settings.hostState.c_str(),
+ settings::hostStateIntf,
+ HOST_CURRENT_STATE);
+ auto state = Host::convertHostStateFromString(stateStr);
+ hostOn = (state == Host::HostState::Running);
}
void Manager::onPropertyChanged(const std::string& key,
@@ -206,13 +205,39 @@
}
}
-void Manager::onPgoodChanged(bool pgood)
+void Manager::onHostStateChanged(sdbusplus::message::message& msg)
{
- hostOn = pgood;
+ using Interface = std::string;
+ using Property = std::string;
+ using Value = std::string;
+ using Properties = std::map<Property, sdbusplus::message::variant<Value>>;
+ using Host = sdbusplus::xyz::openbmc_project::State::server::Host;
+
+ Interface interface;
+ Properties properties;
+
+ msg.read(interface, properties);
+
+ for(const auto& p : properties)
+ {
+ if (p.first == HOST_CURRENT_STATE)
+ {
+ auto state = Host::convertHostStateFromString(p.second.get<std::string>());
+ onHostState(state == Host::HostState::Running);
+ break;
+ }
+ }
+}
+
+void Manager::onHostState(bool on)
+{
+ hostOn = on;
if (hostOn)
{
+ log<level::INFO>("Changing time settings is *deferred* now");
return;
}
+ log<level::INFO>("Changing time settings allowed now");
if (!requestedMode.empty())
{
if (setCurrentTimeMode(requestedMode))
@@ -231,28 +256,6 @@
}
}
-int Manager::onPgoodChanged(sd_bus_message* msg,
- void* userData,
- sd_bus_error* retError)
-{
- using properties = std::map < std::string,
- sdbusplus::message::variant<int> >;
- auto m = sdbusplus::message::message(msg);
- // message type: sa{sv}as
- std::string ignore;
- properties props;
- m.read(ignore, props);
- for (const auto& item : props)
- {
- if (item.first == PGOOD_STR)
- {
- static_cast<Manager*>(userData)
- ->onPgoodChanged(static_cast<bool>(item.second.get<int>()));
- }
- }
- return 0;
-}
-
bool Manager::setCurrentTimeMode(const std::string& mode)
{
auto newMode = utils::strToMode(mode);
diff --git a/manager.hpp b/manager.hpp
index f9ee511..e84652c 100644
--- a/manager.hpp
+++ b/manager.hpp
@@ -45,8 +45,8 @@
/** @brief The match of settings property change */
std::vector<sdbusplus::bus::match::match> settingsMatches;
- /** @brief The match of pgood change */
- sdbusplus::bus::match::match pgoodChangeMatch;
+ /** @brief The match of host state change */
+ std::unique_ptr<sdbusplus::bus::match::match> hostStateChangeMatch;
/** @brief The container to hold all the listeners */
std::set<PropertyChangeListner*> listeners;
@@ -135,11 +135,17 @@
void onPropertyChanged(const std::string& key,
const std::string& value);
- /** @brief Notified on pgood has changed
+ /** @brief Notified on host state has changed
*
- * @param[in] pgood - The changed pgood value
+ * @param[in] msg - sdbusplus dbusmessage
*/
- void onPgoodChanged(bool pgood);
+ void onHostStateChanged(sdbusplus::message::message& msg);
+
+ /** @brief Notified on host state has changed
+ *
+ * @param[in] on - Indicate if the host is on or off
+ */
+ void onHostState(bool on);
/** @brief Set the property as requested time mode/owner
*
@@ -179,16 +185,6 @@
void* userData,
sd_bus_error* retError);
- /** @brief Notified on pgood has changed
- *
- * @param[in] msg - Data associated with subscribed signal
- * @param[in] userData - Pointer to this object instance
- * @param[out] retError - Not used but required with signal API
- */
- static int onPgoodChanged(sd_bus_message* msg,
- void* userData,
- sd_bus_error* retError);
-
/** @brief The string of time mode property */
static constexpr auto PROPERTY_TIME_MODE = "TimeSyncMethod";
diff --git a/settings.cpp b/settings.cpp
index 650d03d..a3eada9 100644
--- a/settings.cpp
+++ b/settings.cpp
@@ -17,7 +17,7 @@
{
auto bus = sdbusplus::bus::new_default();
std::vector<std::string> settingsIntfs =
- {timeOwnerIntf, timeSyncIntf};
+ {timeOwnerIntf, timeSyncIntf, hostStateIntf};
auto depth = 0;
auto mapperCall = bus.new_method_call(mapperService,
@@ -57,6 +57,10 @@
{
timeSyncMethod = path;
}
+ else if (hostStateIntf == interface)
+ {
+ hostState = path;
+ }
}
}
diff --git a/settings.hpp b/settings.hpp
index db1e5e4..95d20dc 100644
--- a/settings.hpp
+++ b/settings.hpp
@@ -13,6 +13,7 @@
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";
/** @class Objects
* @brief Fetch paths of settings D-bus objects of interest upon construction
@@ -47,6 +48,9 @@
/** @brief time sync method settings object */
Path timeSyncMethod;
+
+ /** @brief host state object */
+ Path hostState;
};
} // namespace settings
diff --git a/test/TestManager.cpp b/test/TestManager.cpp
index d8c51ca..decc21d 100644
--- a/test/TestManager.cpp
+++ b/test/TestManager.cpp
@@ -56,9 +56,9 @@
{
manager.onPropertyChanged(key, value);
}
- void notifyPgoodChanged(bool pgood)
+ void notifyOnHostState(bool hostOn)
{
- manager.onPgoodChanged(pgood);
+ manager.onHostState(hostOn);
}
};
@@ -74,11 +74,11 @@
}
-TEST_F(TestManager, DISABLED_pgoodChange)
+TEST_F(TestManager, DISABLED_hostStateChange)
{
- notifyPgoodChanged(true);
+ notifyOnHostState(true);
EXPECT_TRUE(hostOn());
- notifyPgoodChanged(false);
+ notifyOnHostState(false);
EXPECT_FALSE(hostOn());
}
@@ -104,7 +104,7 @@
EXPECT_EQ("", getRequestedOwner());
// When host is on, property changes are saved as requested ones
- notifyPgoodChanged(true);
+ notifyOnHostState(true);
// Check mocked listeners shall not receive notifications
EXPECT_CALL(listener1, onModeChanged(Mode::Manual)).Times(0);
@@ -132,14 +132,14 @@
EXPECT_CALL(listener2, onModeChanged(Mode::NTP)).Times(1);
EXPECT_CALL(listener2, onOwnerChanged(Owner::Split)).Times(1);
- notifyPgoodChanged(false);
+ 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
- notifyPgoodChanged(true);
+ notifyOnHostState(true);
ASSERT_DEATH(notifyPropertyChanged("invalid property", "whatever"), "");
}
@@ -154,7 +154,7 @@
"xyz.openbmc_project.Time.Owner.Owners.Host");
// Set host on
- notifyPgoodChanged(true);
+ notifyOnHostState(true);
// Check mocked listeners shall not receive notifications
EXPECT_CALL(listener1, onModeChanged(_)).Times(0);
@@ -197,7 +197,7 @@
EXPECT_CALL(listener2, onModeChanged(_)).Times(0);
EXPECT_CALL(listener2, onOwnerChanged(_)).Times(0);
- notifyPgoodChanged(false);
+ notifyOnHostState(false);
EXPECT_EQ("", getRequestedMode());
EXPECT_EQ("", getRequestedOwner());