regulators: Enhance additional error data capture
Enhance the additional error data capture support to allow the same
register to be captured multiple times.
The additional error data is stored as key/value pairs. The key format
is normally:
<deviceID>_register_<register>
For example, "vdd1_register_0x7A".
If the same register is captured multiple times, a counter is appended
to make the key unique. For example, "vdd1_register_0x7A_2".
One use case for capturing the same register multiple times is when a
voltage regulator produces multiple rails. It may be necessary to do
the following:
* Set the PMBus PAGE to 0
* Capture a register for the first rail
* Set the PMBus PAGE to 1
* Capture the same register again for the second rail
Signed-off-by: Shawn McCarney <shawnmm@us.ibm.com>
Change-Id: I55c3e071b4b60e103f1de5778ede58b032ea29da
diff --git a/phosphor-regulators/src/actions/i2c_capture_bytes_action.cpp b/phosphor-regulators/src/actions/i2c_capture_bytes_action.cpp
index 0bb24b1..338ef4a 100644
--- a/phosphor-regulators/src/actions/i2c_capture_bytes_action.cpp
+++ b/phosphor-regulators/src/actions/i2c_capture_bytes_action.cpp
@@ -37,24 +37,9 @@
uint8_t values[UINT8_MAX];
interface.read(reg, size, values, i2c::I2CInterface::Mode::I2C);
- // Build additional error data key: <deviceID>_register_<register>
- std::ostringstream kss;
- kss << environment.getDeviceID() << "_register_0x" << std::hex
- << std::uppercase << static_cast<uint16_t>(reg);
- std::string key = kss.str();
-
- // Build additional error data value: [ <byte 0>, <byte 1>, ... ]
- std::ostringstream vss;
- vss << "[ " << std::hex << std::uppercase;
- for (unsigned int i = 0; i < count; ++i)
- {
- vss << ((i > 0) ? ", " : "") << "0x"
- << static_cast<uint16_t>(values[i]);
- }
- vss << " ]";
- std::string value = vss.str();
-
- // Store additional error data in action environment
+ // Store error data in action environment as a string key/value pair
+ std::string key = getErrorDataKey(environment);
+ std::string value = getErrorDataValue(values);
environment.addAdditionalErrorData(key, value);
}
catch (const i2c::I2CException& e)
@@ -75,4 +60,49 @@
return ss.str();
}
+std::string
+ I2CCaptureBytesAction::getErrorDataKey(ActionEnvironment& environment) const
+{
+ // Additional error data key format: <deviceID>_register_<register>
+ std::ostringstream ss;
+ ss << environment.getDeviceID() << "_register_0x" << std::hex
+ << std::uppercase << static_cast<uint16_t>(reg);
+ std::string key = ss.str();
+
+ // Verify key does not already exist in the action environment. This occurs
+ // when the same device and register is captured multiple times.
+ if (environment.getAdditionalErrorData().contains(key))
+ {
+ // Add counter suffix to key and loop until unused key is found
+ int counter = 2;
+ std::string keyWithSuffix;
+ do
+ {
+ keyWithSuffix = key + '_' + std::to_string(counter);
+ if (!environment.getAdditionalErrorData().contains(keyWithSuffix))
+ {
+ key = keyWithSuffix;
+ break;
+ }
+ ++counter;
+ } while (true);
+ }
+
+ return key;
+}
+
+std::string
+ I2CCaptureBytesAction::getErrorDataValue(const uint8_t* values) const
+{
+ // Additional error data value format: [ <byte 0>, <byte 1>, ... ]
+ std::ostringstream ss;
+ ss << "[ " << std::hex << std::uppercase;
+ for (unsigned int i = 0; i < count; ++i)
+ {
+ ss << ((i > 0) ? ", " : "") << "0x" << static_cast<uint16_t>(values[i]);
+ }
+ ss << " ]";
+ return ss.str();
+}
+
} // namespace phosphor::power::regulators
diff --git a/phosphor-regulators/src/actions/i2c_capture_bytes_action.hpp b/phosphor-regulators/src/actions/i2c_capture_bytes_action.hpp
index 0e7beb6..03d6e9e 100644
--- a/phosphor-regulators/src/actions/i2c_capture_bytes_action.hpp
+++ b/phosphor-regulators/src/actions/i2c_capture_bytes_action.hpp
@@ -111,6 +111,25 @@
private:
/**
+ * Returns the key for storing additional error data as a key/value pair in
+ * the action environment.
+ *
+ * @param environment action execution environment
+ * @return error data key
+ */
+ std::string getErrorDataKey(ActionEnvironment& environment) const;
+
+ /**
+ * Returns the value for storing additional error data as a key/value pair
+ * in the action environment.
+ *
+ * @param values Array of byte values read from the device. The count data
+ * member specifies the number of bytes that were read.
+ * @return error data value
+ */
+ std::string getErrorDataValue(const uint8_t* values) const;
+
+ /**
* Device register address. Note: named 'reg' because 'register' is a
* reserved keyword.
*/
diff --git a/phosphor-regulators/test/actions/i2c_capture_bytes_action_tests.cpp b/phosphor-regulators/test/actions/i2c_capture_bytes_action_tests.cpp
index f806a09..82c77ce 100644
--- a/phosphor-regulators/test/actions/i2c_capture_bytes_action_tests.cpp
+++ b/phosphor-regulators/test/actions/i2c_capture_bytes_action_tests.cpp
@@ -139,6 +139,53 @@
ADD_FAILURE() << "Should not have caught exception.";
}
+ // Test where works: Same device + register captured multiple times
+ try
+ {
+ // Create mock I2CInterface: read() will be called 3 times and will
+ // return values 0xD7, 0x13, and 0xFB
+ std::unique_ptr<i2c::MockedI2CInterface> i2cInterface =
+ std::make_unique<i2c::MockedI2CInterface>();
+ uint8_t values_1[] = {0xD7};
+ uint8_t values_2[] = {0x13};
+ uint8_t values_3[] = {0xFB};
+ EXPECT_CALL(*i2cInterface, isOpen)
+ .Times(3)
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*i2cInterface, read(0xCA, TypedEq<uint8_t&>(1), NotNull(),
+ i2c::I2CInterface::Mode::I2C))
+ .Times(3)
+ .WillOnce(SetArrayArgument<2>(values_1, values_1 + 1))
+ .WillOnce(SetArrayArgument<2>(values_2, values_2 + 1))
+ .WillOnce(SetArrayArgument<2>(values_3, values_3 + 1));
+
+ // Create Device, IDMap, MockServices, and ActionEnvironment
+ Device device{
+ "vdd1", true,
+ "/xyz/openbmc_project/inventory/system/chassis/motherboard/vdd1",
+ std::move(i2cInterface)};
+ IDMap idMap{};
+ idMap.addDevice(device);
+ MockServices services{};
+ ActionEnvironment env{idMap, "vdd1", services};
+
+ I2CCaptureBytesAction action{0xCA, 1};
+ EXPECT_EQ(action.execute(env), true);
+ EXPECT_EQ(action.execute(env), true);
+ EXPECT_EQ(action.execute(env), true);
+ EXPECT_EQ(env.getAdditionalErrorData().size(), 3);
+ EXPECT_EQ(env.getAdditionalErrorData().at("vdd1_register_0xCA"),
+ "[ 0xD7 ]");
+ EXPECT_EQ(env.getAdditionalErrorData().at("vdd1_register_0xCA_2"),
+ "[ 0x13 ]");
+ EXPECT_EQ(env.getAdditionalErrorData().at("vdd1_register_0xCA_3"),
+ "[ 0xFB ]");
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
+
// Test where fails: Getting I2CInterface fails
try
{