Fix sensor override
c1d019a6056a2a0ef50e577b3139ab5a8dc49355 Sensor Optimization
Recently changed the way Ids were calculated in the sensor subsystem.
Unfortunately, it wasn't clear to the author that this would effect the
sensor override system, which relies on matching up a member ID with a
dbus path, and was broken by this change.
This commit breaks out the code to calculate the type and name from a
given URI segment into a helper method.
Tested: Inspection only. Very few systems support this feature. Code appears more correct than previously, which is known broken, so the lack of testing here seems reasonable.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9aa8099a947a36b5ce914bc07ae60f1ebf0d209b
diff --git a/meson.build b/meson.build
index 967b309..997dd1d 100644
--- a/meson.build
+++ b/meson.build
@@ -367,6 +367,7 @@
'test/redfish-core/include/utils/stl_utils_test.cpp',
'test/redfish-core/include/utils/time_utils_test.cpp',
'test/redfish-core/lib/chassis_test.cpp',
+ 'test/redfish-core/lib/sensors_test.cpp',
'test/redfish-core/lib/log_services_dump_test.cpp',
'test/redfish-core/lib/service_root_test.cpp',
'test/redfish-core/lib/thermal_subsystem_test.cpp',
diff --git a/redfish-core/lib/sensors.hpp b/redfish-core/lib/sensors.hpp
index 627ba7e..e53a7d2 100644
--- a/redfish-core/lib/sensors.hpp
+++ b/redfish-core/lib/sensors.hpp
@@ -21,6 +21,7 @@
#include <boost/algorithm/string/classification.hpp>
#include <boost/algorithm/string/find.hpp>
#include <boost/algorithm/string/predicate.hpp>
+#include <boost/algorithm/string/replace.hpp>
#include <boost/algorithm/string/split.hpp>
#include <boost/range/algorithm/replace_copy_if.hpp>
#include <dbus_singleton.hpp>
@@ -732,7 +733,7 @@
{
if (chassisSubNode == sensors::node::sensors)
{
- std::string subNodeEscaped(chassisSubNode);
+ std::string subNodeEscaped(sensorType);
subNodeEscaped.erase(
std::remove(subNodeEscaped.begin(), subNodeEscaped.end(), '_'),
subNodeEscaped.end());
@@ -2578,6 +2579,24 @@
return false;
}
+inline std::pair<std::string, std::string>
+ splitSensorNameAndType(std::string_view sensorId)
+{
+ size_t index = sensorId.find('_');
+ if (index == std::string::npos)
+ {
+ return std::make_pair<std::string, std::string>("", "");
+ }
+ std::string sensorType{sensorId.substr(0, index)};
+ std::string sensorName{sensorId.substr(index + 1)};
+ // fan_pwm and fan_tach need special handling
+ if (sensorType == "fantach" || sensorType == "fanpwm")
+ {
+ sensorType.insert(3, 1, '_');
+ }
+ return std::make_pair(sensorType, sensorName);
+}
+
/**
* @brief Entry point for overriding sensor values of given sensor
*
@@ -2634,8 +2653,10 @@
for (const auto& item : overrideMap)
{
const auto& sensor = item.first;
- if (!findSensorNameUsingSensorPath(sensor, *sensorsList,
- *sensorNames))
+ std::pair<std::string, std::string> sensorNameType =
+ splitSensorNameAndType(sensor);
+ if (!findSensorNameUsingSensorPath(sensorNameType.second,
+ *sensorsList, *sensorNames))
{
BMCWEB_LOG_INFO << "Unable to find memberId " << item.first;
messages::resourceNotFound(sensorAsyncResp->asyncResp->res,
@@ -2876,33 +2897,28 @@
{
return;
}
- size_t index = sensorId.find('_');
- if (index == std::string::npos)
+ std::pair<std::string, std::string> nameType =
+ splitSensorNameAndType(sensorId);
+ if (nameType.first.empty() || nameType.second.empty())
{
messages::resourceNotFound(asyncResp->res, sensorId, "Sensor");
return;
}
+
asyncResp->res.jsonValue["@odata.id"] = crow::utility::urlFromPieces(
"redfish", "v1", "Chassis", chassisId, "Sensors", sensorId);
- std::string sensorType = sensorId.substr(0, index);
- std::string sensorName = sensorId.substr(index + 1);
- // fan_pwm and fan_tach need special handling
- if (sensorType == "fantach" || sensorType == "fanpwm")
- {
- sensorType.insert(3, 1, '_');
- }
BMCWEB_LOG_DEBUG << "Sensor doGet enter";
const std::array<const char*, 1> interfaces = {
"xyz.openbmc_project.Sensor.Value"};
- std::string sensorPath =
- "/xyz/openbmc_project/sensors/" + sensorType + '/' + sensorName;
+ std::string sensorPath = "/xyz/openbmc_project/sensors/" + nameType.first +
+ '/' + nameType.second;
// Get a list of all of the sensors that implement Sensor.Value
// and get the path and service name associated with the sensor
crow::connections::systemBus->async_method_call(
- [asyncResp, sensorPath,
- sensorName](const boost::system::error_code ec,
+ [asyncResp,
+ sensorPath](const boost::system::error_code ec,
const ::dbus::utility::MapperGetObject& subtree) {
BMCWEB_LOG_DEBUG << "respHandler1 enter";
if (ec)
diff --git a/test/redfish-core/lib/sensors_test.cpp b/test/redfish-core/lib/sensors_test.cpp
new file mode 100644
index 0000000..591b911
--- /dev/null
+++ b/test/redfish-core/lib/sensors_test.cpp
@@ -0,0 +1,36 @@
+#include "sensors.hpp"
+
+#include <gmock/gmock.h> // IWYU pragma: keep
+#include <gtest/gtest.h> // IWYU pragma: keep
+
+// IWYU pragma: no_include <gtest/gtest-message.h>
+// IWYU pragma: no_include <gtest/gtest-test-part.h>
+// IWYU pragma: no_include "gtest/gtest_pred_impl.h"
+// IWYU pragma: no_include <gmock/gmock-matchers.h>
+// IWYU pragma: no_include <gtest/gtest-matchers.h>
+
+namespace redfish
+{
+namespace
+{
+
+TEST(SplitSensorNameAndType, Type)
+{
+ EXPECT_EQ(splitSensorNameAndType("fantach_foo_1").first, "fan_tach");
+ EXPECT_EQ(splitSensorNameAndType("temperature_foo2").first, "temperature");
+}
+
+TEST(SplitSensorNameAndType, Name)
+{
+ EXPECT_EQ(splitSensorNameAndType("fantach_foo_1").second, "foo_1");
+ EXPECT_EQ(splitSensorNameAndType("temperature_foo2").second, "foo2");
+}
+
+TEST(SplitSensorNameAndType, Error)
+{
+ EXPECT_TRUE(splitSensorNameAndType("fantach").first.empty());
+ EXPECT_TRUE(splitSensorNameAndType("temperature").second.empty());
+}
+
+} // namespace
+} // namespace redfish