Update event initialization
Using a null dbus message to the event signal will perform the necessary
steps to initialize the event parameters using the given signal
handler's function directly.
i.e.) In the case for subscribing to PropertiesChanged signals,
initially the given property must be read. When the PropertiesChanged
signal struct is given a null message, it finds the property, reads the
value, and then can perform the given signal handler function directly.
This produces the same functional path as receiving a PropertiesChanged
signal containing the property value within the message.
Change-Id: I35575cfff66eb0305156be786cb1f5536d42bb1c
Signed-off-by: Matthew Barth <msbarth@us.ibm.com>
diff --git a/control/functor.hpp b/control/functor.hpp
index a5044da..daaa1b7 100644
--- a/control/functor.hpp
+++ b/control/functor.hpp
@@ -1,6 +1,7 @@
#pragma once
#include "types.hpp"
+#include "sdbusplus.hpp"
#include <phosphor-logging/log.hpp>
namespace phosphor
@@ -11,7 +12,10 @@
{
class Zone;
+using namespace phosphor::fan;
using namespace phosphor::logging;
+using InternalFailure = sdbusplus::xyz::openbmc_project::Common::
+ Error::InternalFailure;
/**
* @brief Create a handler function object
@@ -55,9 +59,11 @@
PropertyChanged& operator=(const PropertyChanged&) = default;
PropertyChanged(PropertyChanged&&) = default;
PropertyChanged& operator=(PropertyChanged&&) = default;
- PropertyChanged(const char* iface,
+ PropertyChanged(const char* path,
+ const char* iface,
const char* property,
U&& handler) :
+ _path(path),
_iface(iface),
_property(property),
_handler(std::forward<U>(handler)) { }
@@ -65,35 +71,61 @@
/** @brief Run signal handler function
*
* Extract the property from the PropertiesChanged
- * message and run the handler function.
+ * message (or read the property when the message is null)
+ * and run the handler function.
*/
- void operator()(sdbusplus::bus::bus&,
+ void operator()(sdbusplus::bus::bus& bus,
sdbusplus::message::message& msg,
Zone& zone) const
{
- std::map<std::string, sdbusplus::message::variant<T>> properties;
- const char* iface = nullptr;
-
- msg.read(iface);
- if (!iface || strcmp(iface, _iface))
+ if (msg)
{
- return;
- }
+ std::map<std::string, sdbusplus::message::variant<T>> properties;
+ const char* iface = nullptr;
- msg.read(properties);
- auto it = properties.find(_property);
- if (it == properties.cend())
+ msg.read(iface);
+ if (!iface || strcmp(iface, _iface))
+ {
+ return;
+ }
+
+ msg.read(properties);
+ auto it = properties.find(_property);
+ if (it == properties.cend())
+ {
+ log<level::ERR>("Unable to find property on interface",
+ entry("PROPERTY=%s", _property),
+ entry("INTERFACE=%s", _iface));
+ return;
+ }
+
+ _handler(zone, std::forward<T>(it->second.template get<T>()));
+ }
+ else
{
- log<level::ERR>("Unable to find property on interface",
- entry("PROPERTY=%s", _property),
- entry("INTERFACE=%s", _iface));
- return;
+ try
+ {
+ auto val = util::SDBusPlus::getProperty<T>(bus,
+ _path,
+ _iface,
+ _property);
+ _handler(zone, std::forward<T>(val));
+ }
+ catch (const InternalFailure& ife)
+ {
+ // Property will not be used unless a property changed
+ // signal message is received for this property.
+ log<level::INFO>(
+ "Property not used, unless PropertyChanged signal received",
+ entry("PATH=%s", _path),
+ entry("INTERFACE=%s", _iface),
+ entry("PROPERTY=%s", _property));
+ }
}
-
- _handler(zone, std::forward<T>(it->second.template get<T>()));
}
private:
+ const char* _path;
const char* _iface;
const char* _property;
U _handler;
@@ -102,19 +134,24 @@
/**
* @brief Used to process a Dbus property changed signal event
*
- * @param[in] iface - Sensor value interface
- * @param[in] property - Sensor value property
+ * @param[in] path - Object path
+ * @param[in] iface - Object interface
+ * @param[in] property - Object property
* @param[in] handler - Handler function to perform
*
* @tparam T - The type of the property
* @tparam U - The type of the handler
*/
template <typename T, typename U>
-auto propertySignal(const char* iface,
+auto propertySignal(const char* path,
+ const char* iface,
const char* property,
U&& handler)
{
- return PropertyChanged<T, U>(iface, property, std::forward<U>(handler));
+ return PropertyChanged<T, U>(path,
+ iface,
+ property,
+ std::forward<U>(handler));
}
/**
@@ -151,34 +188,37 @@
sdbusplus::message::message& msg,
Zone& zone) const
{
- std::map<std::string,
- std::map<std::string,
- sdbusplus::message::variant<T>>> intfProp;
- sdbusplus::message::object_path op;
-
- msg.read(op);
- auto objPath = static_cast<const std::string&>(op).c_str();
- if (!objPath || strcmp(objPath, _path))
+ if (msg)
{
- // Object path does not match this handler's path
- return;
- }
+ std::map<std::string,
+ std::map<std::string,
+ sdbusplus::message::variant<T>>> intfProp;
+ sdbusplus::message::object_path op;
- msg.read(intfProp);
- auto itIntf = intfProp.find(_iface);
- if (itIntf == intfProp.cend())
- {
- // Interface not found on this handler's path
- return;
- }
- auto itProp = itIntf->second.find(_property);
- if (itProp == itIntf->second.cend())
- {
- // Property not found on this handler's path
- return;
- }
+ msg.read(op);
+ auto objPath = static_cast<const std::string&>(op).c_str();
+ if (!objPath || strcmp(objPath, _path))
+ {
+ // Object path does not match this handler's path
+ return;
+ }
- _handler(zone, std::forward<T>(itProp->second.template get<T>()));
+ msg.read(intfProp);
+ auto itIntf = intfProp.find(_iface);
+ if (itIntf == intfProp.cend())
+ {
+ // Interface not found on this handler's path
+ return;
+ }
+ auto itProp = itIntf->second.find(_property);
+ if (itProp == itIntf->second.cend())
+ {
+ // Property not found on this handler's path
+ return;
+ }
+
+ _handler(zone, std::forward<T>(itProp->second.template get<T>()));
+ }
}
private:
diff --git a/control/gen-fan-zone-defs.py b/control/gen-fan-zone-defs.py
index 6d04bbd..061c314 100755
--- a/control/gen-fan-zone-defs.py
+++ b/control/gen-fan-zone-defs.py
@@ -11,11 +11,11 @@
from argparse import ArgumentParser
from mako.template import Template
-tmpl = '''
+tmpl = '''\
<%!
def indent(str, depth):
return ''.join(4*' '*depth+line for line in str.splitlines(True))
-%>
+%>\
<%def name="genSSE(event)" buffered="True">
Group{
%for member in event['group']:
@@ -49,7 +49,7 @@
std::vector<Signal>{
%for s in event['signals']:
Signal{
- ${s['match']}(
+ match::${s['match']}(
%for i, mp in enumerate(s['mparams']):
%if (i+1) != len(s['mparams']):
"${mp}",
@@ -79,17 +79,16 @@
},
%endfor
}
-</%def>
+</%def>\
/* This is a generated file. */
-#include <sdbusplus/bus.hpp>
#include "manager.hpp"
#include "functor.hpp"
#include "actions.hpp"
#include "handlers.hpp"
#include "preconditions.hpp"
+#include "matches.hpp"
using namespace phosphor::fan::control;
-using namespace sdbusplus::bus::match::rules;
const unsigned int Manager::_powerOnDelay{${mgr_data['power_on_delay']}};
@@ -200,7 +199,7 @@
std::vector<Signal>{
%for s in event['pc']['pcsigs']:
Signal{
- ${s['match']}(
+ match::${s['match']}(
%for i, mp in enumerate(s['mparams']):
%if (i+1) != len(s['mparams']):
"${mp}",
diff --git a/control/matches.hpp b/control/matches.hpp
new file mode 100644
index 0000000..855a749
--- /dev/null
+++ b/control/matches.hpp
@@ -0,0 +1,76 @@
+#pragma once
+
+#include <sdbusplus/bus.hpp>
+#include "sdbusplus.hpp"
+
+namespace phosphor
+{
+namespace fan
+{
+namespace control
+{
+namespace match
+{
+
+using namespace phosphor::fan;
+using namespace sdbusplus::bus::match;
+using InternalFailure = sdbusplus::xyz::openbmc_project::Common::
+ Error::InternalFailure;
+
+/**
+ * @brief A match function that constructs a PropertiesChanged match string
+ * @details Constructs a PropertiesChanged match string with a given object
+ * path and interface
+ *
+ * @param[in] obj - Object's path name
+ * @param[in] iface - Interface name
+ *
+ * @return - A PropertiesChanged match string
+ */
+inline auto propertiesChanged(const std::string& obj, const std::string& iface)
+{
+ return rules::propertiesChanged(obj, iface);
+}
+
+/**
+ * @brief A match function that constructs an InterfacesAdded match string
+ * @details Constructs an InterfacesAdded match string with a given object
+ * path
+ *
+ * @param[in] obj - Object's path name
+ *
+ * @return - An InterfacesAdded match string
+ */
+inline auto interfacesAdded(const std::string& obj)
+{
+ return rules::interfacesAdded(obj);
+}
+
+/**
+ * @brief A match function that constructs a NameOwnerChanged match string
+ * @details Constructs a NameOwnerChanged match string with a given object
+ * path and interface
+ *
+ * @param[in] obj - Object's path name
+ * @param[in] iface - Interface name
+ *
+ * @return - A NameOwnerChanged match string
+ */
+inline auto nameOwnerChanged(const std::string& obj, const std::string& iface)
+{
+ std::string noc;
+ try
+ {
+ noc = rules::nameOwnerChanged(util::SDBusPlus::getService(obj, iface));
+ }
+ catch (const InternalFailure& ife)
+ {
+ // Unable to construct NameOwnerChanged match string
+ }
+ return noc;
+}
+
+} // namespace match
+} // namespace control
+} // namespace fan
+} // namespace phosphor
diff --git a/control/types.hpp b/control/types.hpp
index f6b2b60..d3ddb16 100644
--- a/control/types.hpp
+++ b/control/types.hpp
@@ -56,8 +56,8 @@
constexpr auto intervalPos = 0;
using Timer = std::tuple<std::chrono::seconds>;
-constexpr auto signaturePos = 0;
-constexpr auto handlerObjPos = 1;
+constexpr auto sigMatchPos = 0;
+constexpr auto sigHandlerPos = 1;
using Signal = std::tuple<std::string, Handler>;
constexpr auto groupPos = 0;
@@ -70,9 +70,10 @@
std::vector<Signal>>;
constexpr auto eventGroupPos = 0;
-constexpr auto eventHandlerPos = 1;
-constexpr auto eventActionsPos = 2;
-using EventData = std::tuple<Group, Handler, std::vector<Action>>;
+constexpr auto eventMatchPos = 1;
+constexpr auto eventHandlerPos = 2;
+constexpr auto eventActionsPos = 3;
+using EventData = std::tuple<Group, std::string, Handler, std::vector<Action>>;
constexpr auto timerTimerPos = 0;
using TimerEvent = std::tuple<phosphor::fan::util::Timer>;
diff --git a/control/zone.cpp b/control/zone.cpp
index fa426e6..3d7852e 100644
--- a/control/zone.cpp
+++ b/control/zone.cpp
@@ -194,46 +194,35 @@
void Zone::initEvent(const SetSpeedEvent& event)
{
- // Get the current value for each property
- for (auto& group : std::get<groupPos>(event))
- {
- try
- {
- refreshProperty(_bus,
- group.first,
- std::get<intfPos>(group.second),
- std::get<propPos>(group.second));
- }
- catch (const InternalFailure& ife)
- {
- log<level::INFO>(
- "Unable to find property",
- entry("PATH=%s", group.first.c_str()),
- entry("INTERFACE=%s", std::get<intfPos>(group.second).c_str()),
- entry("PROPERTY=%s", std::get<propPos>(group.second).c_str()));
- }
- }
- // Setup signal matches for events
+ sdbusplus::message::message nullMsg{nullptr};
+
for (auto& sig : std::get<signalsPos>(event))
{
+ // Initialize the event signal using handler
+ std::get<sigHandlerPos>(sig)(_bus, nullMsg, *this);
+ // Setup signal matches of the property for event
std::unique_ptr<EventData> eventData =
std::make_unique<EventData>(
EventData
{
std::get<groupPos>(event),
- std::get<handlerObjPos>(sig),
+ std::get<sigMatchPos>(sig),
+ std::get<sigHandlerPos>(sig),
std::get<actionsPos>(event)
}
);
- std::unique_ptr<sdbusplus::server::match::match> match =
- std::make_unique<sdbusplus::server::match::match>(
- _bus,
- std::get<signaturePos>(sig).c_str(),
- std::bind(std::mem_fn(&Zone::handleEvent),
- this,
- std::placeholders::_1,
- eventData.get())
- );
+ std::unique_ptr<sdbusplus::server::match::match> match = nullptr;
+ if (!std::get<sigMatchPos>(sig).empty())
+ {
+ match = std::make_unique<sdbusplus::server::match::match>(
+ _bus,
+ std::get<sigMatchPos>(sig).c_str(),
+ std::bind(std::mem_fn(&Zone::handleEvent),
+ this,
+ std::placeholders::_1,
+ eventData.get())
+ );
+ }
_signalEvents.emplace_back(std::move(eventData), std::move(match));
}
// Attach a timer to run the action of an event
@@ -305,43 +294,14 @@
if (it != std::end(_signalEvents))
{
std::get<signalEventDataPos>(*it).reset();
- std::get<signalMatchPos>(*it).reset();
+ if (std::get<signalMatchPos>(*it) != nullptr)
+ {
+ std::get<signalMatchPos>(*it).reset();
+ }
_signalEvents.erase(it);
}
}
-void Zone::refreshProperty(sdbusplus::bus::bus& bus,
- const std::string& path,
- const std::string& iface,
- const std::string& prop)
-{
- PropertyVariantType property;
- getProperty(_bus, path, iface, prop, property);
- setPropertyValue(path.c_str(), iface.c_str(), prop.c_str(), property);
-}
-
-void Zone::getProperty(sdbusplus::bus::bus& bus,
- const std::string& path,
- const std::string& iface,
- const std::string& prop,
- PropertyVariantType& value)
-{
- auto serv = util::SDBusPlus::getService(bus, path, iface);
- auto hostCall = bus.new_method_call(serv.c_str(),
- path.c_str(),
- "org.freedesktop.DBus.Properties",
- "Get");
- hostCall.append(iface);
- hostCall.append(prop);
- auto hostResponseMsg = bus.call(hostCall);
- if (hostResponseMsg.is_method_error())
- {
- log<level::INFO>("Host call response error for retrieving property");
- elog<InternalFailure>();
- }
- hostResponseMsg.read(value);
-}
-
void Zone::timerExpired(Group eventGroup, std::vector<Action> eventActions)
{
// Perform the actions
diff --git a/control/zone.hpp b/control/zone.hpp
index c7995b6..8869f91 100644
--- a/control/zone.hpp
+++ b/control/zone.hpp
@@ -393,34 +393,6 @@
};
/**
- * @brief Refresh the given property's cached value
- *
- * @param[in] bus - the bus to use
- * @param[in] path - the dbus path name
- * @param[in] iface - the dbus interface name
- * @param[in] prop - the property name
- */
- void refreshProperty(sdbusplus::bus::bus& bus,
- const std::string& path,
- const std::string& iface,
- const std::string& prop);
-
- /**
- * @brief Get a property value from the path/interface given
- *
- * @param[in] bus - the bus to use
- * @param[in] path - the dbus path name
- * @param[in] iface - the dbus interface name
- * @param[in] prop - the property name
- * @param[out] value - the value of the property
- */
- static void getProperty(sdbusplus::bus::bus& bus,
- const std::string& path,
- const std::string& iface,
- const std::string& prop,
- PropertyVariantType& value);
-
- /**
* @brief Dbus signal change callback handler
*
* @param[in] msg - Expanded sdbusplus message data