diff --git a/dbus/dbuspassive.cpp b/dbus/dbuspassive.cpp index 806ef40..75b83f5 100644 --- a/dbus/dbuspassive.cpp +++ b/dbus/dbuspassive.cpp @@ -26,6 +26,10 @@ #include #include +using Property = std::string; +using Value = std::variant; +using PropertyMap = std::map; + std::unique_ptr DbusPassive::createDbusPassive( sdbusplus::bus::bus& bus, const std::string& type, const std::string& id, DbusHelperInterface* helper, const conf::SensorConfig* info, @@ -61,6 +65,41 @@ std::unique_ptr DbusPassive::createDbusPassive( return nullptr; } + if (failed && (type == "fan")) + { + std::string path = getInventoryPath(id); + + try + { + bool present = true; + + auto pimMsg = tempBus.new_method_call("xyz.openbmc_project.Inventory.Manager", path.c_str(), "org.freedesktop.DBus.Properties", "GetAll"); + pimMsg.append("xyz.openbmc_project.Inventory.Item"); + + PropertyMap propMap; + + auto valueResponseMsg = bus.call(pimMsg); + valueResponseMsg.read(propMap); + + // If no error was set, the data should all be there. + auto findPresent = propMap.find("Present"); + if (findPresent != propMap.end()) + { + present = std::get(findPresent->second); + } + + // std::cerr << "DbusPassive::createDbusPassive() present: '" << present << "'\n"; + if (!present) + { + failed = false; + } + } + catch (const std::exception& e) + { + // Ignore failure + } + } + /* if these values are zero, they're ignored. */ if (info->ignoreDbusMinMax) { @@ -68,6 +107,7 @@ std::unique_ptr DbusPassive::createDbusPassive( settings.max = 0; } + // std::cerr << "DbusPassive::createDbusPassive() failed: " << failed << "\n"; return std::make_unique(bus, type, id, helper, settings, failed, path, redundancy); } @@ -78,8 +118,9 @@ DbusPassive::DbusPassive( bool failed, const std::string& path, const std::shared_ptr& redundancy) : ReadInterface(), - _bus(bus), _signal(bus, getMatch(type, id).c_str(), dbusHandleSignal, this), - _id(id), _helper(helper), _failed(failed), path(path), + _id(id), _type(type), _helper(helper), _bus(bus), + _signal(bus, getMatch(type, id).c_str(), dbusHandleSignal, this), + _failed(failed), path(path), redundancy(redundancy) { @@ -109,19 +150,23 @@ void DbusPassive::setValue(double value) bool DbusPassive::getFailed(void) const { + // std::cerr << "DbusPassive::getFailed() redundancy: " << redundancy << "\n"; if (redundancy) { const std::set& failures = redundancy->getFailed(); if (failures.find(path) != failures.end()) { + // std::cerr << "DbusPassive::getFailed() redundancy has signalled failure!\n"; return true; } } + // std::cerr << "DbusPassive::getFailed() _failed: " << _failed << "\n"; return _failed; } void DbusPassive::setFailed(bool value) { + // std::cerr << "DbusPassive::setFailed(" << value << ")\n"; _failed = value; } @@ -189,6 +234,43 @@ int handleSensorValue(sdbusplus::message::message& msg, DbusPassive* owner) { asserted = std::get(criticalAlarmHigh->second); } + + // std::cerr << "handleSensorValue() asserted: " << asserted << " type: '" << owner->_type << "'\n"; + if (asserted && (owner->_type == "fan")) + { + std::string path = getInventoryPath(owner->_id); + + try + { + bool present = true; + + auto pimMsg = owner->_bus.new_method_call("xyz.openbmc_project.Inventory.Manager", path.c_str(), "org.freedesktop.DBus.Properties", "GetAll"); + pimMsg.append("xyz.openbmc_project.Inventory.Item"); + + PropertyMap propMap; + + auto valueResponseMsg = owner->_bus.call(pimMsg); + valueResponseMsg.read(propMap); + + // If no error was set, the data should all be there. + auto findPresent = propMap.find("Present"); + if (findPresent != propMap.end()) + { + present = std::get(findPresent->second); + } + + // std::cerr << "handleSensorValue() present: '" << present << "'\n"; + if (!present) + { + asserted = false; + } + } + catch (const std::exception& e) + { + // Ignore failure + } + } + owner->setFailed(asserted); } diff --git a/dbus/dbuspassive.hpp b/dbus/dbuspassive.hpp index f00a2ea..d025ed4 100644 --- a/dbus/dbuspassive.hpp +++ b/dbus/dbuspassive.hpp @@ -57,12 +57,14 @@ class DbusPassive : public ReadInterface double getMax(void); double getMin(void); - private: + std::string _id; // for debug identification + std::string _type; // for inventory match + DbusHelperInterface* _helper; sdbusplus::bus::bus& _bus; + + private: sdbusplus::server::match::match _signal; int64_t _scale; - std::string _id; // for debug identification - DbusHelperInterface* _helper; std::mutex _lock; double _value = 0; diff --git a/dbus/dbuswrite.cpp b/dbus/dbuswrite.cpp index 502b988..28a04e4 100644 --- a/dbus/dbuswrite.cpp +++ b/dbus/dbuswrite.cpp @@ -65,6 +65,8 @@ void DbusWritePercent::write(double value) "org.freedesktop.DBus.Properties", "Set"); mesg.append(pwmInterface, "Target", std::variant(ovalue)); + // std::cerr << "DbusWritePercent::write(" << value << "), writing to connection '" << connectionName.c_str() << "', path '" << path.c_str() << "' xyz.openbmc_project.Control.FanPwm Target '" << ovalue << "'\n"; + try { // TODO: if we don't use the reply, call_noreply() @@ -111,6 +113,8 @@ void DbusWrite::write(double value) "org.freedesktop.DBus.Properties", "Set"); mesg.append(pwmInterface, "Target", std::variant(value)); + // std::cerr << "DbusWrite::write(" << value << "), writing to connection '" << connectionName.c_str() << "', path '" << path.c_str() << "' xyz.openbmc_project.Control.FanPwm Target '" << value << "'\n"; + try { // TODO: consider call_noreplly diff --git a/dbus/util.cpp b/dbus/util.cpp index 33e0985..8a639eb 100644 --- a/dbus/util.cpp +++ b/dbus/util.cpp @@ -148,6 +148,7 @@ bool DbusHelper::thresholdsAsserted(sdbusplus::bus::bus& bus, { asserted = std::get(findCriticalHigh->second); } + // std::cerr << "DbusHelper::thresholdsAsserted path: '" << path << "' asserted: " << asserted << "\n"; return asserted; } @@ -170,6 +171,11 @@ std::string getSensorPath(const std::string& type, const std::string& id) return std::string("/xyz/openbmc_project/sensors/" + layer + "/" + id); } +std::string getInventoryPath(const std::string& id) +{ + return std::string("/xyz/openbmc_project/inventory/system/chassis/motherboard/" + id); +} + std::string getMatch(const std::string& type, const std::string& id) { return std::string("type='signal'," diff --git a/pid/ec/pid.cpp b/pid/ec/pid.cpp index 7d8b403..7a8b91d 100644 --- a/pid/ec/pid.cpp +++ b/pid/ec/pid.cpp @@ -16,6 +16,8 @@ #include "pid.hpp" +#include + namespace ec { @@ -72,6 +74,8 @@ double pid(pid_info_t* pidinfoptr, double input, double setpoint) output = proportionalTerm + integralTerm + feedFwdTerm; output = clamp(output, pidinfoptr->outLim.min, pidinfoptr->outLim.max); + // std::cerr << "pid() input: " << input << " error: " << error << " proportionalTerm: " << proportionalTerm << " integralTerm: " << integralTerm << " feedFwdTerm: " << feedFwdTerm << " output: " << output << "\n"; + // slew rate // TODO(aarena) - Simplify logic as Andy suggested by creating dynamic // outLim_min/max that are affected by slew rate control and just clamping diff --git a/pid/fancontroller.cpp b/pid/fancontroller.cpp index dd26d16..3a2c743 100644 --- a/pid/fancontroller.cpp +++ b/pid/fancontroller.cpp @@ -106,6 +106,7 @@ double FanController::setptProc(void) setFanDirection(FanSpeedDirection::NEUTRAL); } + // std::cerr << "FanController::setptProc() prev: " << prev << " going to return: " << maxRPM << "\n"; setSetpoint(maxRPM); return (maxRPM); @@ -128,12 +129,15 @@ void FanController::outputProc(double value) } } + // std::cerr << "FanController::outputProc(" << value << ") _owner->getFailSafeMode(): " << _owner->getFailSafeMode() << " percent: " << percent << "\n"; + // value and kFanFailSafeDutyCycle are 10 for 10% so let's fix that. percent /= 100; // PidSensorMap for writing. for (const auto& it : _inputs) { + // std::cerr << "FanController::outputProc(" << value << ") for '" << it << "', writing value '" << percent << "'\n"; auto sensor = _owner->getSensor(it); sensor->write(percent); } diff --git a/pid/pidcontroller.cpp b/pid/pidcontroller.cpp index e3eaaff..e23149e 100644 --- a/pid/pidcontroller.cpp +++ b/pid/pidcontroller.cpp @@ -73,6 +73,8 @@ void PIDController::process(void) output = ec::pid(info, lastInput, setpt); } + // std::cerr << "PIDController::process() setpt: " << setpt << " input: " << input << " output: " << output << "\n"; + // Output new value outputProc(output); diff --git a/pid/zone.cpp b/pid/zone.cpp index 6a63671..8cdb9f7 100644 --- a/pid/zone.cpp +++ b/pid/zone.cpp @@ -53,6 +53,11 @@ void PIDZone::setManualMode(bool mode) bool PIDZone::getFailSafeMode(void) const { // If any keys are present at least one sensor is in fail safe mode. + // if (!_failSafeSensors.empty()) + // { + // std::set::iterator it = _failSafeSensors.begin(); + // std::cerr << "PIDZone::getFailSafeMode sensor '" << *it << "' failed!\n"; + // } return !_failSafeSensors.empty(); } @@ -139,6 +144,7 @@ void PIDZone::determineMaxSetPointRequest(void) */ max = std::max(getMinThermalSetpoint(), max); + // std::cerr << "PIDZone::determineMaxSetPointRequest _SetPoints.size(): " << _SetPoints.size() << " tuningEnabled: " << tuningEnabled << " max: " << max << "\n"; if (tuningEnabled) { /* @@ -251,6 +257,7 @@ void PIDZone::updateFanTelemetry(void) // check if fan fail. if (sensor->getFailed()) { + std::cerr << "Fan '" << f << "' has failed!.\n"; _failSafeSensors.insert(f); } else if (timeout != 0 && duration >= period) @@ -288,6 +295,17 @@ void PIDZone::updateSensors(void) for (const auto& t : _thermalInputs) { auto sensor = _mgr.getSensor(t); + if (!sensor) + { + // std::cerr << "Skipping missing sensor '" << t << "'.\n"; + // Check if it's in there: remove it. + auto kt = _failSafeSensors.find(t); + if (kt != _failSafeSensors.end()) + { + _failSafeSensors.erase(kt); + } + continue; + } ReadReturn r = sensor->read(); int64_t timeout = sensor->getTimeout(); @@ -303,7 +321,7 @@ void PIDZone::updateSensors(void) } else if (timeout != 0 && duration >= period) { - // std::cerr << "Entering fail safe mode.\n"; + std::cerr << "Entering fail safe mode.\n"; _failSafeSensors.insert(t); } else @@ -315,6 +333,8 @@ void PIDZone::updateSensors(void) _failSafeSensors.erase(kt); } } + + // std::cerr << "PIDZone::updateSensors for '" << t << "', got value '" << _cachedValuesByName[t] << "'\n"; } return; diff --git a/sensors/builder.cpp b/sensors/builder.cpp index 4da1cf2..464ec9c 100644 --- a/sensors/builder.cpp +++ b/sensors/builder.cpp @@ -87,9 +87,7 @@ SensorManager } if (ri == nullptr) { - throw SensorBuildException( - "Failed to create dbus passive sensor: " + name + - " of type: " + info->type); + continue; } break; case IOInterfaceType::EXTERNAL: diff --git a/sensors/manager.hpp b/sensors/manager.hpp index 411b302..bd1dd48 100644 --- a/sensors/manager.hpp +++ b/sensors/manager.hpp @@ -38,6 +38,8 @@ class SensorManager // TODO(venture): Should implement read/write by name. Sensor* getSensor(const std::string& name) const { + if (_sensorMap.count(name) <= 0) + return nullptr; return _sensorMap.at(name).get(); } diff --git a/util.hpp b/util.hpp index e40b61f..5e2feae 100644 --- a/util.hpp +++ b/util.hpp @@ -149,6 +149,7 @@ class DbusHelper : public DbusHelperInterface }; std::string getSensorPath(const std::string& type, const std::string& id); +std::string getInventoryPath(const std::string& id); std::string getMatch(const std::string& type, const std::string& id); void scaleSensorReading(const double min, const double max, double& value); bool validType(const std::string& type);