UpdateVirtualSensor uses information from signals
UpdateVirtualSensor uses information obtained from signals to avoid
extensive dbus queries.
Tested:
1:Execute DBus cmd to see if VirtualSensor can correctly obtain the
value of DbusSensor
busctl tree xyz.openbmc_project.VirtualSensor
busctl introspect xyz.openbmc_project.VirtualSensor xxx
2:Waiting for the value change of DbusSensor,Check if the value of
the virtual sensor has changed after the dbusSensor changes.
Fixes openbmc/phosphor-virtual-sensor#1
Change-Id: If11f9017b31ce5cf06f910a38c65637c55d74b24
Signed-off-by: Tao Lin <lintao.lc@ieisystem.com>
diff --git a/dbusSensor.cpp b/dbusSensor.cpp
new file mode 100644
index 0000000..f79705b
--- /dev/null
+++ b/dbusSensor.cpp
@@ -0,0 +1,152 @@
+#include "dbusSensor.hpp"
+
+#include "virtualSensor.hpp"
+
+#include <sdbusplus/bus.hpp>
+#include <sdbusplus/bus/match.hpp>
+
+#include <cmath>
+static constexpr auto sensorIntf =
+ sdbusplus::common::xyz::openbmc_project::sensor::Value::interface;
+
+/** When the Entity Manager removes the sensor, the interfaceRemoveSignal sent
+ * uses the path /xyz/openbmc_project/sensors
+ * */
+static constexpr auto interfacesSensorPath = "/xyz/openbmc_project/sensors";
+
+namespace phosphor::virtual_sensor
+{
+
+DbusSensor::DbusSensor(sdbusplus::bus_t& bus, const std::string& path,
+ VirtualSensor& virtualSensor) :
+ bus(bus),
+ path(path), virtualSensor(virtualSensor),
+ signalPropChange(
+ bus, sdbusplus::bus::match::rules::propertiesChanged(path, sensorIntf),
+ [this](sdbusplus::message_t& message) {
+ handleDbusSignalPropChange(message);
+}),
+ signalRemove(
+ bus,
+ sdbusplus::bus::match::rules::interfacesRemoved(interfacesSensorPath),
+ [this](sdbusplus::message_t& message) {
+ handleDbusSignalRemove(message);
+})
+{
+ initSensorValue();
+}
+
+double DbusSensor::getSensorValue()
+{
+ return value;
+}
+
+void DbusSensor::initSensorValue()
+{
+ try
+ {
+ // If servName is not empty, reduce one DbusCall
+ if (servName.empty())
+ {
+ value = std::numeric_limits<double>::quiet_NaN();
+ servName = getService(bus, path, sensorIntf);
+ }
+
+ if (!servName.empty())
+ {
+ signalNameOwnerChanged.reset();
+ signalNameOwnerChanged = std::make_unique<sdbusplus::bus::match_t>(
+ bus,
+ sdbusplus::bus::match::rules::nameOwnerChanged() +
+ sdbusplus::bus::match::rules::arg0namespace(servName),
+ [this](sdbusplus::message_t& message) {
+ handleDbusSignalNameOwnerChanged(message);
+ });
+
+ value = getDbusProperty<double>(bus, servName, path, sensorIntf,
+ "Value");
+ }
+ }
+ catch (const std::exception& e)
+ {
+ value = std::numeric_limits<double>::quiet_NaN();
+ }
+
+ return;
+}
+
+void DbusSensor::handleDbusSignalNameOwnerChanged(sdbusplus::message_t& msg)
+{
+ try
+ {
+ auto [name, oldOwner,
+ newOwner] = msg.unpack<std::string, std::string, std::string>();
+
+ if (!oldOwner.empty() && !name.empty())
+ {
+ if (name == servName)
+ {
+ // Connection removed
+
+ value = std::numeric_limits<double>::quiet_NaN();
+ virtualSensor.updateVirtualSensor();
+ }
+ }
+ }
+ catch (const std::exception& e)
+ {
+ lg2::error("Error in dbusSensor NameOwnerChanged: {PATH} {ERROR}",
+ "PATH", path, "ERROR", e);
+ }
+}
+
+void DbusSensor::handleDbusSignalPropChange(sdbusplus::message_t& msg)
+{
+ try
+ {
+ using SensorValuePropertiesVariant = sdbusplus::server::xyz::
+ openbmc_project::sensor::Value::PropertiesVariant;
+ auto [msgIfce, msgData] =
+ msg.unpack<std::string,
+ std::map<std::string, SensorValuePropertiesVariant>>();
+
+ std::string path = msg.get_path();
+
+ if (auto itr = msgData.find("Value"); itr != msgData.end())
+ {
+ value = std::get<double>(itr->second);
+ if (!std::isfinite(value))
+ {
+ value = std::numeric_limits<double>::quiet_NaN();
+ }
+
+ virtualSensor.updateVirtualSensor();
+ }
+ }
+ catch (const std::exception& e)
+ {
+ lg2::error("Error in dbusSensor PropertyChange: {PATH} {ERROR}",
+ "PATH", path, "ERROR", e);
+ }
+}
+
+void DbusSensor::handleDbusSignalRemove(sdbusplus::message_t& msg)
+{
+ try
+ {
+ auto objPath = msg.unpack<sdbusplus::message::object_path>();
+
+ if (this->path == objPath)
+ {
+ value = std::numeric_limits<double>::quiet_NaN();
+ virtualSensor.updateVirtualSensor();
+ }
+ }
+ catch (const std::exception& e)
+ {
+ lg2::error("Error in dbusSensor interfaceRemove: {PATH} {ERROR}",
+ "PATH", path, "ERROR", e);
+ }
+}
+
+} // namespace phosphor::virtual_sensor
diff --git a/dbusSensor.hpp b/dbusSensor.hpp
index 5c119a5..41d8c69 100644
--- a/dbusSensor.hpp
+++ b/dbusSensor.hpp
@@ -1,11 +1,12 @@
-#include "dbusUtils.hpp"
+#pragma once
#include <sdbusplus/bus.hpp>
#include <sdbusplus/bus/match.hpp>
-const char* sensorIntf = "xyz.openbmc_project.Sensor.Value";
+namespace phosphor::virtual_sensor
+{
-int handleDbusSignal(sd_bus_message* msg, void* usrData, sd_bus_error* err);
+class VirtualSensor;
class DbusSensor
{
@@ -18,44 +19,48 @@
* @param[in] bus - Handle to system dbus
* @param[in] path - The Dbus path of sensor
*/
- DbusSensor(sdbusplus::bus_t& bus, const std::string& path, void* ctx) :
- bus(bus), path(path),
- signal(
- bus,
- sdbusplus::bus::match::rules::propertiesChanged(path, sensorIntf),
- handleDbusSignal, ctx)
- {}
+ DbusSensor(sdbusplus::bus_t& bus, const std::string& path,
+ VirtualSensor& virtualSensor);
- /** @brief Get sensor value property from D-bus interface */
- double getSensorValue()
- {
- if (servName.empty())
- {
- servName = getService(bus, path, sensorIntf);
- if (servName.empty())
- {
- return std::numeric_limits<double>::quiet_NaN();
- }
- }
-
- try
- {
- return getDbusProperty<double>(bus, servName, path, sensorIntf,
- "Value");
- }
- catch (const std::exception& e)
- {
- return std::numeric_limits<double>::quiet_NaN();
- }
- }
+ /** @brief Get sensor value from local */
+ double getSensorValue();
private:
/** @brief sdbusplus bus client connection. */
sdbusplus::bus_t& bus;
+
/** @brief complete path for sensor */
std::string path{};
+
/** @brief service name for the sensor daemon */
std::string servName{};
+
+ /** @brief point to the VirtualSensor */
+ VirtualSensor& virtualSensor;
+
/** @brief signal for sensor value change */
- sdbusplus::bus::match_t signal;
+ sdbusplus::bus::match_t signalPropChange;
+
+ /** @brief signal for sensor interface remove */
+ sdbusplus::bus::match_t signalRemove;
+
+ /** @brief Match for this dbus sensor service destroy */
+ std::unique_ptr<sdbusplus::bus::match_t> signalNameOwnerChanged;
+
+ /** @brief dbus sensor value */
+ double value = std::numeric_limits<double>::quiet_NaN();
+
+ /** @brief Get sensor value property from D-bus interface */
+ void initSensorValue();
+
+ /** @brief Handle for this dbus sensor NameOwnerChanged */
+ void handleDbusSignalNameOwnerChanged(sdbusplus::message_t& msg);
+
+ /** @brief Handle for this dbus sensor PropertyChanged */
+ void handleDbusSignalPropChange(sdbusplus::message_t& msg);
+
+ /** @brief Handle for this dbus sensor InterfaceRemove */
+ void handleDbusSignalRemove(sdbusplus::message_t& msg);
};
+
+} // namespace phosphor::virtual_sensor
diff --git a/dbusUtils.cpp b/dbusUtils.cpp
new file mode 100644
index 0000000..ea3879a
--- /dev/null
+++ b/dbusUtils.cpp
@@ -0,0 +1,81 @@
+#include "dbusUtils.hpp"
+
+#include <xyz/openbmc_project/ObjectMapper/common.hpp>
+
+const char* propIntf = "org.freedesktop.DBus.Properties";
+const char* mapperBusName = "xyz.openbmc_project.ObjectMapper";
+const char* mapperPath = "/xyz/openbmc_project/object_mapper";
+const char* mapperIntf =
+ sdbusplus::common::xyz::openbmc_project::ObjectMapper::interface;
+
+const char* methodGetObject = "GetObject";
+const char* methodGet = "Get";
+const char* methodSet = "Set";
+
+using namespace sdbusplus::xyz::openbmc_project::Common::Error;
+
+std::string getService(sdbusplus::bus_t& bus, const std::string& path,
+ const char* intf)
+{
+ /* Get mapper object for sensor path */
+ auto mapper = bus.new_method_call(mapperBusName, mapperPath, mapperIntf,
+ methodGetObject);
+
+ mapper.append(path.c_str());
+ mapper.append(std::vector<std::string>({intf}));
+
+ std::unordered_map<std::string, std::vector<std::string>> resp;
+
+ try
+ {
+ auto msg = bus.call(mapper);
+ msg.read(resp);
+ }
+ catch (const sdbusplus::exception_t& ex)
+ {
+ if (ex.name() == std::string(sdbusplus::xyz::openbmc_project::Common::
+ Error::ResourceNotFound::errName))
+ {
+ // The service isn't on D-Bus yet.
+ return std::string{};
+ }
+ else if (ex.name() == std::string("org.freedesktop.DBus.Error.Timeout"))
+ {
+ lg2::info("Mapper timeout while looking up {PATH}", "PATH", path);
+ return std::string{};
+ }
+
+ throw;
+ }
+
+ if (resp.begin() == resp.end())
+ {
+ // Shouldn't happen, if the mapper can't find it it is handled above.
+ throw std::runtime_error("Unable to find Object: " + path);
+ }
+
+ return resp.begin()->first;
+}
+
+int setDbusProperty(sdbusplus::bus_t& bus, const std::string& service,
+ const std::string& path, const std::string& intf,
+ const std::string& property, const Value& value)
+{
+ try
+ {
+ auto method = bus.new_method_call(service.c_str(), path.c_str(),
+ propIntf, methodSet);
+ method.append(intf, property, value);
+ auto msg = bus.call(method);
+ }
+ catch (const sdbusplus::exception_t& e)
+ {
+ lg2::error(
+ "Faild to set dbus property. service:{SERVICE} path:{PATH} intf:{INTF} Property:{PROP},{ERROR}",
+ "SERVICE", service, "PATH", path, "INTF", intf, "PROP", property,
+ "ERROR", e);
+ return -1;
+ }
+
+ return 0;
+}
diff --git a/dbusUtils.hpp b/dbusUtils.hpp
index 984ac5f..4f99872 100644
--- a/dbusUtils.hpp
+++ b/dbusUtils.hpp
@@ -1,64 +1,15 @@
+#pragma once
+
#include <phosphor-logging/elog-errors.hpp>
#include <phosphor-logging/lg2.hpp>
#include <xyz/openbmc_project/Common/error.hpp>
-const constexpr char* entityManagerBusName =
- "xyz.openbmc_project.EntityManager";
-const char* propIntf = "org.freedesktop.DBus.Properties";
-const char* mapperBusName = "xyz.openbmc_project.ObjectMapper";
-const char* mapperPath = "/xyz/openbmc_project/object_mapper";
-const char* mapperIntf = "xyz.openbmc_project.ObjectMapper";
-
-const char* methodGetObject = "GetObject";
-const char* methodGet = "Get";
-const char* methodSet = "Set";
-
using namespace sdbusplus::xyz::openbmc_project::Common::Error;
using Value = std::variant<int64_t, double, std::string, bool>;
std::string getService(sdbusplus::bus_t& bus, const std::string& path,
- const char* intf)
-{
- /* Get mapper object for sensor path */
- auto mapper = bus.new_method_call(mapperBusName, mapperPath, mapperIntf,
- methodGetObject);
-
- mapper.append(path.c_str());
- mapper.append(std::vector<std::string>({intf}));
-
- std::unordered_map<std::string, std::vector<std::string>> resp;
-
- try
- {
- auto msg = bus.call(mapper);
- msg.read(resp);
- }
- catch (const sdbusplus::exception_t& ex)
- {
- if (ex.name() == std::string(sdbusplus::xyz::openbmc_project::Common::
- Error::ResourceNotFound::errName))
- {
- // The service isn't on D-Bus yet.
- return std::string{};
- }
- else if (ex.name() == std::string("org.freedesktop.DBus.Error.Timeout"))
- {
- lg2::info("Mapper timeout while looking up {PATH}", "PATH", path);
- return std::string{};
- }
-
- throw;
- }
-
- if (resp.begin() == resp.end())
- {
- // Shouldn't happen, if the mapper can't find it it is handled above.
- throw std::runtime_error("Unable to find Object: " + path);
- }
-
- return resp.begin()->first;
-}
+ const char* intf);
template <typename T>
@@ -68,8 +19,8 @@
{
Value value;
- auto method = bus.new_method_call(service.c_str(), path.c_str(), propIntf,
- methodGet);
+ auto method = bus.new_method_call(service.c_str(), path.c_str(),
+ "org.freedesktop.DBus.Properties", "Get");
method.append(intf, property);
@@ -88,23 +39,4 @@
int setDbusProperty(sdbusplus::bus_t& bus, const std::string& service,
const std::string& path, const std::string& intf,
- const std::string& property, const Value& value)
-{
- try
- {
- auto method = bus.new_method_call(service.c_str(), path.c_str(),
- propIntf, methodSet);
- method.append(intf, property, value);
- auto msg = bus.call(method);
- }
- catch (const sdbusplus::exception_t& e)
- {
- lg2::error(
- "Faild to set dbus property. service:{SERVICE} path:{PATH} intf:{INTF} Property:{PROP},{ERRMSG}",
- "SERVICE", service, "PATH", path, "INTF", intf, "PROP", property,
- "ERRMSG", e);
- return -1;
- }
-
- return 0;
-}
+ const std::string& property, const Value& value);
diff --git a/meson.build b/meson.build
index 8265ab5..0ca55ad 100644
--- a/meson.build
+++ b/meson.build
@@ -37,6 +37,8 @@
'virtual-sensor',
[
'virtualSensor.cpp',
+ 'dbusSensor.cpp',
+ 'dbusUtils.cpp',
],
dependencies: [
dependency('phosphor-dbus-interfaces'),
diff --git a/thresholds.hpp b/thresholds.hpp
index be238a1..114138f 100644
--- a/thresholds.hpp
+++ b/thresholds.hpp
@@ -1,12 +1,16 @@
#pragma once
+#include "dbusUtils.hpp"
+
#include <xyz/openbmc_project/Sensor/Threshold/Critical/server.hpp>
#include <xyz/openbmc_project/Sensor/Threshold/HardShutdown/server.hpp>
#include <xyz/openbmc_project/Sensor/Threshold/PerformanceLoss/server.hpp>
#include <xyz/openbmc_project/Sensor/Threshold/SoftShutdown/server.hpp>
#include <xyz/openbmc_project/Sensor/Threshold/Warning/server.hpp>
-namespace phosphor::virtualSensor
+const constexpr char* entityManagerBusName =
+ "xyz.openbmc_project.EntityManager";
+namespace phosphor::virtual_sensor
{
template <typename... T>
@@ -447,4 +451,4 @@
}
};
-} // namespace phosphor::virtualSensor
+} // namespace phosphor::virtual_sensor
diff --git a/virtualSensor.cpp b/virtualSensor.cpp
index e773433..e74ed4c 100644
--- a/virtualSensor.cpp
+++ b/virtualSensor.cpp
@@ -14,34 +14,7 @@
PHOSPHOR_LOG2_USING_WITH_FLAGS;
-int handleDbusSignal(sd_bus_message* msg, void* usrData, sd_bus_error*)
-{
- if (usrData == nullptr)
- {
- throw std::runtime_error("Invalid match");
- }
-
- auto sdbpMsg = sdbusplus::message_t(msg);
- std::string msgIfce;
- std::map<std::string, std::variant<int64_t, double, bool>> msgData;
-
- sdbpMsg.read(msgIfce, msgData);
-
- if (msgData.find("Value") != msgData.end())
- {
- using namespace phosphor::virtualSensor;
- VirtualSensor* obj = static_cast<VirtualSensor*>(usrData);
- // TODO(openbmc/phosphor-virtual-sensor#1): updateVirtualSensor should
- // be changed to take the information we got from the signal, to avoid
- // having to do numerous dbus queries.
- obj->updateVirtualSensor();
- }
- return 0;
-}
-
-namespace phosphor
-{
-namespace virtualSensor
+namespace phosphor::virtual_sensor
{
FuncMaxIgnoreNaN<double> VirtualSensor::funcMaxIgnoreNaN;
@@ -234,7 +207,7 @@
auto sensorObjPath = sensorDbusPath + sensorType + "/" + sensor;
auto paramPtr = std::make_unique<SensorParam>(bus, sensorObjPath,
- this);
+ *this);
symbols.create_variable(sensor);
paramMap.emplace(std::move(sensor), std::move(paramPtr));
}
@@ -357,7 +330,7 @@
auto path = sensorDbusPath + sensorType + "/" + name;
auto paramPtr = std::make_unique<SensorParam>(bus, path,
- this);
+ *this);
std::string paramName = j["ParamName"];
symbols.create_variable(paramName);
paramMap.emplace(std::move(paramName), std::move(paramPtr));
@@ -1057,8 +1030,7 @@
}
}
-} // namespace virtualSensor
-} // namespace phosphor
+} // namespace phosphor::virtual_sensor
/**
* @brief Main
@@ -1073,7 +1045,7 @@
"/xyz/openbmc_project/sensors");
// Create an virtual sensors object
- phosphor::virtualSensor::VirtualSensors virtualSensors(bus);
+ phosphor::virtual_sensor::VirtualSensors virtualSensors(bus);
// Request service bus name
bus.request_name("xyz.openbmc_project.VirtualSensor");
diff --git a/virtualSensor.hpp b/virtualSensor.hpp
index 01e3221..0667246 100644
--- a/virtualSensor.hpp
+++ b/virtualSensor.hpp
@@ -1,3 +1,5 @@
+#pragma once
+
#include "dbusSensor.hpp"
#include "exprtkTools.hpp"
#include "thresholds.hpp"
@@ -11,9 +13,7 @@
#include <map>
#include <string>
-namespace phosphor
-{
-namespace virtualSensor
+namespace phosphor::virtual_sensor
{
PHOSPHOR_LOG2_USING_WITH_FLAGS;
@@ -65,7 +65,8 @@
* @param[in] path - The Dbus path of sensor
* @param[in] ctx - sensor context for update
*/
- SensorParam(sdbusplus::bus_t& bus, const std::string& path, void* ctx) :
+ SensorParam(sdbusplus::bus_t& bus, const std::string& path,
+ VirtualSensor& ctx) :
dbusSensor(std::make_unique<DbusSensor>(bus, path, ctx)),
paramType(dbusParam)
{}
@@ -75,7 +76,9 @@
private:
std::unique_ptr<DbusSensor> dbusSensor = nullptr;
- double value = 0;
+
+ /** @brief virtual sensor value */
+ double value = std::numeric_limits<double>::quiet_NaN();
ParamType paramType;
};
@@ -122,8 +125,10 @@
/** @brief Set sensor value */
void setSensorValue(double value);
+
/** @brief Update sensor at regular intrval */
void updateVirtualSensor();
+
/** @brief Check if sensor value is in valid range */
bool sensorInRange(double value);
@@ -300,5 +305,4 @@
void setupMatches();
};
-} // namespace virtualSensor
-} // namespace phosphor
+} // namespace phosphor::virtual_sensor