presence: Sensor conflict checking for AnyOf
There can be more than one way to detect the presence of a fan, such as
by a GPIO and by a nonzero tach reading. The AnyOf redundancy policy
only requires one of these to indicate present when determining the
overall fan presence state.
This commit adds the functionality to check for the case when one of the
method reports not present while another reports present. In this case,
the one reporting not present will be considered the wrong one, and
depending on the detection type either an information event log or just
a journal trace will be created.
Only one log per method per power cycle will occur. Since one of the
methods probably looks for nonzero tach readings, there is a 5 second
delay after a power on is detected before a conflict check is done.
If the GPIO method is where the problem is detected, an event log is
created. If it's instead the tach sensor method, then a trace will just
be put in the journal because there is already code watching for and
creating event logs for stopped tachs - the fan monitor code.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: I72a764ecff4076d6dc40335b92d177b6b3cfa2d9
diff --git a/presence/anyof.cpp b/presence/anyof.cpp
index b77d6d7..4dc2ec3 100644
--- a/presence/anyof.cpp
+++ b/presence/anyof.cpp
@@ -16,6 +16,7 @@
#include "anyof.hpp"
#include "fan.hpp"
+#include "get_power_state.hpp"
#include "psensor.hpp"
#include <phosphor-logging/log.hpp>
@@ -28,14 +29,32 @@
{
namespace presence
{
+
+using namespace std::chrono_literals;
+static const auto powerOnDelayTime = 5s;
+
AnyOf::AnyOf(const Fan& fan,
const std::vector<std::reference_wrapper<PresenceSensor>>& s) :
RedundancyPolicy(fan),
- state()
+ state(), _powerState(getPowerStateObject()),
+ _powerOnDelayTimer(sdeventplus::Event::get_default(),
+ std::bind(&AnyOf::delayedAfterPowerOn, this)),
+ _powerOn(false)
{
for (auto& sensor : s)
{
- state.emplace_back(sensor, false);
+ state.emplace_back(sensor, false, false);
+ }
+
+ _powerState->addCallback(
+ std::get<1>(fan) + "-anyOf",
+ std::bind(&AnyOf::powerStateChanged, this, std::placeholders::_1));
+
+ // If power is already on, give the fans some time to spin up
+ // before considering power to actually be on.
+ if (_powerState->isPowerOn())
+ {
+ _powerOnDelayTimer.restartOnce(powerOnDelayTime);
}
}
@@ -44,16 +63,34 @@
// Find the sensor that changed state.
auto sit =
std::find_if(state.begin(), state.end(), [&sensor](const auto& s) {
- return std::get<0>(s).get() == sensor;
+ return std::get<sensorPos>(s).get() == sensor;
});
if (sit != state.end())
{
+ auto origState =
+ std::any_of(state.begin(), state.end(),
+ [](const auto& s) { return std::get<presentPos>(s); });
+
// Update our cache of the sensors state and re-evaluate.
- std::get<bool>(*sit) = present;
+ std::get<presentPos>(*sit) = present;
auto newState =
std::any_of(state.begin(), state.end(),
- [](const auto& s) { return std::get<bool>(s); });
+ [](const auto& s) { return std::get<presentPos>(s); });
setPresence(fan, newState);
+
+ // At least 1 sensor said a fan was present, check if any disagree.
+ if (newState)
+ {
+ if (!origState)
+ {
+ // Fan plug detected, re-enable conflict logging
+ std::for_each(state.begin(), state.end(), [](auto& s) {
+ std::get<conflictPos>(s) = false;
+ });
+ }
+
+ checkSensorConflicts();
+ }
}
}
@@ -63,13 +100,77 @@
for (auto& s : state)
{
- auto& sensor = std::get<0>(s);
- std::get<bool>(s) = sensor.get().start();
+ auto& sensor = std::get<sensorPos>(s);
+ std::get<presentPos>(s) = sensor.get().start();
}
- auto present = std::any_of(state.begin(), state.end(),
- [](const auto& s) { return std::get<bool>(s); });
+ auto present = std::any_of(state.begin(), state.end(), [](const auto& s) {
+ return std::get<presentPos>(s);
+ });
setPresence(fan, present);
+
+ // At least one of the contained methods indicated present,
+ // so check that they all agree.
+ if (present)
+ {
+ checkSensorConflicts();
+ }
+}
+
+void AnyOf::checkSensorConflicts()
+{
+ if (!isPowerOn())
+ {
+ return;
+ }
+
+ // If at least one, but not all, sensors indicate present, then
+ // tell the not present ones to log a conflict if not already done.
+ if (std::any_of(
+ state.begin(), state.end(),
+ [this](const auto& s) { return std::get<presentPos>(s); }) &&
+ !std::all_of(state.begin(), state.end(),
+ [this](const auto& s) { return std::get<presentPos>(s); }))
+ {
+ for (auto& [sensor, present, conflict] : state)
+ {
+ if (!present && !conflict)
+ {
+ sensor.get().logConflict(invNamespace + std::get<1>(fan));
+ conflict = true;
+ }
+ }
+ }
+}
+
+void AnyOf::powerStateChanged(bool powerOn)
+{
+ if (powerOn)
+ {
+ // Clear the conflict state from last time
+ std::for_each(state.begin(), state.end(), [](auto& state) {
+ std::get<conflictPos>(state) = false;
+ });
+
+ // Wait to give the fans time to start spinning before
+ // considering power actually on.
+ _powerOnDelayTimer.restartOnce(powerOnDelayTime);
+ }
+ else
+ {
+ _powerOn = false;
+
+ if (_powerOnDelayTimer.isEnabled())
+ {
+ _powerOnDelayTimer.setEnabled(false);
+ }
+ }
+}
+
+void AnyOf::delayedAfterPowerOn()
+{
+ _powerOn = true;
+ checkSensorConflicts();
}
} // namespace presence
diff --git a/presence/anyof.hpp b/presence/anyof.hpp
index efe3c79..91865b0 100644
--- a/presence/anyof.hpp
+++ b/presence/anyof.hpp
@@ -1,8 +1,11 @@
#pragma once
#include "fan.hpp"
+#include "power_state.hpp"
#include "rpolicy.hpp"
+#include <sdeventplus/utility/timer.hpp>
+
#include <functional>
#include <vector>
@@ -62,8 +65,83 @@
void monitor() override;
private:
- /** @brief All presence sensors in the redundancy set. */
- std::vector<std::tuple<std::reference_wrapper<PresenceSensor>, bool>> state;
+ /**
+ * @brief Checks that the sensors contained in this policy all
+ * agree on the presence value. If they don't, then call
+ * logConflict() on the sensors that don't think the fan
+ * is present as they may be broken.
+ *
+ * This check will only happen when power is on.
+ */
+ void checkSensorConflicts();
+
+ /**
+ * @brief Callback function called after a post-poweron delay.
+ *
+ * The _powerOnDelayTimer is started when _powerState says the
+ * power is on to give fans a bit of time to spin up so tachs
+ * wouldn't be zero. This is the callback function for that timer.
+ *
+ * It will call checkSensorConflicts().
+ */
+ void delayedAfterPowerOn();
+
+ /**
+ * @brief Says if power is on, though not until the post
+ * power on delay is complete.
+ *
+ * @return bool - if power is on.
+ */
+ inline bool isPowerOn() const
+ {
+ return _powerOn;
+ }
+
+ /**
+ * @brief Called by the PowerState object when the power
+ * state changes.
+ *
+ * When power changes to on:
+ * - Clears the memory of previous sensor conflicts.
+ * - Starts the post power on delay timer.
+ *
+ * @param[in] powerOn - If power is now on or off.
+ */
+ void powerStateChanged(bool powerOn);
+
+ static constexpr size_t sensorPos = 0;
+ static constexpr size_t presentPos = 1;
+ static constexpr size_t conflictPos = 2;
+
+ /**
+ * @brief All presence sensors in the redundancy set.
+ *
+ * Each entry contains:
+ * - A reference to a PresenceSensor
+ * - The current presence state
+ * - If the sensors have logged conflicts in their answers.
+ */
+ std::vector<std::tuple<std::reference_wrapper<PresenceSensor>, bool, bool>>
+ state;
+
+ /**
+ * @brief Pointer to the PowerState object used to track power changes.
+ */
+ std::shared_ptr<PowerState> _powerState;
+
+ /**
+ * @brief Post power on delay timer, where the conflict checking code
+ * doesn't consider power on until this timer expires.
+ *
+ * This gives fans a chance to start spinning before checking them.
+ */
+ sdeventplus::utility::Timer<sdeventplus::ClockId::Monotonic>
+ _powerOnDelayTimer;
+
+ /**
+ * @brief Current power state.
+ */
+ bool _powerOn;
};
} // namespace presence
diff --git a/presence/fan.cpp b/presence/fan.cpp
index 9ea83f3..95dc4cd 100644
--- a/presence/fan.cpp
+++ b/presence/fan.cpp
@@ -31,7 +31,6 @@
using namespace std::literals::string_literals;
-static const auto invNamespace = "/xyz/openbmc_project/inventory"s;
static const auto itemIface = "xyz.openbmc_project.Inventory.Item"s;
static const auto invMgrIface = "xyz.openbmc_project.Inventory.Manager"s;
static const auto fanIface = "xyz.openbmc_project.Inventory.Item.Fan"s;
diff --git a/presence/fan.hpp b/presence/fan.hpp
index 3eab814..9d1fd20 100644
--- a/presence/fan.hpp
+++ b/presence/fan.hpp
@@ -11,6 +11,8 @@
namespace presence
{
+static const std::string invNamespace = "/xyz/openbmc_project/inventory";
+
/** @brief PrettyName, inventory path and time until error. */
using Fan = std::tuple<std::string, std::string, std::optional<size_t>>;
diff --git a/presence/gpio.cpp b/presence/gpio.cpp
index 1aa63a4..8e4a6d2 100644
--- a/presence/gpio.cpp
+++ b/presence/gpio.cpp
@@ -15,12 +15,15 @@
*/
#include "gpio.hpp"
+#include "logging.hpp"
#include "rpolicy.hpp"
+#include "sdbusplus.hpp"
#include <phosphor-logging/elog-errors.hpp>
#include <phosphor-logging/elog.hpp>
#include <sdeventplus/event.hpp>
#include <xyz/openbmc_project/Common/Callout/error.hpp>
+#include <xyz/openbmc_project/Logging/Entry/server.hpp>
#include <functional>
#include <tuple>
@@ -32,6 +35,10 @@
namespace presence
{
+const auto loggingService = "xyz.openbmc_project.Logging";
+const auto loggingPath = "/xyz/openbmc_project/logging";
+const auto loggingCreateIface = "xyz.openbmc_project.Logging.Create";
+
Gpio::Gpio(const std::string& physDevice, const std::string& device,
unsigned int physPin) :
currentState(false),
@@ -86,6 +93,40 @@
currentState = newState;
}
}
+
+void Gpio::logConflict(const std::string& fanInventoryPath) const
+{
+ using namespace sdbusplus::xyz::openbmc_project::Logging::server;
+ std::map<std::string, std::string> ad;
+ Entry::Level severity = Entry::Level::Informational;
+
+ static constexpr auto errorName =
+ "xyz.openbmc_project.Fan.Presence.Error.Detection";
+
+ ad.emplace("_PID", std::to_string(getpid()));
+ ad.emplace("CALLOUT_INVENTORY_PATH", fanInventoryPath);
+ ad.emplace("GPIO_NUM", std::to_string(pin));
+ ad.emplace("GPIO_DEVICE_PATH", (phys.c_str()));
+
+ getLogger().log(
+ fmt::format("GPIO presence detect for fan {} said not present but "
+ "other methods indicated present",
+ fanInventoryPath));
+ try
+ {
+ util::SDBusPlus::callMethod(loggingService, loggingPath,
+ loggingCreateIface, "Create", errorName,
+ severity, ad);
+ }
+ catch (const util::DBusError& e)
+ {
+ getLogger().log(
+ fmt::format("Call to create a {} error for fan {} failed: {}",
+ errorName, fanInventoryPath, e.what()),
+ Logger::error);
+ }
+}
+
} // namespace presence
} // namespace fan
} // namespace phosphor
diff --git a/presence/gpio.hpp b/presence/gpio.hpp
index 5f9e28b..ae4bc7f 100644
--- a/presence/gpio.hpp
+++ b/presence/gpio.hpp
@@ -79,6 +79,13 @@
*/
bool present() override;
+ /**
+ * @brief Called when this presence sensor doesn't agree with other ones.
+ *
+ * @param[in] fanInventoryPath - The fan inventory D-Bus object path.
+ */
+ void logConflict(const std::string& fanInventoryPath) const override;
+
private:
/** @brief Get the policy associated with this sensor. */
virtual RedundancyPolicy& getPolicy() = 0;
diff --git a/presence/psensor.hpp b/presence/psensor.hpp
index 8ce2d7e..cc65fe0 100644
--- a/presence/psensor.hpp
+++ b/presence/psensor.hpp
@@ -1,5 +1,6 @@
#pragma once
#include <cstdint>
+#include <string>
namespace phosphor
{
@@ -78,6 +79,13 @@
friend bool operator==(const PresenceSensor& l, const PresenceSensor& r);
+ /**
+ * @brief Called when this presence sensor doesn't agree with other ones.
+ *
+ * @param[in] fanInventoryPath - The fan inventory D-Bus object path.
+ */
+ virtual void logConflict(const std::string& fanInventoryPath) const = 0;
+
private:
/** @brief Unique sensor ID. */
std::size_t id;
diff --git a/presence/tach.cpp b/presence/tach.cpp
index 9d6c0e3..7304676 100644
--- a/presence/tach.cpp
+++ b/presence/tach.cpp
@@ -15,6 +15,7 @@
*/
#include "tach.hpp"
+#include "logging.hpp"
#include "rpolicy.hpp"
#include <fmt/format.h>
@@ -141,6 +142,17 @@
}
}
+void Tach::logConflict(const std::string& fanInventoryPath) const
+{
+ getLogger().log(fmt::format(
+ "Tach sensor presence detect for fan {} said not present but "
+ "other methods indicated present",
+ fanInventoryPath));
+
+ // Let the code that monitors fan faults create the event
+ // logs for stopped rotors.
+}
+
} // namespace presence
} // namespace fan
} // namespace phosphor
diff --git a/presence/tach.hpp b/presence/tach.hpp
index 4432660..8fd9a08 100644
--- a/presence/tach.hpp
+++ b/presence/tach.hpp
@@ -71,6 +71,13 @@
*/
bool present() override;
+ /**
+ * @brief Called when this presence sensor doesn't agree with other ones.
+ *
+ * @param[in] fanInventoryPath - The fan inventory D-Bus object path.
+ */
+ void logConflict(const std::string& fanInventoryPath) const override;
+
private:
/**
* @brief Get the policy associated with this sensor.