regulators: Support a string or vector for VPD
Add a 'byte_values' alternative to the 'value' entry in the compare VPD
action. This is to support VPD values that are not strings, such as
'HW', a new IBM keyword that describes the version of a piece of
hardware.
To support this, the VPD class now treats all VPD keyword values as
vectors of uint8_ts, including in its data cache. If a compare VPD
action in the JSON contains a string value, it will be converted to the
vector before the CompareVPDAction class is created.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: I3fcabf896f4885feae1b07ee2c3da5929cf8bfa4
diff --git a/phosphor-regulators/test/actions/compare_vpd_action_tests.cpp b/phosphor-regulators/test/actions/compare_vpd_action_tests.cpp
index 1cc7782..26361b9 100644
--- a/phosphor-regulators/test/actions/compare_vpd_action_tests.cpp
+++ b/phosphor-regulators/test/actions/compare_vpd_action_tests.cpp
@@ -36,13 +36,14 @@
TEST(CompareVPDActionTests, Constructor)
{
+ std::vector<uint8_t> value{0x32, 0x44, 0x33, 0x35}; // "2D35"
CompareVPDAction action{
"/xyz/openbmc_project/inventory/system/chassis/disk_backplane", "CCIN",
- "2D35"};
+ value};
EXPECT_EQ(action.getFRU(),
"/xyz/openbmc_project/inventory/system/chassis/disk_backplane");
EXPECT_EQ(action.getKeyword(), "CCIN");
- EXPECT_EQ(action.getValue(), "2D35");
+ EXPECT_EQ(action.getValue(), value);
}
TEST(CompareVPDActionTests, Execute)
@@ -51,39 +52,35 @@
{
std::string fru{"/xyz/openbmc_project/inventory/system"};
std::string keyword{"Model"};
+ std::vector<uint8_t> abcdValue{0x41, 0x42, 0x43, 0x44};
// Create MockServices object. VPD service will return "ABCD" as VPD
- // value 4 times.
+ // value 3 times.
MockServices services{};
MockVPD& vpd = services.getMockVPD();
EXPECT_CALL(vpd, getValue(fru, keyword))
- .Times(4)
- .WillRepeatedly(Return("ABCD"));
+ .Times(3)
+ .WillRepeatedly(Return(abcdValue));
IDMap idMap{};
ActionEnvironment environment{idMap, "", services};
// Test where returns true: actual value == expected value
{
- CompareVPDAction action{fru, keyword, "ABCD"};
+ CompareVPDAction action{fru, keyword, abcdValue};
EXPECT_TRUE(action.execute(environment));
}
// Test where returns false: actual value != expected value
{
- CompareVPDAction action{fru, keyword, "BEEF"};
+ CompareVPDAction action{fru, keyword,
+ std::vector<uint8_t>{1, 2, 3, 4}};
EXPECT_FALSE(action.execute(environment));
}
- // Test where returns false: expected value differs by case
+ // Test where returns false: expected value is empty
{
- CompareVPDAction action{fru, keyword, "abcd"};
- EXPECT_FALSE(action.execute(environment));
- }
-
- // Test where returns false: expected value is an empty string
- {
- CompareVPDAction action{fru, keyword, ""};
+ CompareVPDAction action{fru, keyword, std::vector<uint8_t>{}};
EXPECT_FALSE(action.execute(environment));
}
}
@@ -92,6 +89,7 @@
{
std::string fru{"/xyz/openbmc_project/inventory/system"};
std::string keyword{"Model"};
+ std::vector<uint8_t> emptyValue{};
// Create MockServices object. VPD service will return "" as VPD value
// 2 times.
@@ -99,20 +97,21 @@
MockVPD& vpd = services.getMockVPD();
EXPECT_CALL(vpd, getValue(fru, keyword))
.Times(2)
- .WillRepeatedly(Return(""));
+ .WillRepeatedly(Return(emptyValue));
IDMap idMap{};
ActionEnvironment environment{idMap, "", services};
// Test where returns true: actual value == expected value
{
- CompareVPDAction action{fru, keyword, ""};
+ CompareVPDAction action{fru, keyword, emptyValue};
EXPECT_TRUE(action.execute(environment));
}
// Test where returns false: actual value != expected value
{
- CompareVPDAction action{fru, keyword, "ABCD"};
+ CompareVPDAction action{fru, keyword,
+ std::vector<uint8_t>{1, 2, 3}};
EXPECT_FALSE(action.execute(environment));
}
}
@@ -135,15 +134,17 @@
try
{
- CompareVPDAction action{fru, keyword, "ABCD"};
+ CompareVPDAction action{fru, keyword,
+ std::vector<uint8_t>{1, 2, 3}};
action.execute(environment);
ADD_FAILURE() << "Should not have reached this line.";
}
catch (const ActionError& e)
{
- EXPECT_STREQ(e.what(), "ActionError: compare_vpd: { fru: "
- "/xyz/openbmc_project/inventory/system, "
- "keyword: Model, value: ABCD }");
+ EXPECT_STREQ(e.what(),
+ "ActionError: compare_vpd: { fru: "
+ "/xyz/openbmc_project/inventory/system, "
+ "keyword: Model, value: [ 0x1, 0x2, 0x3 ] }");
try
{
// Re-throw inner exception
@@ -170,7 +171,7 @@
{
CompareVPDAction action{
"/xyz/openbmc_project/inventory/system/chassis/disk_backplane", "CCIN",
- "2D35"};
+ std::vector<uint8_t>{1, 2, 3, 4}};
EXPECT_EQ(action.getFRU(),
"/xyz/openbmc_project/inventory/system/chassis/disk_backplane");
}
@@ -179,7 +180,7 @@
{
CompareVPDAction action{
"/xyz/openbmc_project/inventory/system/chassis/disk_backplane", "CCIN",
- "2D35"};
+ std::vector<uint8_t>{1, 2, 3, 4}};
EXPECT_EQ(action.getKeyword(), "CCIN");
}
@@ -187,17 +188,29 @@
{
CompareVPDAction action{
"/xyz/openbmc_project/inventory/system/chassis/disk_backplane", "CCIN",
- "2D35"};
- EXPECT_EQ(action.getValue(), "2D35");
+ std::vector<uint8_t>{1, 2, 3, 4}};
+ EXPECT_EQ(action.getValue(), (std::vector<uint8_t>{0x1, 0x2, 0x3, 0x4}));
}
TEST(CompareVPDActionTests, ToString)
{
- CompareVPDAction action{
- "/xyz/openbmc_project/inventory/system/chassis/disk_backplane", "CCIN",
- "2D35"};
- EXPECT_EQ(action.toString(), "compare_vpd: { fru: "
- "/xyz/openbmc_project/inventory/system/"
- "chassis/disk_backplane, keyword: "
- "CCIN, value: 2D35 }");
+ {
+ CompareVPDAction action{
+ "/xyz/openbmc_project/inventory/system/chassis/disk_backplane",
+ "CCIN", std::vector<uint8_t>{0x01, 0xA3, 0x0, 0xFF}};
+ EXPECT_EQ(action.toString(), "compare_vpd: { fru: "
+ "/xyz/openbmc_project/inventory/system/"
+ "chassis/disk_backplane, keyword: "
+ "CCIN, value: [ 0x1, 0xA3, 0x0, 0xFF ] }");
+ }
+
+ {
+ CompareVPDAction action{
+ "/xyz/openbmc_project/inventory/system/chassis/disk_backplane",
+ "CCIN", std::vector<uint8_t>{}};
+ EXPECT_EQ(action.toString(), "compare_vpd: { fru: "
+ "/xyz/openbmc_project/inventory/system/"
+ "chassis/disk_backplane, keyword: "
+ "CCIN, value: [ ] }");
+ }
}
diff --git a/phosphor-regulators/test/config_file_parser_tests.cpp b/phosphor-regulators/test/config_file_parser_tests.cpp
index c2cf08e..3249465 100644
--- a/phosphor-regulators/test/config_file_parser_tests.cpp
+++ b/phosphor-regulators/test/config_file_parser_tests.cpp
@@ -1085,7 +1085,7 @@
TEST(ConfigFileParserTests, ParseCompareVPD)
{
- // Test where works
+ // Test where works, using "value"
{
const json element = R"(
{
@@ -1099,7 +1099,25 @@
action->getFRU(),
"/xyz/openbmc_project/inventory/system/chassis/disk_backplane");
EXPECT_EQ(action->getKeyword(), "CCIN");
- EXPECT_EQ(action->getValue(), "2D35");
+ EXPECT_EQ(action->getValue(),
+ (std::vector<uint8_t>{0x32, 0x44, 0x33, 0x35}));
+ }
+
+ // Test where works, using "byte_values"
+ {
+ const json element = R"(
+ {
+ "fru": "system/chassis/disk_backplane",
+ "keyword": "CCIN",
+ "byte_values": ["0x11", "0x22", "0x33"]
+ }
+ )"_json;
+ std::unique_ptr<CompareVPDAction> action = parseCompareVPD(element);
+ EXPECT_EQ(
+ action->getFRU(),
+ "/xyz/openbmc_project/inventory/system/chassis/disk_backplane");
+ EXPECT_EQ(action->getKeyword(), "CCIN");
+ EXPECT_EQ(action->getValue(), (std::vector<uint8_t>{0x11, 0x22, 0x33}));
}
// Test where fails: Element is not an object
@@ -1181,7 +1199,28 @@
}
catch (const std::invalid_argument& e)
{
- EXPECT_STREQ(e.what(), "Required property missing: value");
+ EXPECT_STREQ(e.what(), "Invalid property: Must contain "
+ "either value or byte_values");
+ }
+
+ // Test where fails: both value and byte_value specified
+ try
+ {
+ const json element = R"(
+ {
+ "fru": "system/chassis/disk_backplane",
+ "keyword": "CCIN",
+ "value": "2D35",
+ "byte_values": [ "0x01", "0x02" ]
+ }
+ )"_json;
+ parseCompareVPD(element);
+ ADD_FAILURE() << "Should not have reached this line.";
+ }
+ catch (const std::invalid_argument& e)
+ {
+ EXPECT_STREQ(e.what(), "Invalid property: Must contain "
+ "either value or byte_values");
}
// Test where fails: fru value is invalid
@@ -1237,6 +1276,24 @@
{
EXPECT_STREQ(e.what(), "Element is not a string");
}
+
+ // Test where fails: byte_values is wrong format
+ try
+ {
+ const json element = R"(
+ {
+ "fru": "system/chassis/disk_backplane",
+ "keyword": "CCIN",
+ "byte_values": [1, 2, 3]
+ }
+ )"_json;
+ parseCompareVPD(element);
+ ADD_FAILURE() << "Should not have reached this line.";
+ }
+ catch (const std::invalid_argument& e)
+ {
+ EXPECT_STREQ(e.what(), "Element is not a string");
+ }
}
TEST(ConfigFileParserTests, ParseConfiguration)
diff --git a/phosphor-regulators/test/mock_vpd.hpp b/phosphor-regulators/test/mock_vpd.hpp
index 9b0d494..7c0779f 100644
--- a/phosphor-regulators/test/mock_vpd.hpp
+++ b/phosphor-regulators/test/mock_vpd.hpp
@@ -40,7 +40,7 @@
MOCK_METHOD(void, clearCache, (), (override));
- MOCK_METHOD(std::string, getValue,
+ MOCK_METHOD(std::vector<uint8_t>, getValue,
(const std::string& inventoryPath, const std::string& keyword),
(override));
};
diff --git a/phosphor-regulators/test/validate-regulators-config_tests.cpp b/phosphor-regulators/test/validate-regulators-config_tests.cpp
index e6b84ec..92aff99 100644
--- a/phosphor-regulators/test/validate-regulators-config_tests.cpp
+++ b/phosphor-regulators/test/validate-regulators-config_tests.cpp
@@ -652,6 +652,14 @@
json configFile = compareVpdFile;
EXPECT_JSON_VALID(configFile);
}
+ // Valid, using byte_values.
+ {
+ json configFile = compareVpdFile;
+ configFile["rules"][0]["actions"][1]["compare_vpd"].erase("value");
+ configFile["rules"][0]["actions"][1]["compare_vpd"]["byte_values"] = {
+ "0x01", "0x02"};
+ EXPECT_JSON_VALID(configFile);
+ }
// Invalid: no FRU property.
{
@@ -694,14 +702,14 @@
}
// Invalid: property keyword is not "CCIN", "Manufacturer", "Model",
- // "PartNumber"
+ // "PartNumber", "HW"
{
json configFile = compareVpdFile;
configFile["rules"][0]["actions"][1]["compare_vpd"]["keyword"] =
"Number";
EXPECT_JSON_INVALID(configFile, "Validation failed.",
"'Number' is not one of ['CCIN', "
- "'Manufacturer', 'Model', 'PartNumber']");
+ "'Manufacturer', 'Model', 'PartNumber', 'HW']");
}
// Invalid: property value wrong type.
@@ -711,6 +719,39 @@
EXPECT_JSON_INVALID(configFile, "Validation failed.",
"1 is not of type 'string'");
}
+
+ // Invalid: property byte_values has wrong type
+ {
+ json configFile = compareVpdFile;
+ configFile["rules"][0]["actions"][1]["compare_vpd"].erase("value");
+ configFile["rules"][0]["actions"][1]["compare_vpd"]["byte_values"] =
+ "0x50";
+ EXPECT_JSON_INVALID(configFile, "Validation failed.",
+ "'0x50' is not of type 'array'");
+ }
+
+ // Invalid: property byte_values is empty
+ {
+ json configFile = compareVpdFile;
+ configFile["rules"][0]["actions"][1]["compare_vpd"].erase("value");
+ configFile["rules"][0]["actions"][1]["compare_vpd"]["byte_values"] =
+ json::array();
+ EXPECT_JSON_INVALID(configFile, "Validation failed.",
+ "[] is too short");
+ }
+
+ // Invalid: properties byte_values and value both exist
+ {
+ json configFile = compareVpdFile;
+ configFile["rules"][0]["actions"][1]["compare_vpd"]["byte_values"] = {
+ "0x01", "0x02"};
+ EXPECT_JSON_INVALID(
+ configFile, "Validation failed.",
+ "{'byte_values': ['0x01', '0x02'], 'fru': "
+ "'system/chassis/motherboard/regulator2', 'keyword': 'CCIN', "
+ "'value': '2D35'} is valid under each of {'required': "
+ "['byte_values']}, {'required': ['value']}");
+ }
}
TEST(ValidateRegulatorsConfigTest, ConfigFile)
{