pseq: Refactor storage of debug data
After a pgood fault is detected, debug data is collected from the power
sequencer device. This data is stored in the additional data map used
to log an error. The data is also written to the journal.
Previously the debug data was stored by one virtual method declared in
the StandardDevice class. This method stored all the debug data
including the GPIO values.
GPIO values are device-specific, so it is likely that subclasses will
need to override this method to provide better formatting of the GPIO
values. However, this means they will also be overriding other data
storage performed by an ancestor class. This will likely lead to
duplicated code.
Refactor the storage of GPIO values to a separate virtual method. This
will allow child classes to override just that functionality while
inheriting and reusing other data storage from ancestor classes.
Tested:
* Verified automated tests ran successfully
Change-Id: I654a6a6a2f5386e06f9e7887cecfbb74523b715f
Signed-off-by: Shawn McCarney <shawnmm@us.ibm.com>
diff --git a/phosphor-power-sequencer/src/standard_device.cpp b/phosphor-power-sequencer/src/standard_device.cpp
index b49b67f..a11a9cc 100644
--- a/phosphor-power-sequencer/src/standard_device.cpp
+++ b/phosphor-power-sequencer/src/standard_device.cpp
@@ -92,10 +92,22 @@
Services& services, const std::vector<int>& gpioValues,
std::map<std::string, std::string>& additionalData)
{
- additionalData.emplace("DEVICE_NAME", name);
- if (!gpioValues.empty())
+ try
{
- std::string valuesStr = format_utils::toString(std::span(gpioValues));
+ additionalData.emplace("DEVICE_NAME", name);
+ storeGPIOValues(services, gpioValues, additionalData);
+ }
+ catch (...)
+ {}
+}
+
+void StandardDevice::storeGPIOValues(
+ Services& services, const std::vector<int>& values,
+ std::map<std::string, std::string>& additionalData)
+{
+ if (!values.empty())
+ {
+ std::string valuesStr = format_utils::toString(std::span(values));
services.logInfoMsg(
std::format("Device {} GPIO values: {}", name, valuesStr));
additionalData.emplace("GPIO_VALUES", valuesStr);
diff --git a/phosphor-power-sequencer/src/standard_device.hpp b/phosphor-power-sequencer/src/standard_device.hpp
index af7e29e..210494f 100644
--- a/phosphor-power-sequencer/src/standard_device.hpp
+++ b/phosphor-power-sequencer/src/standard_device.hpp
@@ -112,11 +112,10 @@
/**
* Store pgood fault debug data in the specified additional data map.
*
- * The default implementation stores the device name and all the GPIO
- * values. The GPIO values are stored as a simple list of integers.
+ * The default implementation stores the device name and then calls
+ * storeGPIOValues().
*
- * Sub-classes should override if needed to store device-specific data
- * and/or a formatted representation of the GPIO values.
+ * Sub-classes should override if needed to store device-specific data.
*
* This method should NOT throw exceptions. If debug data cannot be
* obtained, the error should be caught and ignored so that pgood error
@@ -131,6 +130,24 @@
std::map<std::string, std::string>& additionalData);
/**
+ * Store GPIO values in the specified additional data map.
+ *
+ * The default implementation stores the values as a simple list of
+ * integers.
+ *
+ * Sub-classes should override if more advanced formatting is needed. For
+ * example, GPIOs could be stored individually with a name and value, or
+ * related GPIOs could be formatted as a group.
+ *
+ * @param services System services like hardware presence and the journal
+ * @param values GPIO values obtained from the device (if any)
+ * @param additionalData Additional data to include in an error log
+ */
+ virtual void
+ storeGPIOValues(Services& services, const std::vector<int>& values,
+ std::map<std::string, std::string>& additionalData);
+
+ /**
* Device name.
*/
std::string name{};