6 files changed, 199 insertions, 151 deletions

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 @@ namespace control
 {
 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 @@ struct PropertyChanged
     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 @@ struct PropertyChanged
     /** @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 @@ private:
 /**
  * @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 @@ struct InterfaceAdded
                     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 @@ import yaml
 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 @@ Timer{
 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 @@ std::vector<Signal>{
     },
 %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 @@ const std::vector<ZoneGroup> Manager::_zoneLayouts
                         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 @@ using PrecondGroup = std::tuple<std::string,
 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 @@ using SetSpeedEvent = std::tuple<Group,
                                  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::decTimerExpired()
 
 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 @@ void Zone::removeEvent(const SetSpeedEvent& event)
     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 @@ class Zone
         };
 
         /**
-         * @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