regulators: Add PMBusReadSensorAction::execute()
Implementation and Testing for PMBusReadSensorAction::execute().
Signed-off-by: Bob King <Bob_King@wistron.com>
Change-Id: I350eccfd6daaf957865329756cdfb3be48f49377
diff --git a/phosphor-regulators/src/actions/pmbus_read_sensor_action.cpp b/phosphor-regulators/src/actions/pmbus_read_sensor_action.cpp
new file mode 100644
index 0000000..4cc6a69
--- /dev/null
+++ b/phosphor-regulators/src/actions/pmbus_read_sensor_action.cpp
@@ -0,0 +1,122 @@
+/**
+ * Copyright © 2020 IBM Corporation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "pmbus_read_sensor_action.hpp"
+
+#include "action_error.hpp"
+#include "pmbus_error.hpp"
+
+#include <exception>
+#include <ios>
+#include <sstream>
+
+namespace phosphor::power::regulators
+{
+
+bool PMBusReadSensorAction::execute(ActionEnvironment& environment)
+{
+ try
+ {
+ pmbus_utils::SensorReading reading{};
+ reading.type = type;
+
+ // Get I2C interface to current device
+ i2c::I2CInterface& interface = getI2CInterface(environment);
+
+ // Read value of the PMBus command code
+ uint16_t value;
+ interface.read(command, value);
+
+ switch (format)
+ {
+ case pmbus_utils::SensorDataFormat::linear_11:
+ // Convert linear_11 format to a normal decimal number
+ reading.value = pmbus_utils::convertFromLinear(value);
+ break;
+ case pmbus_utils::SensorDataFormat::linear_16:
+ // Get exponent value for converting linear format to volts
+ // value
+ int8_t exponentValue = getExponentValue(interface);
+
+ // Convert linear_16 format to a normal decimal number
+ reading.value =
+ pmbus_utils::convertFromVoutLinear(value, exponentValue);
+ break;
+ }
+
+ environment.addSensorReading(reading);
+ }
+ // Nest the following exception types within an ActionError so the caller
+ // will have both the low level error information and the action information
+ catch (const i2c::I2CException& i2cError)
+ {
+ std::throw_with_nested(ActionError(*this));
+ }
+ catch (const PMBusError& pmbusError)
+ {
+ std::throw_with_nested(ActionError(*this));
+ }
+ return true;
+}
+
+std::string PMBusReadSensorAction::toString() const
+{
+ std::ostringstream ss;
+ ss << "pmbus_read_sensor: { ";
+ ss << "type: " << pmbus_utils::toString(type) << ", " << std::hex
+ << std::uppercase;
+ ss << "command: 0x" << static_cast<uint16_t>(command) << ", " << std::dec
+ << std::nouppercase;
+ ss << "format: " << pmbus_utils::toString(format);
+
+ if (exponent.has_value())
+ {
+ ss << ", exponent: " << static_cast<int16_t>(exponent.value());
+ }
+
+ ss << " }";
+
+ return ss.str();
+}
+
+int8_t PMBusReadSensorAction::getExponentValue(i2c::I2CInterface& interface)
+{
+ // Check if an exponent value is defined for this action
+ if (exponent.has_value())
+ {
+ return exponent.value();
+ }
+
+ // Read value of the VOUT_MODE command
+ uint8_t voutModeValue;
+ interface.read(pmbus_utils::VOUT_MODE, voutModeValue);
+
+ // Parse VOUT_MODE value to get data format and parameter value
+ pmbus_utils::VoutDataFormat format;
+ int8_t parameter;
+ pmbus_utils::parseVoutMode(voutModeValue, format, parameter);
+
+ // Verify format is linear; other formats not currently supported
+ if (format != pmbus_utils::VoutDataFormat::linear)
+ {
+ throw PMBusError("VOUT_MODE contains unsupported data format");
+ }
+
+ // Return parameter value; it contains the exponent when format is linear
+ return parameter;
+}
+
+} // namespace phosphor::power::regulators
diff --git a/phosphor-regulators/src/actions/pmbus_read_sensor_action.hpp b/phosphor-regulators/src/actions/pmbus_read_sensor_action.hpp
index 8737eba..c39fd8a 100644
--- a/phosphor-regulators/src/actions/pmbus_read_sensor_action.hpp
+++ b/phosphor-regulators/src/actions/pmbus_read_sensor_action.hpp
@@ -22,7 +22,6 @@
#include <cstdint>
#include <optional>
-#include <stdexcept>
#include <string>
namespace phosphor::power::regulators
@@ -78,16 +77,29 @@
/**
* Executes this action.
*
- * TODO: Not implemented yet
+ * Reads one sensor using the I2C interface.
+ *
+ * The sensor value type is specified in the constructor.
+ *
+ * The PMBus command code is specified in the constructor.
+ * It is the register to read on the device.
+ *
+ * The sensor data format is specified in the constructor. Currently
+ * the linear_11 and linear_16 formats are supported.
+ *
+ * The linear_16 data format requires an exponent value.
+ * If an exponent value was specified in the constructor, that
+ * value will be used. Otherwise the exponent value will be obtained from
+ * the VOUT_MODE command.
+ *
+ * The device is obtained from the ActionEnvironment.
+ *
+ * Throws an exception if an error occurs.
*
* @param environment Action execution environment.
* @return true
*/
- virtual bool execute(ActionEnvironment& /* environment */) override
- {
- // TODO: Not implemented yet
- return true;
- }
+ virtual bool execute(ActionEnvironment& environment) override;
/**
* Returns the PMBus command code.
@@ -134,28 +146,25 @@
*
* @return description of action
*/
- virtual std::string toString() const override
- {
- std::ostringstream ss;
- ss << "pmbus_read_sensor: { ";
- ss << "type: " << pmbus_utils::toString(type) << ", " << std::hex
- << std::uppercase;
- ss << "command: 0x" << static_cast<uint16_t>(command) << ", "
- << std::dec << std::nouppercase;
- ss << "format: " << pmbus_utils::toString(format);
-
- if (exponent.has_value())
- {
- ss << ", exponent: " << static_cast<int16_t>(exponent.value());
- }
-
- ss << " }";
-
- return ss.str();
- }
+ virtual std::string toString() const override;
private:
/**
+ * Gets the exponent value to use to convert a linear_16 format value to a
+ * decimal volts value.
+ *
+ * If an exponent value is defined for this action, that value is returned.
+ * Otherwise VOUT_MODE is read from the current device to obtain the
+ * exponent value.
+ *
+ * Throws an exception if an error occurs.
+ *
+ * @param interface I2C interface to the current device
+ * @return exponent value
+ */
+ int8_t getExponentValue(i2c::I2CInterface& interface);
+
+ /**
* Sensor value type.
*/
const pmbus_utils::SensorValueType type{};
diff --git a/phosphor-regulators/src/meson.build b/phosphor-regulators/src/meson.build
index 1a1602f..4adeb58 100644
--- a/phosphor-regulators/src/meson.build
+++ b/phosphor-regulators/src/meson.build
@@ -22,6 +22,7 @@
'actions/i2c_write_bit_action.cpp',
'actions/i2c_write_byte_action.cpp',
'actions/i2c_write_bytes_action.cpp',
+ 'actions/pmbus_read_sensor_action.cpp',
'actions/pmbus_write_vout_command_action.cpp'
]
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 4af095c..6edd1b8 100644
--- a/phosphor-regulators/test/actions/pmbus_read_sensor_action_tests.cpp
+++ b/phosphor-regulators/test/actions/pmbus_read_sensor_action_tests.cpp
@@ -14,20 +14,34 @@
* limitations under the License.
*/
#include "action_environment.hpp"
+#include "action_error.hpp"
+#include "device.hpp"
#include "i2c_action.hpp"
#include "i2c_interface.hpp"
+#include "id_map.hpp"
+#include "mocked_i2c_interface.hpp"
+#include "pmbus_error.hpp"
#include "pmbus_read_sensor_action.hpp"
#include "pmbus_utils.hpp"
#include <cstdint>
+#include <memory>
#include <optional>
#include <stdexcept>
#include <string>
+#include <utility>
+#include <gmock/gmock.h>
#include <gtest/gtest.h>
using namespace phosphor::power::regulators;
+using ::testing::A;
+using ::testing::Return;
+using ::testing::SetArgReferee;
+using ::testing::Throw;
+using ::testing::TypedEq;
+
TEST(PMBusReadSensorActionTests, Constructor)
{
// Test where works: exponent value is specified
@@ -72,7 +86,327 @@
TEST(PMBusReadSensorActionTests, Execute)
{
- // TODO: Not implemented yet
+ // Test where works: linear_11 defined in action
+ try
+ {
+ // Create mock I2CInterface.
+ // * will read 0xD2E0 from READ_IOUT (command/register 0x8C)
+ // * will not read from VOUT_MODE (command/register 0x20)
+ // assume output current is 11.5 amps,
+ // exponent = -6 = 11010, mantissa = 736 = 010 1110 0000
+ // linear data format = 1101 0010 1110 0000 = 0xD2E0
+ std::unique_ptr<i2c::MockedI2CInterface> i2cInterface =
+ std::make_unique<i2c::MockedI2CInterface>();
+ EXPECT_CALL(*i2cInterface, isOpen).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(*i2cInterface, read(TypedEq<uint8_t>(0x8C), A<uint16_t&>()))
+ .Times(1)
+ .WillOnce(SetArgReferee<1>(0xD2E0));
+ EXPECT_CALL(*i2cInterface, read(A<uint8_t>(), A<uint8_t&>())).Times(0);
+
+ // Create Device, IDMap, and ActionEnvironment
+ Device device{"reg1", true, "/system/chassis/motherboard/reg1",
+ std::move(i2cInterface)};
+ IDMap idMap{};
+ idMap.addDevice(device);
+ ActionEnvironment env{idMap, "reg1"};
+
+ // Create and execute action
+ pmbus_utils::SensorValueType type{pmbus_utils::SensorValueType::iout};
+ uint8_t command = 0x8C;
+ pmbus_utils::SensorDataFormat format{
+ pmbus_utils::SensorDataFormat::linear_11};
+ 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);
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
+
+ // Test where works: linear_16 with exponent defined in action
+ try
+ {
+ // Create mock I2CInterface.
+ // * will read 0x0002 from READ_VOUT (command/register 0x8B)
+ // * will not read from VOUT_MODE (command/register 0x20)
+ // assume output voltage is 16 volts,
+ // exponent = 3
+ // linear data format = 0000 0000 0000 0010 = 0x0002 = 2
+ std::unique_ptr<i2c::MockedI2CInterface> i2cInterface =
+ std::make_unique<i2c::MockedI2CInterface>();
+ EXPECT_CALL(*i2cInterface, isOpen).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(*i2cInterface, read(TypedEq<uint8_t>(0x8B), A<uint16_t&>()))
+ .Times(1)
+ .WillOnce(SetArgReferee<1>(0x0002));
+ EXPECT_CALL(*i2cInterface, read(A<uint8_t>(), A<uint8_t&>())).Times(0);
+
+ // Create Device, IDMap, and ActionEnvironment
+ Device device{"reg1", true, "/system/chassis/motherboard/reg1",
+ std::move(i2cInterface)};
+ IDMap idMap{};
+ idMap.addDevice(device);
+ ActionEnvironment env{idMap, "reg1"};
+
+ // Create and execute action
+ pmbus_utils::SensorValueType type{pmbus_utils::SensorValueType::vout};
+ uint8_t command = 0x8B;
+ pmbus_utils::SensorDataFormat format{
+ pmbus_utils::SensorDataFormat::linear_16};
+ 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);
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
+
+ // Test where works: linear_16 with no exponent defined in action
+ try
+ {
+ // Create mock I2CInterface.
+ // * will read 0xB877 from vout_peak (command/register 0xC6)
+ // * will read 0b0001'0111 (linear format, -9 exponent) from VOUT_MODE
+ // assume output voltage is 0.232421875 volts,
+ // linear data format = 0000 0000 0111 0111 = 0x0077
+ std::unique_ptr<i2c::MockedI2CInterface> i2cInterface =
+ std::make_unique<i2c::MockedI2CInterface>();
+ EXPECT_CALL(*i2cInterface, isOpen).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(*i2cInterface, read(TypedEq<uint8_t>(0xC6), A<uint16_t&>()))
+ .Times(1)
+ .WillOnce(SetArgReferee<1>(0x0077));
+ EXPECT_CALL(*i2cInterface, read(TypedEq<uint8_t>(0x20), A<uint8_t&>()))
+ .Times(1)
+ .WillOnce(SetArgReferee<1>(0b0001'0111));
+ // Create Device, IDMap, and ActionEnvironment
+ Device device{"reg1", true, "/system/chassis/motherboard/reg1",
+ std::move(i2cInterface)};
+ IDMap idMap{};
+ idMap.addDevice(device);
+ ActionEnvironment env{idMap, "reg1"};
+
+ // Create and execute action
+ pmbus_utils::SensorValueType type{
+ pmbus_utils::SensorValueType::vout_peak};
+ uint8_t command = 0xC6;
+ pmbus_utils::SensorDataFormat format{
+ pmbus_utils::SensorDataFormat::linear_16};
+ 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);
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
+
+ // Test where fails: Unable to get I2C interface to current device
+ try
+ {
+ // Create IDMap and ActionEnvironment
+ IDMap idMap{};
+ ActionEnvironment env{idMap, "reg1"};
+
+ // Create and execute action
+ pmbus_utils::SensorValueType type{pmbus_utils::SensorValueType::pout};
+ uint8_t command = 0x96;
+ pmbus_utils::SensorDataFormat format{
+ pmbus_utils::SensorDataFormat::linear_11};
+ std::optional<int8_t> exponent{};
+ PMBusReadSensorAction action{type, command, format, exponent};
+ action.execute(env);
+ ADD_FAILURE() << "Should not have reached this line.";
+ }
+ catch (const std::invalid_argument& e)
+ {
+ EXPECT_STREQ(e.what(), "Unable to find device with ID \"reg1\"");
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
+
+ // Test where fails: VOUT_MODE data format is not linear
+ try
+ {
+ // Create mock I2CInterface. Expect action to do the following:
+ // * will read 0b0010'0000 (vid data format) from VOUT_MODE
+ std::unique_ptr<i2c::MockedI2CInterface> i2cInterface =
+ std::make_unique<i2c::MockedI2CInterface>();
+ EXPECT_CALL(*i2cInterface, isOpen).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(*i2cInterface, read(TypedEq<uint8_t>(0x20), A<uint8_t&>()))
+ .Times(1)
+ .WillOnce(SetArgReferee<1>(0b0010'0000));
+
+ // Create Device, IDMap, and ActionEnvironment
+ Device device{"reg1", true, "/system/chassis/motherboard/reg1",
+ std::move(i2cInterface)};
+ IDMap idMap{};
+ idMap.addDevice(device);
+ ActionEnvironment env{idMap, "reg1"};
+
+ // Create and execute action
+ pmbus_utils::SensorValueType type{pmbus_utils::SensorValueType::vout};
+ uint8_t command = 0x8B;
+ pmbus_utils::SensorDataFormat format{
+ pmbus_utils::SensorDataFormat::linear_16};
+ std::optional<int8_t> exponent{};
+ PMBusReadSensorAction action{type, command, format, exponent};
+ action.execute(env);
+ ADD_FAILURE() << "Should not have reached this line.";
+ }
+ catch (const ActionError& e)
+ {
+ EXPECT_STREQ(e.what(), "ActionError: pmbus_read_sensor: { type: vout, "
+ "command: 0x8B, format: linear_16 }");
+ try
+ {
+ // Re-throw inner PMBusError
+ std::rethrow_if_nested(e);
+ ADD_FAILURE() << "Should not have reached this line.";
+ }
+ catch (const PMBusError& pe)
+ {
+ EXPECT_STREQ(
+ pe.what(),
+ "PMBusError: VOUT_MODE contains unsupported data format");
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
+
+ // Test where fails: Reading VOUT_MODE fails
+ try
+ {
+ // Create mock I2CInterface. Expect action to do the following:
+ // * will try to read VOUT_MODE; exception will be thrown
+ std::unique_ptr<i2c::MockedI2CInterface> i2cInterface =
+ std::make_unique<i2c::MockedI2CInterface>();
+ EXPECT_CALL(*i2cInterface, isOpen).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(*i2cInterface, read(TypedEq<uint8_t>(0x20), A<uint8_t&>()))
+ .Times(1)
+ .WillOnce(Throw(
+ i2c::I2CException{"Failed to read byte", "/dev/i2c-1", 0x70}));
+
+ // Create Device, IDMap, and ActionEnvironment
+ Device device{"reg1", true, "/system/chassis/motherboard/reg1",
+ std::move(i2cInterface)};
+ IDMap idMap{};
+ idMap.addDevice(device);
+ ActionEnvironment env{idMap, "reg1"};
+
+ // Create and execute action
+ pmbus_utils::SensorValueType type{
+ pmbus_utils::SensorValueType::vout_peak};
+ uint8_t command = 0xC6;
+ pmbus_utils::SensorDataFormat format{
+ pmbus_utils::SensorDataFormat::linear_16};
+ std::optional<int8_t> exponent{};
+ PMBusReadSensorAction action{type, command, format, exponent};
+ action.execute(env);
+ ADD_FAILURE() << "Should not have reached this line.";
+ }
+ catch (const ActionError& e)
+ {
+ EXPECT_STREQ(e.what(),
+ "ActionError: pmbus_read_sensor: { type: vout_peak, "
+ "command: 0xC6, format: linear_16 }");
+ try
+ {
+ // Re-throw inner I2CException
+ std::rethrow_if_nested(e);
+ ADD_FAILURE() << "Should not have reached this line.";
+ }
+ catch (const i2c::I2CException& ie)
+ {
+ EXPECT_STREQ(
+ ie.what(),
+ "I2CException: Failed to read byte: bus /dev/i2c-1, addr 0x70");
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
+
+ // Test where fails: Reading PMBus command code with sensor value fails
+ try
+ {
+ // Create mock I2CInterface. Expect action to do the following:
+ // * will try to read PMBus command(0x96); exception will be thrown
+ std::unique_ptr<i2c::MockedI2CInterface> i2cInterface =
+ std::make_unique<i2c::MockedI2CInterface>();
+ EXPECT_CALL(*i2cInterface, isOpen).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(*i2cInterface, read(TypedEq<uint8_t>(0x96), A<uint16_t&>()))
+ .Times(1)
+ .WillOnce(Throw(
+ i2c::I2CException{"Failed to read word", "/dev/i2c-1", 0x70}));
+
+ // Create Device, IDMap, and ActionEnvironment
+ Device device{"reg1", true, "/system/chassis/motherboard/reg1",
+ std::move(i2cInterface)};
+ IDMap idMap{};
+ idMap.addDevice(device);
+ ActionEnvironment env{idMap, "reg1"};
+
+ // Create and execute action
+ pmbus_utils::SensorValueType type{pmbus_utils::SensorValueType::pout};
+ uint8_t command = 0x96;
+ pmbus_utils::SensorDataFormat format{
+ pmbus_utils::SensorDataFormat::linear_11};
+ std::optional<int8_t> exponent{};
+ PMBusReadSensorAction action{type, command, format, exponent};
+ action.execute(env);
+ ADD_FAILURE() << "Should not have reached this line.";
+ }
+ catch (const ActionError& e)
+ {
+ EXPECT_STREQ(e.what(), "ActionError: pmbus_read_sensor: { type: pout, "
+ "command: 0x96, format: linear_11 }");
+ try
+ {
+ // Re-throw inner I2CException
+ std::rethrow_if_nested(e);
+ ADD_FAILURE() << "Should not have reached this line.";
+ }
+ catch (const i2c::I2CException& ie)
+ {
+ EXPECT_STREQ(
+ ie.what(),
+ "I2CException: Failed to read word: bus /dev/i2c-1, addr 0x70");
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
}
TEST(PMBusReadSensorActionTests, GetCommand)
@@ -135,27 +469,26 @@
{
// Test where exponent value is specified
{
- pmbus_utils::SensorValueType type{
- pmbus_utils::SensorValueType::temperature_peak};
- uint8_t command = 0x8C;
+ pmbus_utils::SensorValueType type{pmbus_utils::SensorValueType::vout};
+ uint8_t command = 0x8B;
pmbus_utils::SensorDataFormat format{
pmbus_utils::SensorDataFormat::linear_16};
std::optional<int8_t> exponent{-8};
PMBusReadSensorAction action{type, command, format, exponent};
EXPECT_EQ(action.toString(), "pmbus_read_sensor: { type: "
- "temperature_peak, command: 0x8C, format: "
+ "vout, command: 0x8B, format: "
"linear_16, exponent: -8 }");
}
// Test where exponent value is not specified
{
- pmbus_utils::SensorValueType type{pmbus_utils::SensorValueType::vout};
+ pmbus_utils::SensorValueType type{pmbus_utils::SensorValueType::iout};
uint8_t command = 0x8C;
pmbus_utils::SensorDataFormat format{
pmbus_utils::SensorDataFormat::linear_11};
std::optional<int8_t> exponent{};
PMBusReadSensorAction action{type, command, format, exponent};
- EXPECT_EQ(action.toString(), "pmbus_read_sensor: { type: vout, "
+ EXPECT_EQ(action.toString(), "pmbus_read_sensor: { type: iout, "
"command: 0x8C, format: linear_11 }");
}
}