psusensor: fix psu pwm sensor name
Current bug is that psusensor service does not create pwm sensor object
if pid fan configuration has spaces in Name field, like
{
...
"Name" : "PSU$BUS $ADDRESS Fan 1"
"Outputs": [
"Pwm PSU$BUS $ADDRESS Fan 1"
],
...
}
Fix:
Pwm name need to be escaped, which currently replaces " " with "_"
before being passed to CreatePwmSensor in PSUSensor service.
PwmSensor constructor takes the name as is.
Since the escape algorithm may change in the future,
add a helper function escapeName() and update dbus-sensors
to use this helper funtion when sensors are created.
escapeName() currently calls boost replace_all_copy(name," ", "_"),
so it does not introduce functional changes except the bug identified
above.
Tested:
No regression found for existing working sensors.
Sensors are created as before with the same sensor name.
Fix for non-working case works correctly now.
psu pwm sensor name with space are created correctly with the change.
busctl tree xyz.openbmc_project.PSUSensor
...
|-/xyz/openbmc_project/control
| `-/xyz/openbmc_project/control/fanpwm
| |-/xyz/openbmc_project/control/fanpwm/Pwm_PSU17_2_Fan_1
| |-/xyz/openbmc_project/control/fanpwm/Pwm_PSU17_2_Fan_2
...
Signed-off-by: Zhikui Ren <zhikui.ren@intel.com>
Change-Id: If3b60f93de324e59cc3d774285c23bf50a6f989c
diff --git a/include/Utils.hpp b/include/Utils.hpp
index 61fcdf5..c08daf9 100644
--- a/include/Utils.hpp
+++ b/include/Utils.hpp
@@ -1,6 +1,7 @@
#pragma once
#include <VariantVisitors.hpp>
#include <boost/algorithm/string/predicate.hpp>
+#include <boost/algorithm/string/replace.hpp>
#include <boost/asio/steady_timer.hpp>
#include <boost/container/flat_map.hpp>
#include <sdbusplus/asio/connection.hpp>
@@ -41,6 +42,11 @@
std::vector<std::pair<std::string, std::vector<std::string>>>>>;
using Association = std::tuple<std::string, std::string, std::string>;
+inline std::string escapeName(const std::string& sensorName)
+{
+ return boost::replace_all_copy(sensorName, " ", "_");
+}
+
std::optional<std::string> openAndRead(const std::string& hwmonFile);
std::optional<std::string>
getFullHwmonFilePath(const std::string& directory,
diff --git a/src/ADCSensor.cpp b/src/ADCSensor.cpp
index 2de9ea8..9758168 100644
--- a/src/ADCSensor.cpp
+++ b/src/ADCSensor.cpp
@@ -18,7 +18,6 @@
#include <ADCSensor.hpp>
#include <boost/algorithm/string/predicate.hpp>
-#include <boost/algorithm/string/replace.hpp>
#include <boost/asio/read_until.hpp>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <sdbusplus/asio/connection.hpp>
@@ -50,8 +49,7 @@
PowerState readState,
const std::string& sensorConfiguration,
std::optional<BridgeGpio>&& bridgeGpio) :
- Sensor(boost::replace_all_copy(sensorName, " ", "_"),
- std::move(thresholdsIn), sensorConfiguration,
+ Sensor(escapeName(sensorName), std::move(thresholdsIn), sensorConfiguration,
"xyz.openbmc_project.Configuration.ADC", false, false,
maxVoltageReading / scaleFactor, minVoltageReading / scaleFactor,
conn, readState),
diff --git a/src/CPUSensor.cpp b/src/CPUSensor.cpp
index 1ebe6c6..e955fef 100644
--- a/src/CPUSensor.cpp
+++ b/src/CPUSensor.cpp
@@ -40,9 +40,8 @@
std::vector<thresholds::Threshold>&& thresholdsIn,
const std::string& sensorConfiguration, int cpuId,
bool show, double dtsOffset) :
- Sensor(boost::replace_all_copy(sensorName, " ", "_"),
- std::move(thresholdsIn), sensorConfiguration, objectType, false,
- false, 0, 0, conn, PowerState::on),
+ Sensor(escapeName(sensorName), 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 a6d1e28..fad0020 100644
--- a/src/ExitAirTempSensor.cpp
+++ b/src/ExitAirTempSensor.cpp
@@ -162,10 +162,9 @@
sdbusplus::asio::object_server& objectServer,
std::vector<thresholds::Threshold>&& thresholdData,
std::shared_ptr<ExitAirTempSensor>& parent) :
- Sensor(boost::replace_all_copy(sensorName, " ", "_"),
- std::move(thresholdData), sensorConfiguration,
- "xyz.openbmc_project.Configuration.ExitAirTemp", false, false,
- cfmMaxReading, cfmMinReading, conn, PowerState::on),
+ Sensor(escapeName(sensorName), std::move(thresholdData),
+ sensorConfiguration, "xyz.openbmc_project.Configuration.ExitAirTemp",
+ false, false, cfmMaxReading, cfmMinReading, conn, PowerState::on),
std::enable_shared_from_this<CFMSensor>(), parent(parent),
objServer(objectServer)
{
@@ -510,10 +509,10 @@
const std::string& sensorName, const std::string& sensorConfiguration,
sdbusplus::asio::object_server& objectServer,
std::vector<thresholds::Threshold>&& thresholdData) :
- Sensor(boost::replace_all_copy(sensorName, " ", "_"),
- std::move(thresholdData), sensorConfiguration,
- "xyz.openbmc_project.Configuration.ExitAirTemp", false, false,
- exitAirMaxReading, exitAirMinReading, conn, PowerState::on),
+ Sensor(escapeName(sensorName), std::move(thresholdData),
+ sensorConfiguration, "xyz.openbmc_project.Configuration.ExitAirTemp",
+ false, false, exitAirMaxReading, exitAirMinReading, conn,
+ PowerState::on),
std::enable_shared_from_this<ExitAirTempSensor>(), objServer(objectServer)
{
sensorInterface = objectServer.add_interface(
diff --git a/src/ExternalSensor.cpp b/src/ExternalSensor.cpp
index 6b42a15..961206c 100644
--- a/src/ExternalSensor.cpp
+++ b/src/ExternalSensor.cpp
@@ -5,7 +5,6 @@
#include <unistd.h>
#include <boost/algorithm/string/predicate.hpp>
-#include <boost/algorithm/string/replace.hpp>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <sdbusplus/asio/connection.hpp>
#include <sdbusplus/asio/object_server.hpp>
@@ -27,9 +26,8 @@
std::vector<thresholds::Threshold>&& thresholdsIn,
const std::string& sensorConfiguration, double maxReading,
double minReading, double timeoutSecs, const PowerState& powerState) :
- Sensor(boost::replace_all_copy(sensorName, " ", "_"),
- std::move(thresholdsIn), sensorConfiguration, objectType, true, true,
- maxReading, minReading, conn, powerState),
+ Sensor(escapeName(sensorName), 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()),
writeTimeout(
diff --git a/src/HwmonTempSensor.cpp b/src/HwmonTempSensor.cpp
index d52221a..549fff7 100644
--- a/src/HwmonTempSensor.cpp
+++ b/src/HwmonTempSensor.cpp
@@ -18,7 +18,6 @@
#include <HwmonTempSensor.hpp>
#include <boost/algorithm/string/predicate.hpp>
-#include <boost/algorithm/string/replace.hpp>
#include <boost/asio/read_until.hpp>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <sdbusplus/asio/connection.hpp>
@@ -53,9 +52,8 @@
boost::asio::io_service& io, const std::string& sensorName,
std::vector<thresholds::Threshold>&& thresholdsIn, const float pollRate,
const std::string& sensorConfiguration, const PowerState powerState) :
- Sensor(boost::replace_all_copy(sensorName, " ", "_"),
- std::move(thresholdsIn), sensorConfiguration, objectType, false,
- false, maxReading, minReading, conn, powerState),
+ Sensor(escapeName(sensorName), std::move(thresholdsIn), sensorConfiguration,
+ objectType, false, 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 7ce3321..eedf21e 100644
--- a/src/IpmbSensor.cpp
+++ b/src/IpmbSensor.cpp
@@ -18,7 +18,6 @@
#include <Utils.hpp>
#include <VariantVisitors.hpp>
#include <boost/algorithm/string/predicate.hpp>
-#include <boost/algorithm/string/replace.hpp>
#include <boost/container/flat_map.hpp>
#include <sdbusplus/asio/connection.hpp>
#include <sdbusplus/asio/object_server.hpp>
@@ -65,10 +64,9 @@
std::vector<thresholds::Threshold>&& thresholdData,
uint8_t deviceAddress, uint8_t hostSMbusIndex,
const float pollRate, std::string& sensorTypeName) :
- Sensor(boost::replace_all_copy(sensorName, " ", "_"),
- std::move(thresholdData), sensorConfiguration,
- "xyz.openbmc_project.Configuration.ExitAirTemp", false, false,
- ipmbMaxReading, ipmbMinReading, conn, PowerState::on),
+ Sensor(escapeName(sensorName), std::move(thresholdData),
+ sensorConfiguration, "xyz.openbmc_project.Configuration.ExitAirTemp",
+ false, false, ipmbMaxReading, ipmbMinReading, conn, PowerState::on),
deviceAddress(deviceAddress), hostSMbusIndex(hostSMbusIndex),
sensorPollMs(static_cast<int>(pollRate * 1000)), objectServer(objectServer),
waitTimer(io)
diff --git a/src/MCUTempSensor.cpp b/src/MCUTempSensor.cpp
index 8f8aa91..4f8c848 100644
--- a/src/MCUTempSensor.cpp
+++ b/src/MCUTempSensor.cpp
@@ -17,7 +17,6 @@
#include <Utils.hpp>
#include <VariantVisitors.hpp>
#include <boost/algorithm/string/predicate.hpp>
-#include <boost/algorithm/string/replace.hpp>
#include <boost/container/flat_map.hpp>
#include <sdbusplus/asio/connection.hpp>
#include <sdbusplus/asio/object_server.hpp>
@@ -56,10 +55,9 @@
std::vector<thresholds::Threshold>&& thresholdData,
uint8_t busId, uint8_t mcuAddress,
uint8_t tempReg) :
- Sensor(boost::replace_all_copy(sensorName, " ", "_"),
- std::move(thresholdData), sensorConfiguration,
- "xyz.openbmc_project.Configuration.ExitAirTemp", false, false,
- mcuTempMaxReading, mcuTempMinReading, conn),
+ Sensor(escapeName(sensorName), std::move(thresholdData),
+ sensorConfiguration, "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 b9e1215..5f66eb3 100644
--- a/src/NVMeSensor.cpp
+++ b/src/NVMeSensor.cpp
@@ -15,7 +15,6 @@
*/
#include <NVMeSensor.hpp>
-#include <boost/algorithm/string/replace.hpp>
#include <iostream>
@@ -29,8 +28,7 @@
std::vector<thresholds::Threshold>&& thresholdsIn,
const std::string& sensorConfiguration,
const int busNumber) :
- Sensor(boost::replace_all_copy(sensorName, " ", "_"),
- std::move(thresholdsIn), sensorConfiguration,
+ Sensor(escapeName(sensorName), std::move(thresholdsIn), sensorConfiguration,
"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 362155a..4e0d32b 100644
--- a/src/PSUSensor.cpp
+++ b/src/PSUSensor.cpp
@@ -18,7 +18,6 @@
#include <PSUSensor.hpp>
#include <boost/algorithm/string/predicate.hpp>
-#include <boost/algorithm/string/replace.hpp>
#include <boost/asio/read_until.hpp>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <sdbusplus/asio/connection.hpp>
@@ -46,9 +45,8 @@
const std::string& sensorUnits, unsigned int factor,
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,
- false, max, min, conn, powerState),
+ Sensor(escapeName(sensorName), 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/PSUSensorMain.cpp b/src/PSUSensorMain.cpp
index fd151d9..779873e 100644
--- a/src/PSUSensorMain.cpp
+++ b/src/PSUSensorMain.cpp
@@ -425,8 +425,7 @@
// on rescans, only update sensors we were signaled by
if (!firstScan)
{
- std::string psuNameStr =
- "/" + boost::replace_all_copy(*psuName, " ", "_");
+ std::string psuNameStr = "/" + escapeName(*psuName);
auto it =
std::find_if(sensorsChanged->begin(), sensorsChanged->end(),
[psuNameStr](std::string& s) {
@@ -458,7 +457,8 @@
do
{
// Individual string fields: Name, Name1, Name2, Name3, ...
- psuNames.push_back(std::get<std::string>(findPSUName->second));
+ psuNames.push_back(
+ escapeName(std::get<std::string>(findPSUName->second)));
findPSUName = baseConfig->second.find("Name" + std::to_string(i++));
} while (findPSUName != baseConfig->second.end());
diff --git a/src/TachSensor.cpp b/src/TachSensor.cpp
index 483928e..c375dbf 100644
--- a/src/TachSensor.cpp
+++ b/src/TachSensor.cpp
@@ -19,7 +19,6 @@
#include <TachSensor.hpp>
#include <Utils.hpp>
#include <boost/algorithm/string/predicate.hpp>
-#include <boost/algorithm/string/replace.hpp>
#include <boost/asio/read_until.hpp>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <gpiod.hpp>
@@ -50,9 +49,9 @@
const std::pair<size_t, size_t>& limits,
const PowerState& powerState,
const std::optional<std::string>& ledIn) :
- Sensor(boost::replace_all_copy(fanName, " ", "_"), std::move(thresholdsIn),
- sensorConfiguration, objectType, false, false, limits.second,
- limits.first, conn, powerState),
+ Sensor(escapeName(fanName), std::move(thresholdsIn), 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),
@@ -184,7 +183,8 @@
void TachSensor::checkThresholds(void)
{
- bool status = thresholds::checkThresholds(this);
+ // WA - treat value <= 0 as not present
+ bool status = false;
if (redundancy && *redundancy)
{