regulators: Remove SensorReading data type
Remove the SensorReading data type defined in pmbus_utils.hpp and used
in several locations. This data type is no longer needed.
The original design was for sensor readings to be accumulated in the
ActionEnvironment while the PMBus actions were being executed. Then all
the sensor readings for a voltage rail would be published on D-Bus at
the same time.
The current design is for sensor readings to be immediately published on
D-Bus as they are read using the new Sensors service. They no longer
need to be accumulated in the ActionEnvironment.
Signed-off-by: Shawn McCarney <shawnmm@us.ibm.com>
Change-Id: Ib61a07b5a7f58be3e2dae540ffb4d5128636b7c0
diff --git a/phosphor-regulators/src/actions/action_environment.hpp b/phosphor-regulators/src/actions/action_environment.hpp
index bfd1b37..72aa8f4 100644
--- a/phosphor-regulators/src/actions/action_environment.hpp
+++ b/phosphor-regulators/src/actions/action_environment.hpp
@@ -16,14 +16,12 @@
#pragma once
#include "id_map.hpp"
-#include "pmbus_utils.hpp"
#include "services.hpp"
#include <cstddef> // for size_t
#include <optional>
#include <stdexcept>
#include <string>
-#include <vector>
namespace phosphor::power::regulators
{
@@ -42,7 +40,6 @@
* - current volts value (if any)
* - mapping from device and rule IDs to the corresponding objects
* - rule call stack depth (to detect infinite recursion)
- * - sensor readings
*/
class ActionEnvironment
{
@@ -75,16 +72,6 @@
}
/**
- * Adds the specified sensor reading to this action environment.
- *
- * @param reading sensor reading from a regulator rail
- */
- void addSensorReading(const pmbus_utils::SensorReading& reading)
- {
- sensorReadings.emplace_back(reading);
- }
-
- /**
* Decrements the rule call stack depth by one.
*
* Should be used when a call to a rule returns. Does nothing if depth is
@@ -146,16 +133,6 @@
}
/**
- * Returns the sensor readings stored in this action environment.
- *
- * @return sensor readings
- */
- const std::vector<pmbus_utils::SensorReading>& getSensorReadings() const
- {
- return sensorReadings;
- }
-
- /**
* Returns the services in this action environment.
*
* @return system services
@@ -241,11 +218,6 @@
* Rule call stack depth.
*/
size_t ruleDepth{0};
-
- /**
- * Sensor readings for a single regulator rail.
- */
- std::vector<pmbus_utils::SensorReading> sensorReadings{};
};
} // namespace phosphor::power::regulators
diff --git a/phosphor-regulators/src/actions/pmbus_read_sensor_action.cpp b/phosphor-regulators/src/actions/pmbus_read_sensor_action.cpp
index f8a15d7..fd58ae2 100644
--- a/phosphor-regulators/src/actions/pmbus_read_sensor_action.cpp
+++ b/phosphor-regulators/src/actions/pmbus_read_sensor_action.cpp
@@ -30,9 +30,6 @@
{
try
{
- pmbus_utils::SensorReading reading{};
- reading.type = type;
-
// Get I2C interface to current device
i2c::I2CInterface& interface = getI2CInterface(environment);
@@ -40,23 +37,25 @@
uint16_t value;
interface.read(command, value);
+ // Convert PMBus 2-byte value to a double
+ // TODO: Store result of conversion in a double. Not doing it yet
+ // because it results in an unused variable compile error during CI.
switch (format)
{
case pmbus_utils::SensorDataFormat::linear_11:
// Convert linear_11 format to a normal decimal number
- reading.value = pmbus_utils::convertFromLinear(value);
+ pmbus_utils::convertFromLinear(value);
break;
case pmbus_utils::SensorDataFormat::linear_16:
// Get exponent value for converting linear format to volts
int8_t exponentValue = getExponentValue(environment, interface);
// Convert linear_16 format to a normal decimal number
- reading.value =
- pmbus_utils::convertFromVoutLinear(value, exponentValue);
+ pmbus_utils::convertFromVoutLinear(value, exponentValue);
break;
}
- environment.addSensorReading(reading);
+ // TODO: Publish sensor value using Sensors service
}
// Nest the following exception types within an ActionError so the caller
// will have both the low level error information and the action information
diff --git a/phosphor-regulators/src/pmbus_utils.hpp b/phosphor-regulators/src/pmbus_utils.hpp
index 4d4b047..438ee46 100644
--- a/phosphor-regulators/src/pmbus_utils.hpp
+++ b/phosphor-regulators/src/pmbus_utils.hpp
@@ -142,22 +142,6 @@
};
/**
- * This struct represents one sensor reading from a device.
- */
-struct SensorReading
-{
- /**
- * Sensor value type that was read.
- */
- SensorValueType type;
-
- /**
- * Sensor value that was read.
- */
- double value;
-};
-
-/**
* Parse the one byte value of the VOUT_MODE command.
*
* VOUT_MODE contains a 'mode' field that indicates the data format used for
diff --git a/phosphor-regulators/test/actions/action_environment_tests.cpp b/phosphor-regulators/test/actions/action_environment_tests.cpp
index ffb75e9..de52845 100644
--- a/phosphor-regulators/test/actions/action_environment_tests.cpp
+++ b/phosphor-regulators/test/actions/action_environment_tests.cpp
@@ -19,7 +19,6 @@
#include "id_map.hpp"
#include "mock_services.hpp"
#include "mocked_i2c_interface.hpp"
-#include "pmbus_utils.hpp"
#include "rule.hpp"
#include <cstddef> // for size_t
@@ -65,29 +64,6 @@
}
}
-TEST(ActionEnvironmentTests, AddSensorReading)
-{
- IDMap idMap{};
- MockServices services{};
- ActionEnvironment env{idMap, "", services};
- pmbus_utils::SensorReading reading;
- reading.type = pmbus_utils::SensorValueType::iout;
- reading.value = 1;
- EXPECT_EQ(env.getSensorReadings().size(), 0);
- env.addSensorReading(reading);
- EXPECT_EQ(env.getSensorReadings().size(), 1);
- EXPECT_EQ(env.getSensorReadings()[0].type,
- pmbus_utils::SensorValueType::iout);
- EXPECT_EQ(env.getSensorReadings()[0].value, 1);
- reading.type = pmbus_utils::SensorValueType::vout;
- reading.value = 2;
- env.addSensorReading(reading);
- EXPECT_EQ(env.getSensorReadings().size(), 2);
- EXPECT_EQ(env.getSensorReadings()[1].type,
- pmbus_utils::SensorValueType::vout);
- EXPECT_EQ(env.getSensorReadings()[1].value, 2);
-}
-
TEST(ActionEnvironmentTests, DecrementRuleDepth)
{
IDMap idMap{};
@@ -223,29 +199,6 @@
EXPECT_EQ(env.getRuleDepth(), 0);
}
-TEST(ActionEnvironmentTests, GetSensorReadings)
-{
- IDMap idMap{};
- MockServices services{};
- ActionEnvironment env{idMap, "", services};
- pmbus_utils::SensorReading reading;
- reading.type = pmbus_utils::SensorValueType::pout;
- reading.value = 1.3;
- EXPECT_EQ(env.getSensorReadings().size(), 0);
- env.addSensorReading(reading);
- EXPECT_EQ(env.getSensorReadings().size(), 1);
- EXPECT_EQ(env.getSensorReadings()[0].type,
- pmbus_utils::SensorValueType::pout);
- EXPECT_EQ(env.getSensorReadings()[0].value, 1.3);
- reading.type = pmbus_utils::SensorValueType::temperature;
- reading.value = -1;
- env.addSensorReading(reading);
- EXPECT_EQ(env.getSensorReadings().size(), 2);
- EXPECT_EQ(env.getSensorReadings()[1].type,
- pmbus_utils::SensorValueType::temperature);
- EXPECT_EQ(env.getSensorReadings()[1].value, -1);
-}
-
TEST(ActionEnvironmentTests, GetServices)
{
IDMap idMap{};
diff --git a/phosphor-regulators/test/actions/pmbus_read_sensor_action_tests.cpp b/phosphor-regulators/test/actions/pmbus_read_sensor_action_tests.cpp
index d0f1398..09edfe7 100644
--- a/phosphor-regulators/test/actions/pmbus_read_sensor_action_tests.cpp
+++ b/phosphor-regulators/test/actions/pmbus_read_sensor_action_tests.cpp
@@ -122,10 +122,14 @@
std::optional<int8_t> exponent{};
PMBusReadSensorAction action{type, command, format, exponent};
EXPECT_EQ(action.execute(env), true);
- EXPECT_EQ(env.getSensorReadings().size(), 1);
- EXPECT_EQ(env.getSensorReadings()[0].type,
- pmbus_utils::SensorValueType::iout);
- EXPECT_DOUBLE_EQ(env.getSensorReadings()[0].value, 11.5);
+ /**
+ * TODO: Replace with EXPECT calls using MockSensors
+ *
+ * EXPECT_EQ(env.getSensorReadings().size(), 1);
+ * EXPECT_EQ(env.getSensorReadings()[0].type,
+ * pmbus_utils::SensorValueType::iout);
+ * EXPECT_DOUBLE_EQ(env.getSensorReadings()[0].value, 11.5);
+ */
}
catch (...)
{
@@ -167,10 +171,14 @@
std::optional<int8_t> exponent{3};
PMBusReadSensorAction action{type, command, format, exponent};
EXPECT_EQ(action.execute(env), true);
- EXPECT_EQ(env.getSensorReadings().size(), 1);
- EXPECT_EQ(env.getSensorReadings()[0].type,
- pmbus_utils::SensorValueType::vout);
- EXPECT_DOUBLE_EQ(env.getSensorReadings()[0].value, 16);
+ /**
+ * TODO: Replace with EXPECT calls using MockSensors
+ *
+ * EXPECT_EQ(env.getSensorReadings().size(), 1);
+ * EXPECT_EQ(env.getSensorReadings()[0].type,
+ * pmbus_utils::SensorValueType::vout);
+ * EXPECT_DOUBLE_EQ(env.getSensorReadings()[0].value, 16);
+ */
}
catch (...)
{
@@ -213,10 +221,14 @@
std::optional<int8_t> exponent{};
PMBusReadSensorAction action{type, command, format, exponent};
EXPECT_EQ(action.execute(env), true);
- EXPECT_EQ(env.getSensorReadings().size(), 1);
- EXPECT_EQ(env.getSensorReadings()[0].type,
- pmbus_utils::SensorValueType::vout_peak);
- EXPECT_DOUBLE_EQ(env.getSensorReadings()[0].value, 0.232421875);
+ /**
+ * TODO: Replace with EXPECT calls using MockSensors
+ *
+ * EXPECT_EQ(env.getSensorReadings().size(), 1);
+ * EXPECT_EQ(env.getSensorReadings()[0].type,
+ * pmbus_utils::SensorValueType::vout_peak);
+ * EXPECT_DOUBLE_EQ(env.getSensorReadings()[0].value, 0.232421875);
+ */
}
catch (...)
{