PEL: Get rid of the enum for a maint procedure
It simplifies the code to just use the string name of the procedure as defined
by the message registry schema, and make it be less maintenance to add
new procedures in the future, especially if they come from the registry
so the enum isn't even needed.
This will also make it the same as upcoming support for symbolic FRU
callouts, which will always just come from the registry or an external
user so would not need an enum.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: Ia8550235678ba1fc717a6b0875e2d7ff3888bfec
diff --git a/extensions/openpower-pels/callout.cpp b/extensions/openpower-pels/callout.cpp
index 905c212..66181ff 100644
--- a/extensions/openpower-pels/callout.cpp
+++ b/extensions/openpower-pels/callout.cpp
@@ -94,7 +94,8 @@
_size = flattenedSize();
}
-Callout::Callout(CalloutPriority priority, MaintProcedure procedure)
+Callout::Callout(CalloutPriority priority,
+ const std::string& procedureFromRegistry)
{
_flags = calloutType | fruIdentIncluded;
@@ -102,7 +103,7 @@
_locationCodeSize = 0;
- _fruIdentity = std::make_unique<FRUIdentity>(procedure);
+ _fruIdentity = std::make_unique<FRUIdentity>(procedureFromRegistry);
_size = flattenedSize();
}
diff --git a/extensions/openpower-pels/callout.hpp b/extensions/openpower-pels/callout.hpp
index c8bb0a3..6b7dbc7 100644
--- a/extensions/openpower-pels/callout.hpp
+++ b/extensions/openpower-pels/callout.hpp
@@ -87,9 +87,10 @@
* out maintenance procedure.
*
* @param[in] priority - The priority of the callout
- * @param[in] procedure - The maintenance procedure
+ * @param[in] procedureFromRegistry - The maintenance procedure name
+ * as defined in the message registry.
*/
- Callout(CalloutPriority priority, MaintProcedure procedure);
+ Callout(CalloutPriority priority, const std::string& procedureFromRegistry);
/**
* @brief Returns the size of this object when flattened into a PEL
diff --git a/extensions/openpower-pels/fru_identity.cpp b/extensions/openpower-pels/fru_identity.cpp
index 7e9bf1b..8cf6382 100644
--- a/extensions/openpower-pels/fru_identity.cpp
+++ b/extensions/openpower-pels/fru_identity.cpp
@@ -17,6 +17,10 @@
#include "pel_values.hpp"
+#include <phosphor-logging/log.hpp>
+
+using namespace phosphor::logging;
+
namespace openpower
{
namespace pels
@@ -79,12 +83,12 @@
_size = flattenedSize();
}
-FRUIdentity::FRUIdentity(MaintProcedure procedure)
+FRUIdentity::FRUIdentity(const std::string& procedureFromRegistry)
{
_type = substructureType;
_flags = maintenanceProc;
- setMaintenanceProcedure(procedure);
+ setMaintenanceProcedure(procedureFromRegistry);
_size = flattenedSize();
}
@@ -203,15 +207,25 @@
strncpy(_sn.data(), serialNumber.c_str(), _sn.size());
}
-void FRUIdentity::setMaintenanceProcedure(MaintProcedure procedure)
+void FRUIdentity::setMaintenanceProcedure(
+ const std::string& procedureFromRegistry)
{
_flags |= maintProcSupplied;
_flags &= ~pnSupplied;
- auto proc = pel_values::getMaintProcedure(procedure);
-
- strncpy(_pnOrProcedureID.data(), std::get<pel_values::mpNamePos>(*proc),
+ if (pel_values::maintenanceProcedures.count(procedureFromRegistry))
+ {
+ strncpy(
+ _pnOrProcedureID.data(),
+ pel_values::maintenanceProcedures.at(procedureFromRegistry).c_str(),
_pnOrProcedureID.size());
+ }
+ else
+ {
+ log<level::ERR>("Invalid maintenance procedure",
+ entry("PROCEDURE=%s", procedureFromRegistry.c_str()));
+ strncpy(_pnOrProcedureID.data(), "INVALID", _pnOrProcedureID.size());
+ }
// ensure null terminated
_pnOrProcedureID.back() = 0;
diff --git a/extensions/openpower-pels/fru_identity.hpp b/extensions/openpower-pels/fru_identity.hpp
index 016f3fd..669dc01 100644
--- a/extensions/openpower-pels/fru_identity.hpp
+++ b/extensions/openpower-pels/fru_identity.hpp
@@ -89,9 +89,10 @@
*
* Creates the object with a maintenance procedure callout.
*
- * @param[in] procedure - The procedure to use
+ * @param[in] procedureFromRegistry - The maintenance procedure name
+ * as defined in the message registry.
*/
- FRUIdentity(MaintProcedure procedure);
+ FRUIdentity(const std::string& procedureFromRegistry);
/**
* @brief Flatten the object into the stream
@@ -230,8 +231,11 @@
* @brief Sets the 8 character null terminated procedure
* field. This is in the same field as the part
* number since they are mutually exclusive.
+ *
+ * @param procedureFromRegistry - The procedure name as defined in
+ * the PEL message registry.
*/
- void setMaintenanceProcedure(MaintProcedure procedure);
+ void setMaintenanceProcedure(const std::string& procedureFromRegistry);
/**
* @brief The callout substructure type field. Will be "ID".
diff --git a/extensions/openpower-pels/pel_types.hpp b/extensions/openpower-pels/pel_types.hpp
index f6f66a5..1706c98 100644
--- a/extensions/openpower-pels/pel_types.hpp
+++ b/extensions/openpower-pels/pel_types.hpp
@@ -143,13 +143,5 @@
low = 'L'
};
-/**
- * @brief Maintenance procedure enums
- */
-enum class MaintProcedure
-{
- noVPDforFRU
-};
-
} // namespace pels
} // namespace openpower
diff --git a/extensions/openpower-pels/pel_values.cpp b/extensions/openpower-pels/pel_values.cpp
index 9fb6f05..07a0671 100644
--- a/extensions/openpower-pels/pel_values.cpp
+++ b/extensions/openpower-pels/pel_values.cpp
@@ -212,10 +212,11 @@
{0x4C, "low", "Lowest priority replacement"}};
/**
- * @brief The possible maintenance procedures.
+ * @brief Map of the registry names for the maintenance procedures
+ * to their actual names.
*/
-const MaintenanceProcedureValues maintenanceProcedures = {
- {MaintProcedure::noVPDforFRU, "no_vpd_for_fru", "BMCSP01"}};
+const std::map<std::string, std::string> maintenanceProcedures = {
+ {"no_vpd_for_fru", "BMCSP01"}};
PELValues::const_iterator findByValue(uint32_t value, const PELValues& fields)
{
@@ -329,20 +330,6 @@
return foundValues;
}
-MaintenanceProcedureValues::const_iterator
- getMaintProcedure(MaintProcedure procedure)
-{
- auto proc =
- std::find_if(maintenanceProcedures.begin(), maintenanceProcedures.end(),
- [procedure](const auto& entry) {
- return std::get<mpEnumPos>(entry) == procedure;
- });
-
- assert(proc != maintenanceProcedures.end());
-
- return proc;
-}
-
} // namespace pel_values
} // namespace pels
} // namespace openpower
diff --git a/extensions/openpower-pels/pel_values.hpp b/extensions/openpower-pels/pel_values.hpp
index 163c6da..04c287d 100644
--- a/extensions/openpower-pels/pel_values.hpp
+++ b/extensions/openpower-pels/pel_values.hpp
@@ -26,19 +26,6 @@
using PELFieldValue = std::tuple<uint32_t, const char*, const char*>;
using PELValues = std::vector<PELFieldValue>;
-// The maintenance procedure enum
-const int mpEnumPos = 0;
-
-// The maintenance procedure value from the registry
-const int mpRegistryNamePos = 1;
-
-// The string name of the maintenance procedure
-const int mpNamePos = 2;
-
-using MaintenanceProcedureValue =
- std::tuple<MaintProcedure, const char*, const char*>;
-using MaintenanceProcedureValues = std::vector<MaintenanceProcedureValue>;
-
const std::string sectionVer = "Section Version";
const std::string subSection = "Sub-section type";
const std::string createdBy = "Created by";
@@ -85,19 +72,6 @@
const PELValues& fields);
/**
- * @brief Finds the entry in the maintenance procedure list
- * based on the enum passed in.
- *
- * The function asserts that a result is found.
- *
- * @param[in] procedure - The procedure enum
- *
- * @return const_iterator - Iterator for that list entry
- */
-MaintenanceProcedureValues::const_iterator
- getMaintProcedure(MaintProcedure procedure);
-
-/**
* @brief The values for the 'subsystem' field in the User Header
*/
extern const PELValues subsystemValues;
@@ -158,11 +132,9 @@
extern const std::map<bool, std::string> boolString;
/**
- * @brief All maintenance procedures.
- *
- * The procedure enum, registry name, and actual string value.
+ * @brief Map for maintenance procedures
*/
-extern const MaintenanceProcedureValues maintenanceProcedures;
+extern const std::map<std::string, std::string> maintenanceProcedures;
} // namespace pel_values
} // namespace pels
diff --git a/extensions/openpower-pels/src.cpp b/extensions/openpower-pels/src.cpp
index b15c777..5310947 100644
--- a/extensions/openpower-pels/src.cpp
+++ b/extensions/openpower-pels/src.cpp
@@ -539,7 +539,7 @@
// Use the 'NoVPDforFRU' maintenance procedure instead
callout = std::make_unique<src::Callout>(CalloutPriority::high,
- MaintProcedure::noVPDforFRU);
+ "no_vpd_for_fru");
}
_callouts->addCallout(std::move(callout));
diff --git a/test/openpower-pels/fru_identity_test.cpp b/test/openpower-pels/fru_identity_test.cpp
index 5e738aa..e31ffad 100644
--- a/test/openpower-pels/fru_identity_test.cpp
+++ b/test/openpower-pels/fru_identity_test.cpp
@@ -159,31 +159,46 @@
// Test the constructor that takes in a maint procedure
TEST(FRUIdentityTest, CreateProcedureCalloutTest)
{
- FRUIdentity fru{MaintProcedure::noVPDforFRU};
+ {
+ FRUIdentity fru{"no_vpd_for_fru"};
- EXPECT_EQ(fru.flattenedSize(), 12);
- EXPECT_EQ(fru.type(), 0x4944);
- EXPECT_EQ(fru.failingComponentType(), FRUIdentity::maintenanceProc);
- EXPECT_EQ(fru.getMaintProc().value(), "BMCSP01");
- EXPECT_FALSE(fru.getPN());
- EXPECT_FALSE(fru.getCCIN());
- EXPECT_FALSE(fru.getSN());
+ EXPECT_EQ(fru.flattenedSize(), 12);
+ EXPECT_EQ(fru.type(), 0x4944);
+ EXPECT_EQ(fru.failingComponentType(), FRUIdentity::maintenanceProc);
+ EXPECT_EQ(fru.getMaintProc().value(), "BMCSP01");
+ EXPECT_FALSE(fru.getPN());
+ EXPECT_FALSE(fru.getCCIN());
+ EXPECT_FALSE(fru.getSN());
- // Flatten and unflatten, then compare again
- std::vector<uint8_t> data;
- Stream stream{data};
- fru.flatten(stream);
+ // Flatten and unflatten, then compare again
+ std::vector<uint8_t> data;
+ Stream stream{data};
+ fru.flatten(stream);
- EXPECT_EQ(data.size(), fru.flattenedSize());
+ EXPECT_EQ(data.size(), fru.flattenedSize());
- stream.offset(0);
- FRUIdentity newFRU{stream};
+ stream.offset(0);
+ FRUIdentity newFRU{stream};
- EXPECT_EQ(newFRU.flattenedSize(), 12);
- EXPECT_EQ(newFRU.type(), 0x4944);
- EXPECT_EQ(newFRU.failingComponentType(), FRUIdentity::maintenanceProc);
- EXPECT_EQ(newFRU.getMaintProc().value(), "BMCSP01");
- EXPECT_FALSE(newFRU.getPN());
- EXPECT_FALSE(newFRU.getCCIN());
- EXPECT_FALSE(newFRU.getSN());
+ EXPECT_EQ(newFRU.flattenedSize(), 12);
+ EXPECT_EQ(newFRU.type(), 0x4944);
+ EXPECT_EQ(newFRU.failingComponentType(), FRUIdentity::maintenanceProc);
+ EXPECT_EQ(newFRU.getMaintProc().value(), "BMCSP01");
+ EXPECT_FALSE(newFRU.getPN());
+ EXPECT_FALSE(newFRU.getCCIN());
+ EXPECT_FALSE(newFRU.getSN());
+ }
+
+ {
+ // Invalid maintenance procedure
+ FRUIdentity fru{"invalid"};
+
+ EXPECT_EQ(fru.flattenedSize(), 12);
+ EXPECT_EQ(fru.type(), 0x4944);
+ EXPECT_EQ(fru.failingComponentType(), FRUIdentity::maintenanceProc);
+ EXPECT_EQ(fru.getMaintProc().value(), "INVALID");
+ EXPECT_FALSE(fru.getPN());
+ EXPECT_FALSE(fru.getCCIN());
+ EXPECT_FALSE(fru.getSN());
+ }
}
diff --git a/test/openpower-pels/pel_values_test.cpp b/test/openpower-pels/pel_values_test.cpp
index b90ccca..a62e6c5 100644
--- a/test/openpower-pels/pel_values_test.cpp
+++ b/test/openpower-pels/pel_values_test.cpp
@@ -38,9 +38,4 @@
s = findByName("foo", subsystemValues);
ASSERT_EQ(s, subsystemValues.end());
-
- auto p = getMaintProcedure(MaintProcedure::noVPDforFRU);
- ASSERT_NE(p, maintenanceProcedures.end());
- ASSERT_EQ("no_vpd_for_fru", std::get<mpRegistryNamePos>(*p));
- ASSERT_EQ("BMCSP01", std::get<mpNamePos>(*p));
}
diff --git a/test/openpower-pels/src_callout_test.cpp b/test/openpower-pels/src_callout_test.cpp
index 36e9e38..68090f7 100644
--- a/test/openpower-pels/src_callout_test.cpp
+++ b/test/openpower-pels/src_callout_test.cpp
@@ -281,7 +281,7 @@
// Create a callout object by passing in the maintenance procedure to add.
TEST(CalloutTest, TestProcedureCallout)
{
- Callout callout{CalloutPriority::medium, MaintProcedure::noVPDforFRU};
+ Callout callout{CalloutPriority::medium, "no_vpd_for_fru"};
// size/flags/pri/locsize fields + FRUIdentity size
// No location code.