regulators: Allow empty expected VPD value
Modify the compare_vpd action to support specifying an empty string or
empty byte vector as the expected keyword value.
The VPD service was modified in a previous commit to return an empty
keyword value if the VPD interface or keyword does not exist on the
specified D-Bus object path. Thus, the actual keyword value may be
empty.
Signed-off-by: Shawn McCarney <shawnmm@us.ibm.com>
Change-Id: If48ab6423c40e5b24d6cff72264dc2efe750c85f
diff --git a/phosphor-regulators/docs/config_file/compare_vpd.md b/phosphor-regulators/docs/config_file/compare_vpd.md
index 79927c1..304bfcf 100644
--- a/phosphor-regulators/docs/config_file/compare_vpd.md
+++ b/phosphor-regulators/docs/config_file/compare_vpd.md
@@ -7,6 +7,10 @@
from an EEPROM on a Field-Replaceable Unit (FRU). For this reason, VPD is also
called "FRU data".
+The phosphor-regulators application obtains VPD keyword values from D-Bus.
+Other BMC applications and drivers are responsible for reading VPD from
+hardware components and publishing it on D-Bus.
+
The following VPD keywords are currently supported:
* CCIN
* Manufacturer
@@ -18,13 +22,28 @@
a VPD keyword value. For example, you could set the output voltage only for
regulators with a specific Model number.
+### Unavailable keyword values
+A keyword value may be unavailable if:
+* The hardware component does not support the keyword.
+* An error occurred while attempting to read VPD from the hardware component.
+* The BMC cannot access the VPD due to the system hardware design, such as not
+ being connected to the necessary I2C bus.
+
+If the keyword value is unavailable, it will be treated as having an "empty"
+value:
+* An empty string ("") if the "value" property was specified.
+* An empty array ([]) if the "byte_values" property was specified.
+
+If the expected value is not "empty", the compare_vpd action will return false
+since the values will not match.
+
## Properties
| Name | Required | Type | Description |
| :--- | :------: | :--- | :---------- |
| fru | yes | string | Field-Replaceable Unit (FRU) that contains the VPD. Specify the relative D-Bus inventory path of the FRU. Full inventory paths begin with the root "/xyz/openbmc_project/inventory". Specify the relative path below the root, such as "system/chassis/disk_backplane". |
| keyword | yes | string | VPD keyword. Specify one of the following: "CCIN", "Manufacturer", "Model", "PartNumber", "HW". |
| value | see [notes](#notes) | string | Expected value. |
-| byte\_values | see [notes](#notes) | array of strings | One or more expected byte values expressed in hexadecimal. Each value must be prefixed with 0x and surrounded by double quotes. |
+| byte\_values | see [notes](#notes) | array of strings | Zero or more expected byte values expressed in hexadecimal. Each value must be prefixed with 0x and surrounded by double quotes. |
### Notes
* You must specify either "value" or "byte_values".
diff --git a/phosphor-regulators/schema/config_schema.json b/phosphor-regulators/schema/config_schema.json
index 2e0a8b7..dbfed58 100644
--- a/phosphor-regulators/schema/config_schema.json
+++ b/phosphor-regulators/schema/config_schema.json
@@ -133,7 +133,7 @@
"fru": {"$ref": "#/definitions/inventory_path" },
"keyword": {"$ref": "#/definitions/keyword" },
"value": {"$ref": "#/definitions/string_value" },
- "byte_values": {"$ref": "#/definitions/bytes_values" }
+ "byte_values": {"$ref": "#/definitions/byte_values_min0" }
},
"required": ["fru", "keyword"],
"oneOf": [
@@ -236,21 +236,28 @@
"properties":
{
"register": {"$ref": "#/definitions/register" },
- "values": {"$ref": "#/definitions/bytes_values" },
- "masks": {"$ref": "#/definitions/bytes_masks" }
+ "values": {"$ref": "#/definitions/byte_values" },
+ "masks": {"$ref": "#/definitions/byte_masks" }
},
"required": ["register", "values"],
"additionalProperties": false
},
- "bytes_values":
+ "byte_values":
{
"type": "array",
"items": {"$ref": "#/definitions/byte_value" },
"minItems": 1
},
- "bytes_masks":
+ "byte_values_min0":
+ {
+ "type": "array",
+ "items": {"$ref": "#/definitions/byte_value" },
+ "minItems": 0
+ },
+
+ "byte_masks":
{
"type": "array",
"items": {"$ref": "#/definitions/byte_mask" },
diff --git a/phosphor-regulators/src/config_file_parser.cpp b/phosphor-regulators/src/config_file_parser.cpp
index db0cd7a..3eecbb6 100644
--- a/phosphor-regulators/src/config_file_parser.cpp
+++ b/phosphor-regulators/src/config_file_parser.cpp
@@ -287,7 +287,7 @@
auto byteValuesIt = element.find("byte_values");
if ((valueIt != element.end()) && (byteValuesIt == element.end()))
{
- std::string stringValue = parseString(*valueIt);
+ std::string stringValue = parseString(*valueIt, true);
value.insert(value.begin(), stringValue.begin(), stringValue.end());
++propertyCount;
}
diff --git a/phosphor-regulators/test/actions/compare_vpd_action_tests.cpp b/phosphor-regulators/test/actions/compare_vpd_action_tests.cpp
index 26361b9..c90c09e 100644
--- a/phosphor-regulators/test/actions/compare_vpd_action_tests.cpp
+++ b/phosphor-regulators/test/actions/compare_vpd_action_tests.cpp
@@ -36,19 +36,36 @@
TEST(CompareVPDActionTests, Constructor)
{
- std::vector<uint8_t> value{0x32, 0x44, 0x33, 0x35}; // "2D35"
- CompareVPDAction action{
- "/xyz/openbmc_project/inventory/system/chassis/disk_backplane", "CCIN",
- value};
- EXPECT_EQ(action.getFRU(),
- "/xyz/openbmc_project/inventory/system/chassis/disk_backplane");
- EXPECT_EQ(action.getKeyword(), "CCIN");
- EXPECT_EQ(action.getValue(), value);
+ // Value vector is not empty
+ {
+ std::vector<uint8_t> value{0x32, 0x44, 0x33, 0x35}; // "2D35"
+ CompareVPDAction action{
+ "/xyz/openbmc_project/inventory/system/chassis/disk_backplane",
+ "CCIN", value};
+ EXPECT_EQ(
+ action.getFRU(),
+ "/xyz/openbmc_project/inventory/system/chassis/disk_backplane");
+ EXPECT_EQ(action.getKeyword(), "CCIN");
+ EXPECT_EQ(action.getValue(), value);
+ }
+
+ // Value vector is empty
+ {
+ std::vector<uint8_t> value{};
+ CompareVPDAction action{
+ "/xyz/openbmc_project/inventory/system/chassis/disk_backplane",
+ "CCIN", value};
+ EXPECT_EQ(
+ action.getFRU(),
+ "/xyz/openbmc_project/inventory/system/chassis/disk_backplane");
+ EXPECT_EQ(action.getKeyword(), "CCIN");
+ EXPECT_EQ(action.getValue(), value);
+ }
}
TEST(CompareVPDActionTests, Execute)
{
- // Test where works: Actual VPD value is not an empty string
+ // Test where works: Actual VPD value is not empty
{
std::string fru{"/xyz/openbmc_project/inventory/system"};
std::string keyword{"Model"};
@@ -85,13 +102,13 @@
}
}
- // Test where works: Actual VPD value is an empty string
+ // Test where works: Actual VPD value is empty
{
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
+ // Create MockServices object. VPD service will return empty VPD value
// 2 times.
MockServices services{};
MockVPD& vpd = services.getMockVPD();
@@ -194,6 +211,7 @@
TEST(CompareVPDActionTests, ToString)
{
+ // Test where value vector is not empty
{
CompareVPDAction action{
"/xyz/openbmc_project/inventory/system/chassis/disk_backplane",
@@ -204,6 +222,7 @@
"CCIN, value: [ 0x1, 0xA3, 0x0, 0xFF ] }");
}
+ // Test where value vector is empty
{
CompareVPDAction action{
"/xyz/openbmc_project/inventory/system/chassis/disk_backplane",
diff --git a/phosphor-regulators/test/config_file_parser_tests.cpp b/phosphor-regulators/test/config_file_parser_tests.cpp
index 38bee22..f477d41 100644
--- a/phosphor-regulators/test/config_file_parser_tests.cpp
+++ b/phosphor-regulators/test/config_file_parser_tests.cpp
@@ -1138,7 +1138,7 @@
TEST(ConfigFileParserTests, ParseCompareVPD)
{
- // Test where works, using "value"
+ // Test where works: value property: Not empty
{
const json element = R"(
{
@@ -1156,7 +1156,24 @@
(std::vector<uint8_t>{0x32, 0x44, 0x33, 0x35}));
}
- // Test where works, using "byte_values"
+ // Test where works: value property: Empty
+ {
+ const json element = R"(
+ {
+ "fru": "system/chassis/disk_backplane",
+ "keyword": "CCIN",
+ "value": ""
+ }
+ )"_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>{}));
+ }
+
+ // Test where works: byte_values property: Not empty
{
const json element = R"(
{
@@ -1173,6 +1190,23 @@
EXPECT_EQ(action->getValue(), (std::vector<uint8_t>{0x11, 0x22, 0x33}));
}
+ // Test where works: byte_values property: Empty
+ {
+ const json element = R"(
+ {
+ "fru": "system/chassis/disk_backplane",
+ "keyword": "CCIN",
+ "byte_values": []
+ }
+ )"_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>{}));
+ }
+
// Test where fails: Element is not an object
try
{
@@ -1256,7 +1290,7 @@
"either value or byte_values");
}
- // Test where fails: both value and byte_value specified
+ // Test where fails: both value and byte_values specified
try
{
const json element = R"(
diff --git a/phosphor-regulators/test/validate-regulators-config_tests.cpp b/phosphor-regulators/test/validate-regulators-config_tests.cpp
index f6033d5..2763cc9 100644
--- a/phosphor-regulators/test/validate-regulators-config_tests.cpp
+++ b/phosphor-regulators/test/validate-regulators-config_tests.cpp
@@ -702,12 +702,20 @@
compareVpdFile["rules"][0]["actions"][1]["compare_vpd"]["keyword"] = "CCIN";
compareVpdFile["rules"][0]["actions"][1]["compare_vpd"]["value"] = "2D35";
- // Valid.
+ // Valid: value property: not empty.
{
json configFile = compareVpdFile;
EXPECT_JSON_VALID(configFile);
}
- // Valid, using byte_values.
+
+ // Valid: value property: empty.
+ {
+ json configFile = compareVpdFile;
+ configFile["rules"][0]["actions"][1]["compare_vpd"]["value"] = "";
+ EXPECT_JSON_VALID(configFile);
+ }
+
+ // Valid: byte_values property: not empty.
{
json configFile = compareVpdFile;
configFile["rules"][0]["actions"][1]["compare_vpd"].erase("value");
@@ -716,6 +724,15 @@
EXPECT_JSON_VALID(configFile);
}
+ // Valid: byte_values property: 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_VALID(configFile);
+ }
+
// Invalid: no FRU property.
{
json configFile = compareVpdFile;
@@ -785,16 +802,6 @@
"'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;