sensor: Always add status interface, track fault file with class member
Always add the status interface regardless of whether there is a
fault file or not and update the functional property accordingly.
Added a new sensor class member to track the presence of fault file.
Resolves: openbmc/phosphor-hwmon#10
Signed-off-by: Brandon Kim <brandonkim@google.com>
Change-Id: I4480541691ccfa853d85151965428c564db8ba52
diff --git a/mainloop.cpp b/mainloop.cpp
index a5a818f..6908d78 100644
--- a/mainloop.cpp
+++ b/mainloop.cpp
@@ -377,31 +377,35 @@
int64_t value;
auto& objInfo = std::get<ObjectInfo>(i.second);
auto& obj = std::get<InterfaceMap>(objInfo);
+ std::unique_ptr<sensor::Sensor>& sensor =
+ _sensorObjects[i.first];
- auto it = obj.find(InterfaceType::STATUS);
- if (it != obj.end())
+ auto& statusIface =
+ std::any_cast<std::shared_ptr<StatusObject>&>(
+ obj[InterfaceType::STATUS]);
+ // As long as addStatus is called before addValue, statusIface
+ // should never be nullptr.
+ assert(statusIface);
+
+ if (sensor->hasFaultFile())
{
auto fault = _ioAccess->read(
i.first.first, i.first.second, hwmon::entry::fault,
hwmonio::retries, hwmonio::delay);
- auto statusIface =
- std::any_cast<std::shared_ptr<StatusObject>>(
- it->second);
+ // Skip reading from a sensor with a valid fault file
+ // and set the functional property accordingly
if (!statusIface->functional((fault == 0) ? true : false))
{
continue;
}
}
- // Retry for up to a second if device is busy
- // or has a transient error.
- std::unique_ptr<sensor::Sensor>& sensor =
- _sensorObjects[i.first];
-
{
// RAII object for GPIO unlock / lock
sensor::GpioLock gpioLock(sensor->getGpio());
+ // Retry for up to a second if device is busy
+ // or has a transient error.
value =
_ioAccess->read(i.first.first, i.first.second, input,
hwmonio::retries, hwmonio::delay);
diff --git a/sensor.cpp b/sensor.cpp
index a9e4454..a88410b 100644
--- a/sensor.cpp
+++ b/sensor.cpp
@@ -8,6 +8,7 @@
#include "sensorset.hpp"
#include "sysfs.hpp"
+#include <cassert>
#include <cmath>
#include <cstring>
#include <filesystem>
@@ -39,7 +40,7 @@
const hwmonio::HwmonIOInterface* ioAccess,
const std::string& devPath) :
_sensor(sensor),
- _ioAccess(ioAccess), _devPath(devPath), _scale(0)
+ _ioAccess(ioAccess), _devPath(devPath), _scale(0), _hasFaultFile(false)
{
auto chip = env::getEnv("GPIOCHIP", sensor);
auto access = env::getEnv("GPIO", sensor);
@@ -135,16 +136,15 @@
auto& objPath = std::get<std::string>(info);
SensorValueType val = 0;
- std::shared_ptr<StatusObject> statusIface = nullptr;
- auto it = obj.find(InterfaceType::STATUS);
- if (it != obj.end())
- {
- statusIface = std::any_cast<std::shared_ptr<StatusObject>>(it->second);
- }
- // If there's no fault file or the sensor has a fault file and
- // its status is functional, read the input value.
- if (!statusIface || statusIface->functional())
+ auto& statusIface = std::any_cast<std::shared_ptr<StatusObject>&>(
+ obj[InterfaceType::STATUS]);
+ // As long as addStatus is called before addValue, statusIface
+ // should never be nullptr
+ assert(statusIface);
+
+ // Only read the input value if the status is functional
+ if (statusIface->functional())
{
// RAII object for GPIO unlock / lock
GpioLock gpioLock(getGpio());
@@ -200,11 +200,12 @@
std::string faultID = _sensor.second;
std::string entry = hwmon::entry::fault;
+ bool functional = true;
auto sysfsFullPath =
sysfs::make_sysfs_path(_ioAccess->path(), faultName, faultID, entry);
if (fs::exists(sysfsFullPath))
{
- bool functional = true;
+ _hasFaultFile = true;
try
{
uint32_t fault = _ioAccess->read(faultName, faultID, entry,
@@ -228,18 +229,17 @@
"Logging failing sysfs file",
phosphor::logging::entry("FILE=%s", sysfsFullPath.c_str()));
}
-
- static constexpr bool deferSignals = true;
- auto& bus = *std::get<sdbusplus::bus::bus*>(info);
-
- iface =
- std::make_shared<StatusObject>(bus, objPath.c_str(), deferSignals);
- // Set functional property
- iface->functional(functional);
-
- obj[InterfaceType::STATUS] = iface;
}
+ static constexpr bool deferSignals = true;
+ auto& bus = *std::get<sdbusplus::bus::bus*>(info);
+
+ iface = std::make_shared<StatusObject>(bus, objPath.c_str(), deferSignals);
+ // Set functional property
+ iface->functional(functional);
+
+ obj[InterfaceType::STATUS] = iface;
+
return iface;
}
diff --git a/sensor.hpp b/sensor.hpp
index 2a53af8..ac818d0 100644
--- a/sensor.hpp
+++ b/sensor.hpp
@@ -93,10 +93,10 @@
/**
* @brief Add status interface and functional property for sensor
- * @details When a sensor has an associated fault file, the
- * OperationalStatus interface is added along with setting the
- * Functional property to the corresponding value found in the
- * fault file.
+ * @details OperationalStatus interface is added and the Functional property
+ * is set depending on whether a fault file exists and if it does it will
+ * also depend on the content of the fault file. _hasFaultFile will also be
+ * set to true if fault file exists.
*
* @param[in] info - Sensor object information
*
@@ -124,6 +124,16 @@
return _handle.get();
}
+ /**
+ * @brief Get whether the sensor has a fault file or not.
+ *
+ * @return - Boolean on whether the sensor has a fault file
+ */
+ inline const bool hasFaultFile(void)
+ {
+ return _hasFaultFile;
+ }
+
private:
/** @brief Sensor object's identifiers */
SensorSet::key_type _sensor;
@@ -142,6 +152,9 @@
/** @brief sensor scale from configuration. */
int64_t _scale;
+
+ /** @brief Tracks whether the sensor has a fault file or not. */
+ bool _hasFaultFile;
};
/** @class GpioLock