Sensor mutability interface
We proposed a ValueMutablity interface in
openbmc/phosphor-dbus-interfaces which was accepted here:
https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/36333
It follows the IPMI fashion, checking
the sensor mutability before writing sensor values. The sensor
mutability used to be hardcoded in the ipmi sensor map yaml file.
This provides feature parity with that old YAML hardcoded
"mutability: Mutability::Write|Mutability::Read" setting.
As an example of implementation within dbus-sensors, ExternalSensor
always sets Mutable to true, given its purpose of accepting sensor
writes from an external source. PwmSensor accepts the "Mutable"
parameter, from entity-manager configuration (aka JSON file). All
other sensors always set Mutable to false, but it would be
straightforward to add similar code, like what was done for
PwmSensor, when mutability is desired.
This parameter will be used by the IPMI server here:
https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-host-ipmid/+/45407
There is currently no Redfish equivalent, although that would
be a welcome task for the future. This is not IPMI-specific,
as the Redfish server can also use this as a hint, as to whether
to allow read-write access, or merely to allow read-only access.
This is not to be confused with the "manufacturing mode" option, which
is designed for use during manufacturing test, hence its name. This
feature is designed for production, and is intended to allow just a
few sensors to be writable, without needing to make them all writable.
Tested:
DBus call on fan sensors with configurable tree mutability:
busctl introspect xyz.openbmc_project.FanSensor /xyz/openbmc_project/sensors/fan_pwm/fan0
NAME TYPE SIGNATURE RESULT/VALUE FLAGS
...
xyz.openbmc_project.Sensor.Value interface - - -
.MaxValue property x 100 emits-change
.MinValue property x 0 emits-change
.Unit property s "xyz.openbmc_project.Sensor.Value.Uni... emits-change
.Value property d 42.7451 emits-change writable
xyz.openbmc_project.Sensor.ValueMutability interface - - -
.Mutable property b true emits-change
...
DBus call on external sensors:
busctl introspect xyz.openbmc_project.ExternalSensor /xyz/openbmc_project/sensors/tempera
NAME TYPE SIGNATURE RESULT/VALUE FLAGS
...
xyz.openbmc_project.Sensor.Value interface - - -
.MaxValue property d 127 emits-change
.MinValue property d -128 emits-change
.Unit property s "DegreesC" emits-change
.Value property d nan emits-change writable
xyz.openbmc_project.Sensor.ValueMutability interface - - -
.Mutable property b true emits-change
...
The ValueMutability interface, with "Mutable", is correctly created.
Signed-off-by: Jie Yang <jjy@google.com>
Change-Id: Ifa1cb51bb55cd6f00d2a2f79e9064d1a51354b06
Signed-off-by: Josh Lehan <krellan@google.com>
diff --git a/include/PwmSensor.hpp b/include/PwmSensor.hpp
index d78b8e9..2c8ebe6 100644
--- a/include/PwmSensor.hpp
+++ b/include/PwmSensor.hpp
@@ -13,7 +13,7 @@
std::shared_ptr<sdbusplus::asio::connection>& conn,
sdbusplus::asio::object_server& objectServer,
const std::string& sensorConfiguration,
- const std::string& sensorType);
+ const std::string& sensorType, bool isValueMutable = false);
~PwmSensor();
private:
@@ -23,6 +23,7 @@
std::shared_ptr<sdbusplus::asio::dbus_interface> sensorInterface;
std::shared_ptr<sdbusplus::asio::dbus_interface> controlInterface;
std::shared_ptr<sdbusplus::asio::dbus_interface> association;
+ std::shared_ptr<sdbusplus::asio::dbus_interface> valueMutabilityInterface;
double pwmMax;
void setValue(uint32_t value);
uint32_t getValue(bool errThrow = true);
diff --git a/include/sensor.hpp b/include/sensor.hpp
index 3df7118..d38bcde 100644
--- a/include/sensor.hpp
+++ b/include/sensor.hpp
@@ -18,6 +18,8 @@
constexpr bool enableInstrumentation = false;
constexpr const char* sensorValueInterface = "xyz.openbmc_project.Sensor.Value";
+constexpr const char* valueMutabilityInterfaceName =
+ "xyz.openbmc_project.Sensor.ValueMutability";
constexpr const char* availableInterfaceName =
"xyz.openbmc_project.State.Decorator.Availability";
constexpr const char* operationalInterfaceName =
@@ -40,13 +42,13 @@
Sensor(const std::string& name,
std::vector<thresholds::Threshold>&& thresholdData,
const std::string& configurationPath, const std::string& objectType,
- bool isSettable, const double max, const double min,
+ bool isSettable, bool isMutable, const double max, const double min,
std::shared_ptr<sdbusplus::asio::connection>& conn,
PowerState readState = PowerState::always) :
name(sensor_paths::escapePathForDbus(name)),
configurationPath(configurationPath), objectType(objectType),
- isSensorSettable(isSettable), maxValue(max), minValue(min),
- thresholds(std::move(thresholdData)),
+ isSensorSettable(isSettable), isValueMutable(isMutable), maxValue(max),
+ minValue(min), thresholds(std::move(thresholdData)),
hysteresisTrigger((max - min) * 0.01),
hysteresisPublish((max - min) * 0.0001), dbusConnection(conn),
readState(readState), errCount(0),
@@ -60,6 +62,13 @@
std::string configurationPath;
std::string objectType;
bool isSensorSettable;
+
+ /* A flag indicates if properties of xyz.openbmc_project.Sensor.Value
+ * interface are mutable. If mutable, then
+ * xyz.openbmc_project.Sensor.ValueMutability interface will be
+ * instantiated.
+ */
+ bool isValueMutable;
double maxValue;
double minValue;
std::vector<thresholds::Threshold> thresholds;
@@ -69,6 +78,7 @@
std::shared_ptr<sdbusplus::asio::dbus_interface> association;
std::shared_ptr<sdbusplus::asio::dbus_interface> availableInterface;
std::shared_ptr<sdbusplus::asio::dbus_interface> operationalInterface;
+ std::shared_ptr<sdbusplus::asio::dbus_interface> valueMutabilityInterface;
double value = std::numeric_limits<double>::quiet_NaN();
double rawValue = std::numeric_limits<double>::quiet_NaN();
bool overriddenState = false;
@@ -316,6 +326,21 @@
std::cerr << "error initializing critical threshold interface\n";
}
+ if (isValueMutable)
+ {
+ valueMutabilityInterface =
+ std::make_shared<sdbusplus::asio::dbus_interface>(
+ conn, sensorInterface->get_object_path(),
+ valueMutabilityInterfaceName);
+ valueMutabilityInterface->register_property("Mutable", true);
+ if (!valueMutabilityInterface->initialize())
+ {
+ std::cerr
+ << "error initializing sensor value mutability interface\n";
+ valueMutabilityInterface = nullptr;
+ }
+ }
+
if (!availableInterface)
{
availableInterface =
diff --git a/src/ADCSensor.cpp b/src/ADCSensor.cpp
index e367c17..2de9ea8 100644
--- a/src/ADCSensor.cpp
+++ b/src/ADCSensor.cpp
@@ -52,7 +52,7 @@
std::optional<BridgeGpio>&& bridgeGpio) :
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
std::move(thresholdsIn), sensorConfiguration,
- "xyz.openbmc_project.Configuration.ADC", false,
+ "xyz.openbmc_project.Configuration.ADC", false, false,
maxVoltageReading / scaleFactor, minVoltageReading / scaleFactor,
conn, readState),
std::enable_shared_from_this<ADCSensor>(), objServer(objectServer),
diff --git a/src/CPUSensor.cpp b/src/CPUSensor.cpp
index 9760664..1ebe6c6 100644
--- a/src/CPUSensor.cpp
+++ b/src/CPUSensor.cpp
@@ -41,8 +41,8 @@
const std::string& sensorConfiguration, int cpuId,
bool show, double dtsOffset) :
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
- std::move(thresholdsIn), sensorConfiguration, objectType, false, 0,
- 0, conn, PowerState::on),
+ std::move(thresholdsIn), sensorConfiguration, objectType, false,
+ false, 0, 0, conn, PowerState::on),
objServer(objectServer), inputDev(io), waitTimer(io), path(path),
privTcontrol(std::numeric_limits<double>::quiet_NaN()),
dtsOffset(dtsOffset), show(show), pollTime(CPUSensor::sensorPollMs),
diff --git a/src/ExitAirTempSensor.cpp b/src/ExitAirTempSensor.cpp
index 0165ffc..a6d1e28 100644
--- a/src/ExitAirTempSensor.cpp
+++ b/src/ExitAirTempSensor.cpp
@@ -164,7 +164,7 @@
std::shared_ptr<ExitAirTempSensor>& parent) :
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
std::move(thresholdData), sensorConfiguration,
- "xyz.openbmc_project.Configuration.ExitAirTemp", false,
+ "xyz.openbmc_project.Configuration.ExitAirTemp", false, false,
cfmMaxReading, cfmMinReading, conn, PowerState::on),
std::enable_shared_from_this<CFMSensor>(), parent(parent),
objServer(objectServer)
@@ -512,7 +512,7 @@
std::vector<thresholds::Threshold>&& thresholdData) :
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
std::move(thresholdData), sensorConfiguration,
- "xyz.openbmc_project.Configuration.ExitAirTemp", false,
+ "xyz.openbmc_project.Configuration.ExitAirTemp", false, false,
exitAirMaxReading, exitAirMinReading, conn, PowerState::on),
std::enable_shared_from_this<ExitAirTempSensor>(), objServer(objectServer)
{
diff --git a/src/ExternalSensor.cpp b/src/ExternalSensor.cpp
index 745c65b..6b42a15 100644
--- a/src/ExternalSensor.cpp
+++ b/src/ExternalSensor.cpp
@@ -27,12 +27,8 @@
std::vector<thresholds::Threshold>&& thresholdsIn,
const std::string& sensorConfiguration, double maxReading,
double minReading, double timeoutSecs, const PowerState& powerState) :
- // TODO(): When the Mutable feature is integrated,
- // make sure all ExternalSensor instances are mutable,
- // because that is the entire point of ExternalSensor,
- // to accept sensor values written by an external source.
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
- std::move(thresholdsIn), sensorConfiguration, objectType, true,
+ std::move(thresholdsIn), sensorConfiguration, objectType, true, true,
maxReading, minReading, conn, powerState),
std::enable_shared_from_this<ExternalSensor>(), objServer(objectServer),
writeLast(std::chrono::steady_clock::now()),
diff --git a/src/FanMain.cpp b/src/FanMain.cpp
index cb0afd3..9f1a21e 100644
--- a/src/FanMain.cpp
+++ b/src/FanMain.cpp
@@ -379,6 +379,8 @@
std::string pwmName;
fs::path pwmPath;
+ // The Mutable parameter is optional, defaulting to false
+ bool isValueMutable = false;
if (connector != sensorData->end())
{
auto findPwm = connector->second.find("Pwm");
@@ -404,6 +406,18 @@
{
pwmName = "Pwm_" + std::to_string(pwm + 1);
}
+
+ // Check PWM sensor mutability
+ auto findMutable = connector->second.find("Mutable");
+ if (findMutable != connector->second.end())
+ {
+ auto ptrMutable =
+ std::get_if<bool>(&(findMutable->second));
+ if (ptrMutable)
+ {
+ isValueMutable = *ptrMutable;
+ }
+ }
}
else
{
@@ -440,7 +454,7 @@
{
pwmSensors[pwmPath] = std::make_unique<PwmSensor>(
pwmName, pwmPath, dbusConnection, objectServer,
- *interfacePath, "Fan");
+ *interfacePath, "Fan", isValueMutable);
}
}
diff --git a/src/HwmonTempSensor.cpp b/src/HwmonTempSensor.cpp
index 9e2d845..d52221a 100644
--- a/src/HwmonTempSensor.cpp
+++ b/src/HwmonTempSensor.cpp
@@ -55,7 +55,7 @@
const std::string& sensorConfiguration, const PowerState powerState) :
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
std::move(thresholdsIn), sensorConfiguration, objectType, false,
- maxReading, minReading, conn, powerState),
+ false, maxReading, minReading, conn, powerState),
std::enable_shared_from_this<HwmonTempSensor>(), objServer(objectServer),
inputDev(io, open(path.c_str(), O_RDONLY)), waitTimer(io), path(path),
sensorPollMs(static_cast<unsigned int>(pollRate * 1000))
diff --git a/src/IpmbSensor.cpp b/src/IpmbSensor.cpp
index edbe05f..7ce3321 100644
--- a/src/IpmbSensor.cpp
+++ b/src/IpmbSensor.cpp
@@ -67,7 +67,7 @@
const float pollRate, std::string& sensorTypeName) :
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
std::move(thresholdData), sensorConfiguration,
- "xyz.openbmc_project.Configuration.ExitAirTemp", false,
+ "xyz.openbmc_project.Configuration.ExitAirTemp", false, false,
ipmbMaxReading, ipmbMinReading, conn, PowerState::on),
deviceAddress(deviceAddress), hostSMbusIndex(hostSMbusIndex),
sensorPollMs(static_cast<int>(pollRate * 1000)), objectServer(objectServer),
diff --git a/src/MCUTempSensor.cpp b/src/MCUTempSensor.cpp
index cdc4fea..8f8aa91 100644
--- a/src/MCUTempSensor.cpp
+++ b/src/MCUTempSensor.cpp
@@ -58,7 +58,7 @@
uint8_t tempReg) :
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
std::move(thresholdData), sensorConfiguration,
- "xyz.openbmc_project.Configuration.ExitAirTemp", false,
+ "xyz.openbmc_project.Configuration.ExitAirTemp", false, false,
mcuTempMaxReading, mcuTempMinReading, conn),
busId(busId), mcuAddress(mcuAddress), tempReg(tempReg),
objectServer(objectServer), waitTimer(io)
diff --git a/src/NVMeSensor.cpp b/src/NVMeSensor.cpp
index 7fa966d..b9e1215 100644
--- a/src/NVMeSensor.cpp
+++ b/src/NVMeSensor.cpp
@@ -31,7 +31,7 @@
const int busNumber) :
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
std::move(thresholdsIn), sensorConfiguration,
- "xyz.openbmc_project.Configuration.NVMe", false, maxReading,
+ "xyz.openbmc_project.Configuration.NVMe", false, false, maxReading,
minReading, conn, PowerState::on),
bus(busNumber), objServer(objectServer)
{
diff --git a/src/PSUSensor.cpp b/src/PSUSensor.cpp
index d221ecb..362155a 100644
--- a/src/PSUSensor.cpp
+++ b/src/PSUSensor.cpp
@@ -47,8 +47,8 @@
double max, double min, double offset,
const std::string& label, size_t tSize, double pollRate) :
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
- std::move(thresholdsIn), sensorConfiguration, objectType, false, max,
- min, conn, powerState),
+ std::move(thresholdsIn), sensorConfiguration, objectType, false,
+ false, max, min, conn, powerState),
std::enable_shared_from_this<PSUSensor>(), objServer(objectServer),
inputDev(io), waitTimer(io), path(path), sensorFactor(factor),
sensorOffset(offset), thresholdTimer(io)
diff --git a/src/PwmSensor.cpp b/src/PwmSensor.cpp
index 7b03e68..c1f0674 100644
--- a/src/PwmSensor.cpp
+++ b/src/PwmSensor.cpp
@@ -31,7 +31,7 @@
std::shared_ptr<sdbusplus::asio::connection>& conn,
sdbusplus::asio::object_server& objectServer,
const std::string& sensorConfiguration,
- const std::string& sensorType) :
+ const std::string& sensorType, bool isValueMutable) :
sysPath(sysPath),
objectServer(objectServer), name(name)
{
@@ -146,9 +146,25 @@
}
return curVal;
});
+
sensorInterface->initialize();
controlInterface->initialize();
+ if (isValueMutable)
+ {
+ valueMutabilityInterface =
+ std::make_shared<sdbusplus::asio::dbus_interface>(
+ conn, sensorInterface->get_object_path(),
+ valueMutabilityInterfaceName);
+ valueMutabilityInterface->register_property("Mutable", true);
+ if (!valueMutabilityInterface->initialize())
+ {
+ std::cerr
+ << "error initializing sensor value mutability interface\n";
+ valueMutabilityInterface = nullptr;
+ }
+ }
+
association = objectServer.add_interface(
"/xyz/openbmc_project/sensors/fan_pwm/" + name, association::interface);
diff --git a/src/TachSensor.cpp b/src/TachSensor.cpp
index 30ab202..483928e 100644
--- a/src/TachSensor.cpp
+++ b/src/TachSensor.cpp
@@ -51,8 +51,8 @@
const PowerState& powerState,
const std::optional<std::string>& ledIn) :
Sensor(boost::replace_all_copy(fanName, " ", "_"), std::move(thresholdsIn),
- sensorConfiguration, objectType, false, limits.second, limits.first,
- conn, powerState),
+ sensorConfiguration, objectType, false, false, limits.second,
+ limits.first, conn, powerState),
objServer(objectServer), redundancy(redundancy),
presence(std::move(presenceSensor)),
inputDev(io, open(path.c_str(), O_RDONLY)), waitTimer(io), path(path),