diff options
-rw-r--r-- | dbus/dbusconfiguration.cpp | 112 | ||||
-rw-r--r-- | dbus/dbusconfiguration.hpp | 2 | ||||
-rw-r--r-- | main.cpp | 113 | ||||
-rw-r--r-- | pid/pidloop.cpp | 5 | ||||
-rw-r--r-- | sensors/builder.cpp | 5 | ||||
-rw-r--r-- | sensors/builder.hpp | 5 | ||||
-rw-r--r-- | sensors/manager.hpp | 21 | ||||
-rw-r--r-- | test/pid_zone_unittest.cpp | 4 | ||||
-rw-r--r-- | test/sensor_manager_unittest.cpp | 4 | ||||
-rw-r--r-- | util.hpp | 2 |
10 files changed, 161 insertions, 112 deletions
diff --git a/dbus/dbusconfiguration.cpp b/dbus/dbusconfiguration.cpp index 2c9bbc2..870e5e8 100644 --- a/dbus/dbusconfiguration.cpp +++ b/dbus/dbusconfiguration.cpp @@ -18,15 +18,16 @@ #include "util.hpp" #include <algorithm> +#include <boost/asio/steady_timer.hpp> #include <chrono> #include <functional> #include <iostream> +#include <list> #include <regex> #include <sdbusplus/bus.hpp> #include <sdbusplus/bus/match.hpp> #include <sdbusplus/exception.hpp> #include <set> -#include <thread> #include <unordered_map> #include <variant> @@ -139,16 +140,6 @@ void debugPrint(void) std::cout << "}\n\n"; } -int eventHandler(sd_bus_message*, void*, sd_bus_error*) -{ - // do a brief sleep as we tend to get a bunch of these events at - // once - std::this_thread::sleep_for(std::chrono::seconds(5)); - std::cout << "New configuration detected, restarting\n."; - std::exit(EXIT_SUCCESS); // service file should make us restart - return 1; -} - size_t getZoneIndex(const std::string& name, std::vector<std::string>& zones) { auto it = std::find(zones.begin(), zones.end(), name); @@ -229,8 +220,74 @@ std::vector<std::string> getSelectedProfiles(sdbusplus::bus::bus& bus) return ret; } -void init(sdbusplus::bus::bus& bus) +int eventHandler(sd_bus_message*, void* context, sd_bus_error*) +{ + + if (context == nullptr) + { + throw std::runtime_error("Invalid match"); + } + boost::asio::steady_timer* timer = + static_cast<boost::asio::steady_timer*>(context); + + // do a brief sleep as we tend to get a bunch of these events at + // once + timer->expires_after(std::chrono::seconds(2)); + timer->async_wait([](const boost::system::error_code ec) { + if (ec == boost::asio::error::operation_aborted) + { + /* another timer started*/ + return; + } + + std::cout << "New configuration detected, reloading\n."; + restartControlLoops(); + }); + + return 1; +} + +void createMatches(sdbusplus::bus::bus& bus, boost::asio::steady_timer& timer) +{ + // this is a list because the matches can't be moved + static std::list<sdbusplus::bus::match::match> matches; + + const std::array<std::string, 5> interfaces = { + thermalControlIface, fanProfileConfigurationIface, + pidConfigurationInterface, pidZoneConfigurationInterface, + stepwiseConfigurationInterface}; + + // this list only needs to be created once + if (!matches.empty()) + { + return; + } + + // we restart when the configuration changes or there are new sensors + for (const auto& interface : interfaces) + { + matches.emplace_back( + bus, + "type='signal',member='PropertiesChanged',arg0namespace='" + + interface + "'", + eventHandler, &timer); + } + matches.emplace_back( + bus, + "type='signal',member='InterfacesAdded',arg0path='/xyz/openbmc_project/" + "sensors/'", + eventHandler, &timer); +} + +bool init(sdbusplus::bus::bus& bus, boost::asio::steady_timer& timer) { + + sensorConfig.clear(); + zoneConfig.clear(); + zoneDetailsConfig.clear(); + + createMatches(bus, timer); + using DbusVariantType = std::variant<uint64_t, int64_t, double, std::string, std::vector<std::string>, std::vector<double>>; @@ -240,27 +297,6 @@ void init(sdbusplus::bus::bus& bus) std::unordered_map<std::string, std::unordered_map<std::string, DbusVariantType>>>; - // restart on configuration properties changed - static sdbusplus::bus::match::match configMatch( - bus, - "type='signal',member='PropertiesChanged',arg0namespace='" + - std::string(pidConfigurationInterface) + "'", - eventHandler); - - // restart on profile change - static sdbusplus::bus::match::match profileMatch( - bus, - "type='signal',member='PropertiesChanged',arg0namespace='" + - std::string(thermalControlIface) + "'", - eventHandler); - - // restart on sensors changed - static sdbusplus::bus::match::match sensorAdded( - bus, - "type='signal',member='InterfacesAdded',arg0path='/xyz/openbmc_project/" - "sensors/'", - eventHandler); - auto mapper = bus.new_method_call("xyz.openbmc_project.ObjectMapper", "/xyz/openbmc_project/object_mapper", @@ -768,12 +804,10 @@ void init(sdbusplus::bus::bus& bus) } if (zoneConfig.empty() || zoneDetailsConfig.empty()) { - std::cerr << "No fan zones, application pausing until reboot\n"; - while (1) - { - bus.process_discard(); - bus.wait(); - } + std::cerr + << "No fan zones, application pausing until new configuration\n"; + return false; } + return true; } } // namespace dbus_configuration diff --git a/dbus/dbusconfiguration.hpp b/dbus/dbusconfiguration.hpp index b3032e9..a9b12a9 100644 --- a/dbus/dbusconfiguration.hpp +++ b/dbus/dbusconfiguration.hpp @@ -21,5 +21,5 @@ namespace dbus_configuration { -void init(sdbusplus::bus::bus& bus); +bool init(sdbusplus::bus::bus& bus, boost::asio::steady_timer& timer); } // namespace dbus_configuration @@ -48,47 +48,43 @@ #include "dbus/dbusconfiguration.hpp" #endif -/* The YAML converted sensor list. */ +/* The configuration converted sensor list. */ std::map<std::string, struct conf::SensorConfig> sensorConfig = {}; -/* The YAML converted PID list. */ +/* The configuration converted PID list. */ std::map<int64_t, conf::PIDConf> zoneConfig = {}; -/* The YAML converted Zone configuration. */ +/* The configuration converted Zone configuration. */ std::map<int64_t, struct conf::ZoneConfig> zoneDetailsConfig = {}; /** the swampd daemon will check for the existence of this file. */ constexpr auto jsonConfigurationPath = "/usr/share/swampd/config.json"; +std::string configPath = ""; -int main(int argc, char* argv[]) -{ - int rc = 0; - std::string configPath = ""; - loggingPath = ""; - loggingEnabled = false; - tuningEnabled = false; - - CLI::App app{"OpenBMC Fan Control Daemon"}; - - app.add_option("-c,--conf", configPath, - "Optional parameter to specify configuration at run-time") - ->check(CLI::ExistingFile); - app.add_option("-l,--log", loggingPath, - "Optional parameter to specify logging folder") - ->check(CLI::ExistingDirectory); - app.add_flag("-t,--tuning", tuningEnabled, "Enable or disable tuning"); +/* async io context for operation */ +boost::asio::io_context io; - loggingEnabled = (!loggingPath.empty()); +/* buses for system control */ +static sdbusplus::asio::connection modeControlBus(io); +static sdbusplus::asio::connection + hostBus(io, sdbusplus::bus::new_system().release()); +static sdbusplus::asio::connection + passiveBus(io, sdbusplus::bus::new_system().release()); - CLI11_PARSE(app, argc, argv); +void restartControlLoops() +{ + static SensorManager mgmr; + static std::unordered_map<int64_t, std::unique_ptr<PIDZone>> zones; + static std::list<boost::asio::steady_timer> timers; - auto modeControlBus = sdbusplus::bus::new_system(); - static constexpr auto modeRoot = "/xyz/openbmc_project/settings/fanctrl"; - // Create a manager for the ModeBus because we own it. - sdbusplus::server::manager::manager(modeControlBus, modeRoot); + timers.clear(); #if CONFIGURE_DBUS + + static boost::asio::steady_timer reloadTimer(io); + if (!dbus_configuration::init(modeControlBus, reloadTimer)) { - dbus_configuration::init(modeControlBus); + return; // configuration not ready } + #else const std::string& path = (configPath.length() > 0) ? configPath : jsonConfigurationPath; @@ -110,42 +106,57 @@ int main(int argc, char* argv[]) } #endif - SensorManager mgmr = buildSensors(sensorConfig); - std::unordered_map<int64_t, std::unique_ptr<PIDZone>> zones = - buildZones(zoneConfig, zoneDetailsConfig, mgmr, modeControlBus); + mgmr = buildSensors(sensorConfig, passiveBus, hostBus); + zones = buildZones(zoneConfig, zoneDetailsConfig, mgmr, modeControlBus); if (0 == zones.size()) { std::cerr << "No zones defined, exiting.\n"; - return rc; + std::exit(EXIT_FAILURE); } - /* - * All sensors are managed by one manager, but each zone has a pointer to - * it. - */ + for (const auto& i : zones) + { + auto& timer = timers.emplace_back(io); + std::cerr << "pushing zone " << i.first << "\n"; + pidControlLoop(i.second.get(), timer); + } +} - auto& hostSensorBus = mgmr.getHostBus(); - auto& passiveListeningBus = mgmr.getPassiveBus(); +int main(int argc, char* argv[]) +{ + loggingPath = ""; + loggingEnabled = false; + tuningEnabled = false; - boost::asio::io_context io; - sdbusplus::asio::connection passiveBus(io, passiveListeningBus.release()); + CLI::App app{"OpenBMC Fan Control Daemon"}; - sdbusplus::asio::connection hostBus(io, hostSensorBus.release()); - hostBus.request_name("xyz.openbmc_project.Hwmon.external"); + app.add_option("-c,--conf", configPath, + "Optional parameter to specify configuration at run-time") + ->check(CLI::ExistingFile); + app.add_option("-l,--log", loggingPath, + "Optional parameter to specify logging folder") + ->check(CLI::ExistingDirectory); + app.add_flag("-t,--tuning", tuningEnabled, "Enable or disable tuning"); - sdbusplus::asio::connection modeBus(io, modeControlBus.release()); - modeBus.request_name("xyz.openbmc_project.State.FanCtrl"); + loggingEnabled = (!loggingPath.empty()); - std::list<boost::asio::steady_timer> timers; + CLI11_PARSE(app, argc, argv); - for (const auto& i : zones) - { - auto& timer = timers.emplace_back(io); - std::cerr << "pushing zone" << std::endl; - pidControlLoop(i.second.get(), timer); - } + static constexpr auto modeRoot = "/xyz/openbmc_project/settings/fanctrl"; + // Create a manager for the ModeBus because we own it. + sdbusplus::server::manager::manager( + static_cast<sdbusplus::bus::bus&>(modeControlBus), modeRoot); + hostBus.request_name("xyz.openbmc_project.Hwmon.external"); + modeControlBus.request_name("xyz.openbmc_project.State.FanCtrl"); + + /* + * All sensors are managed by one manager, but each zone has a pointer to + * it. + */ + + restartControlLoops(); io.run(); - return rc; + return 0; } diff --git a/pid/pidloop.cpp b/pid/pidloop.cpp index f613c7e..9eed3a8 100644 --- a/pid/pidloop.cpp +++ b/pid/pidloop.cpp @@ -57,6 +57,11 @@ void pidControlLoop(PIDZone* zone, boost::asio::steady_timer& timer, bool first, timer.expires_after(std::chrono::milliseconds(100)); timer.async_wait( [zone, &timer, ms100cnt](const boost::system::error_code& ec) mutable { + if (ec == boost::asio::error::operation_aborted) + { + return; // timer being canceled, stop loop + } + /* * This should sleep on the conditional wait for the listen thread * to tell us it's in sync. But then we also need a timeout option diff --git a/sensors/builder.cpp b/sensors/builder.cpp index 1a74adb..f5c01b3 100644 --- a/sensors/builder.cpp +++ b/sensors/builder.cpp @@ -38,9 +38,10 @@ static constexpr bool deferSignals = true; static DbusHelper helper; SensorManager - buildSensors(const std::map<std::string, struct conf::SensorConfig>& config) + buildSensors(const std::map<std::string, struct conf::SensorConfig>& config, + sdbusplus::bus::bus& passive, sdbusplus::bus::bus& host) { - SensorManager mgmr; + SensorManager mgmr(passive, host); auto& hostSensorBus = mgmr.getHostBus(); auto& passiveListeningBus = mgmr.getPassiveBus(); diff --git a/sensors/builder.hpp b/sensors/builder.hpp index 224d467..688a931 100644 --- a/sensors/builder.hpp +++ b/sensors/builder.hpp @@ -9,5 +9,6 @@ /** * Build the sensors and associate them with a SensorManager. */ -SensorManager buildSensors( - const std::map<std::string, struct conf::SensorConfig>& config); +SensorManager + buildSensors(const std::map<std::string, struct conf::SensorConfig>& config, + sdbusplus::bus::bus& passive, sdbusplus::bus::bus& host); diff --git a/sensors/manager.hpp b/sensors/manager.hpp index a6b4b9d..411b302 100644 --- a/sensors/manager.hpp +++ b/sensors/manager.hpp @@ -15,19 +15,14 @@ class SensorManager { public: - SensorManager(sdbusplus::bus::bus&& pass, sdbusplus::bus::bus&& host) : - _passiveListeningBus(std::move(pass)), _hostSensorBus(std::move(host)) + SensorManager(sdbusplus::bus::bus& pass, sdbusplus::bus::bus& host) : + _passiveListeningBus(&pass), _hostSensorBus(&host) { // manager gets its interface from the bus. :D - sdbusplus::server::manager::manager(_hostSensorBus, SensorRoot); - } - - SensorManager() : - SensorManager(std::move(sdbusplus::bus::new_system()), - std::move(sdbusplus::bus::new_system())) - { + sdbusplus::server::manager::manager(*_hostSensorBus, SensorRoot); } + SensorManager() = default; ~SensorManager() = default; SensorManager(const SensorManager&) = delete; SensorManager& operator=(const SensorManager&) = delete; @@ -48,20 +43,20 @@ class SensorManager sdbusplus::bus::bus& getPassiveBus(void) { - return _passiveListeningBus; + return *_passiveListeningBus; } sdbusplus::bus::bus& getHostBus(void) { - return _hostSensorBus; + return *_hostSensorBus; } private: std::map<std::string, std::unique_ptr<Sensor>> _sensorMap; std::map<std::string, std::vector<std::string>> _sensorTypeList; - sdbusplus::bus::bus _passiveListeningBus; - sdbusplus::bus::bus _hostSensorBus; + sdbusplus::bus::bus* _passiveListeningBus; + sdbusplus::bus::bus* _hostSensorBus; static constexpr auto SensorRoot = "/xyz/openbmc_project/extsensors"; }; diff --git a/test/pid_zone_unittest.cpp b/test/pid_zone_unittest.cpp index 2270fa6..11ba4c1 100644 --- a/test/pid_zone_unittest.cpp +++ b/test/pid_zone_unittest.cpp @@ -37,7 +37,7 @@ TEST(PidZoneConstructorTest, BoringConstructorTest) IsNull(), _, StrEq("/xyz/openbmc_project/extsensors"))) .WillOnce(Return(0)); - SensorManager m(std::move(bus_mock_passive), std::move(bus_mock_host)); + SensorManager m(bus_mock_passive, bus_mock_host); bool defer = true; const char* objPath = "/path/"; @@ -74,7 +74,7 @@ class PidZoneTest : public ::testing::Test auto bus_mock_mode = sdbusplus::get_mocked_new(&sdbus_mock_mode); // Compiler weirdly not happy about just instantiating mgr(...); - SensorManager m(std::move(bus_mock_passive), std::move(bus_mock_host)); + SensorManager m(bus_mock_passive, bus_mock_host); mgr = std::move(m); SetupDbusObject(&sdbus_mock_mode, defer, objPath, modeInterface, diff --git a/test/sensor_manager_unittest.cpp b/test/sensor_manager_unittest.cpp index 8a89628..9f0e26c 100644 --- a/test/sensor_manager_unittest.cpp +++ b/test/sensor_manager_unittest.cpp @@ -24,7 +24,7 @@ TEST(SensorManagerTest, BoringConstructorTest) IsNull(), _, StrEq("/xyz/openbmc_project/extsensors"))) .WillOnce(Return(0)); - SensorManager s(std::move(bus_mock_passive), std::move(bus_mock_host)); + SensorManager s(bus_mock_passive, bus_mock_host); // Success } @@ -43,7 +43,7 @@ TEST(SensorManagerTest, AddSensorInvalidTypeTest) IsNull(), _, StrEq("/xyz/openbmc_project/extsensors"))) .WillOnce(Return(0)); - SensorManager s(std::move(bus_mock_passive), std::move(bus_mock_host)); + SensorManager s(bus_mock_passive, bus_mock_host); std::string name = "name"; std::string type = "invalid"; @@ -29,6 +29,8 @@ IOInterfaceType getWriteInterfaceType(const std::string& path); IOInterfaceType getReadInterfaceType(const std::string& path); +void restartControlLoops(void); + /* * Given a configuration structure, fill out the information we use within the * PID loop. |