pid: zone split out builders
To enable unit-testing, split the builders into their own
object files.
Tested: Ran on quanta-q71l board and it behaved as expected.
Change-Id: I92168ec9ae4946d12328e9c0b94a36bb89d0f718
Signed-off-by: Patrick Venture <venture@google.com>
diff --git a/pid/builder.cpp b/pid/builder.cpp
new file mode 100644
index 0000000..45838e7
--- /dev/null
+++ b/pid/builder.cpp
@@ -0,0 +1,133 @@
+/**
+ * Copyright 2017 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "pid/builder.hpp"
+
+#include <iostream>
+#include <memory>
+#include <sdbusplus/bus.hpp>
+#include <unordered_map>
+
+#include "conf.hpp"
+#include "pid/fancontroller.hpp"
+#include "pid/thermalcontroller.hpp"
+
+static constexpr bool deferSignals = true;
+static constexpr auto objectPath = "/xyz/openbmc_project/settings/fanctrl/zone";
+
+static std::string GetControlPath(int64_t zone)
+{
+ return std::string(objectPath) + std::to_string(zone);
+}
+
+std::unordered_map<int64_t, std::unique_ptr<PIDZone>> BuildZones(
+ std::map<int64_t, PIDConf>& zonePids,
+ std::map<int64_t, struct zone>& zoneConfigs,
+ SensorManager& mgr,
+ sdbusplus::bus::bus& modeControlBus)
+{
+ std::unordered_map<int64_t, std::unique_ptr<PIDZone>> zones;
+
+ for (auto& zi : zonePids)
+ {
+ auto zoneId = static_cast<int64_t>(zi.first);
+ /* The above shouldn't be necessary but is, and I am having trouble
+ * locating my notes on why. If I recall correctly it was casting it
+ * down to a byte in at least some cases causing weird behaviors.
+ */
+
+ auto zoneConf = zoneConfigs.find(zoneId);
+ if (zoneConf == zoneConfigs.end())
+ {
+ /* The Zone doesn't have a configuration, bail. */
+ static constexpr auto err =
+ "Bailing during load, missing Zone Configuration";
+ std::cerr << err << std::endl;
+ throw std::runtime_error(err);
+ }
+
+ PIDConf& PIDConfig = zi.second;
+
+ auto zone = std::make_unique<PIDZone>(
+ zoneId,
+ zoneConf->second.minthermalrpm,
+ zoneConf->second.failsafepercent,
+ mgr,
+ modeControlBus,
+ GetControlPath(zi.first).c_str(),
+ deferSignals);
+
+ std::cerr << "Zone Id: " << zone->getZoneId() << "\n";
+
+ // For each PID create a Controller and a Sensor.
+ for (auto& pit : PIDConfig)
+ {
+ std::vector<std::string> inputs;
+ std::string name = pit.first;
+ struct controller_info* info = &pit.second;
+
+ std::cerr << "PID name: " << name << "\n";
+
+ /*
+ * TODO(venture): Need to check if input is known to the
+ * SensorManager.
+ */
+ if (info->type == "fan")
+ {
+ for (auto i : info->inputs)
+ {
+ inputs.push_back(i);
+ zone->addFanInput(i);
+ }
+
+ auto pid = FanController::CreateFanPid(
+ zone.get(),
+ name,
+ inputs,
+ info->info);
+ zone->addFanPID(std::move(pid));
+ }
+ else if (info->type == "temp" || info->type == "margin")
+ {
+ for (auto i : info->inputs)
+ {
+ inputs.push_back(i);
+ zone->addThermalInput(i);
+ }
+
+ auto pid = ThermalController::CreateThermalPid(
+ zone.get(),
+ name,
+ inputs,
+ info->setpoint,
+ info->info);
+ zone->addThermalPID(std::move(pid));
+ }
+
+ std::cerr << "inputs: ";
+ for (auto& i : inputs)
+ {
+ std::cerr << i << ", ";
+ }
+ std::cerr << "\n";
+ }
+
+ zone->emit_object_added();
+ zones[zoneId] = std::move(zone);
+ }
+
+ return zones;
+}
diff --git a/pid/builder.hpp b/pid/builder.hpp
new file mode 100644
index 0000000..5c03bd1
--- /dev/null
+++ b/pid/builder.hpp
@@ -0,0 +1,14 @@
+#pragma once
+
+#include <memory>
+#include <sdbusplus/bus.hpp>
+#include <unordered_map>
+
+#include "pid/zone.hpp"
+#include "sensors/manager.hpp"
+
+std::unordered_map<int64_t, std::unique_ptr<PIDZone>> BuildZones(
+ std::map<int64_t, PIDConf>& zonePids,
+ std::map<int64_t, struct zone>& zoneConfigs,
+ SensorManager& mgr,
+ sdbusplus::bus::bus& modeControlBus);
diff --git a/pid/builderconfig.cpp b/pid/builderconfig.cpp
new file mode 100644
index 0000000..33301e2
--- /dev/null
+++ b/pid/builderconfig.cpp
@@ -0,0 +1,153 @@
+/**
+ * Copyright 2017 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "pid/builderconfig.hpp"
+
+#include <fstream>
+#include <iostream>
+#include <libconfig.h++>
+#include <memory>
+#include <sdbusplus/bus.hpp>
+#include <string>
+#include <unordered_map>
+
+#include "conf.hpp"
+#include "pid/builder.hpp"
+
+std::unordered_map<int64_t, std::unique_ptr<PIDZone>> BuildZonesFromConfig(
+ const std::string& path,
+ SensorManager& mgr,
+ sdbusplus::bus::bus& modeControlBus)
+{
+ using namespace libconfig;
+ // zone -> pids
+ std::map<int64_t, PIDConf> pidConfig;
+ // zone -> configs
+ std::map<int64_t, struct zone> zoneConfig;
+
+ std::cerr << "entered BuildZonesFromConfig\n";
+
+ Config cfg;
+
+ /* The load was modeled after the example source provided. */
+ try
+ {
+ cfg.readFile(path.c_str());
+ }
+ catch (const FileIOException& fioex)
+ {
+ std::cerr << "I/O error while reading file: " << fioex.what()
+ << std::endl;
+ throw;
+ }
+ catch (const ParseException& pex)
+ {
+ std::cerr << "Parse error at " << pex.getFile() << ":" << pex.getLine()
+ << " - " << pex.getError() << std::endl;
+ throw;
+ }
+
+ try
+ {
+ const Setting& root = cfg.getRoot();
+ const Setting& zones = root["zones"];
+ int count = zones.getLength();
+
+ /* For each zone. */
+ for (int i = 0; i < count; ++i)
+ {
+ const Setting& zoneSettings = zones[i];
+
+ int id;
+ PIDConf thisZone;
+ struct zone thisZoneConfig;
+
+ zoneSettings.lookupValue("id", id);
+
+ thisZoneConfig.minthermalrpm =
+ zoneSettings.lookup("minthermalrpm");
+ thisZoneConfig.failsafepercent =
+ zoneSettings.lookup("failsafepercent");
+
+ const Setting& pids = zoneSettings["pids"];
+ int pidCount = pids.getLength();
+
+ for (int j = 0; j < pidCount; ++j)
+ {
+ const Setting& pid = pids[j];
+
+ std::string name;
+ controller_info info;
+
+ /*
+ * Mysteriously if you use lookupValue on these, and the type
+ * is float. It won't work right.
+ *
+ * If the configuration file value doesn't look explicitly like
+ * a float it won't let you assign it to one.
+ */
+ name = pid.lookup("name").c_str();
+ info.type = pid.lookup("type").c_str();
+ /* set-point is only required to be set for thermal. */
+ /* TODO(venture): Verify this works optionally here. */
+ info.setpoint = pid.lookup("set-point");
+ info.info.ts = pid.lookup("pid.sampleperiod");
+ info.info.p_c = pid.lookup("pid.p_coefficient");
+ info.info.i_c = pid.lookup("pid.i_coefficient");
+ info.info.ff_off = pid.lookup("pid.ff_off_coefficient");
+ info.info.ff_gain = pid.lookup("pid.ff_gain_coefficient");
+ info.info.i_lim.min = pid.lookup("pid.i_limit.min");
+ info.info.i_lim.max = pid.lookup("pid.i_limit.max");
+ info.info.out_lim.min = pid.lookup("pid.out_limit.min");
+ info.info.out_lim.max = pid.lookup("pid.out_limit.max");
+ info.info.slew_neg = pid.lookup("pid.slew_neg");
+ info.info.slew_pos = pid.lookup("pid.slew_pos");
+
+ std::cerr << "out_lim.min: " << info.info.out_lim.min << "\n";
+ std::cerr << "out_lim.max: " << info.info.out_lim.max << "\n";
+
+ const Setting& inputs = pid["inputs"];
+ int icount = inputs.getLength();
+
+ for (int z = 0; z < icount; ++z)
+ {
+ std::string v;
+ v = pid["inputs"][z].c_str();
+ info.inputs.push_back(v);
+ }
+
+ thisZone[name] = info;
+ }
+
+ pidConfig[static_cast<int64_t>(id)] = thisZone;
+ zoneConfig[static_cast<int64_t>(id)] = thisZoneConfig;
+ }
+ }
+ catch (const SettingTypeException &setex)
+ {
+ std::cerr << "Setting '" << setex.getPath() << "' type exception!"
+ << std::endl;
+ throw;
+ }
+ catch (const SettingNotFoundException& snex)
+ {
+ std::cerr << "Setting '" << snex.getPath() << "' not found!"
+ << std::endl;
+ throw;
+ }
+
+ return BuildZones(pidConfig, zoneConfig, mgr, modeControlBus);
+}
diff --git a/pid/builderconfig.hpp b/pid/builderconfig.hpp
new file mode 100644
index 0000000..ad83932
--- /dev/null
+++ b/pid/builderconfig.hpp
@@ -0,0 +1,14 @@
+#pragma once
+
+#include <memory>
+#include <sdbusplus/bus.hpp>
+#include <string>
+#include <unordered_map>
+
+#include "pid/zone.hpp"
+#include "sensors/manager.hpp"
+
+std::unordered_map<int64_t, std::unique_ptr<PIDZone>> BuildZonesFromConfig(
+ const std::string& path,
+ SensorManager& mgr,
+ sdbusplus::bus::bus& modeControlBus);
diff --git a/pid/controller.hpp b/pid/controller.hpp
index 0b1de3c..d02f9cc 100644
--- a/pid/controller.hpp
+++ b/pid/controller.hpp
@@ -15,7 +15,7 @@
class PIDController
{
public:
- PIDController(const std::string& id, std::shared_ptr<PIDZone> owner)
+ PIDController(const std::string& id, PIDZone* owner)
: _owner(owner),
_id(id)
{ }
@@ -47,7 +47,7 @@
}
protected:
- std::shared_ptr<PIDZone> _owner;
+ PIDZone* _owner;
private:
// parameters
diff --git a/pid/fancontroller.cpp b/pid/fancontroller.cpp
index b0b300f..340c6d0 100644
--- a/pid/fancontroller.cpp
+++ b/pid/fancontroller.cpp
@@ -22,7 +22,7 @@
#include "zone.hpp"
std::unique_ptr<PIDController> FanController::CreateFanPid(
- std::shared_ptr<PIDZone> owner,
+ PIDZone* owner,
const std::string& id,
std::vector<std::string>& inputs,
ec::pidinfo initial)
diff --git a/pid/fancontroller.hpp b/pid/fancontroller.hpp
index 0816991..b627915 100644
--- a/pid/fancontroller.hpp
+++ b/pid/fancontroller.hpp
@@ -18,14 +18,14 @@
{
public:
static std::unique_ptr<PIDController> CreateFanPid(
- std::shared_ptr<PIDZone> owner,
+ PIDZone* owner,
const std::string& id,
std::vector<std::string>& inputs,
ec::pidinfo initial);
FanController(const std::string& id,
std::vector<std::string>& inputs,
- std::shared_ptr<PIDZone> owner)
+ PIDZone* owner)
: PIDController(id, owner),
_inputs(inputs),
_direction(FanSpeedDirection::NEUTRAL)
diff --git a/pid/pidthread.cpp b/pid/pidthread.cpp
index 2231390..d387ac9 100644
--- a/pid/pidthread.cpp
+++ b/pid/pidthread.cpp
@@ -26,7 +26,7 @@
#include "sensors/sensor.hpp"
-static void ProcessThermals(std::shared_ptr<PIDZone> zone)
+static void ProcessThermals(PIDZone* zone)
{
// Get the latest margins.
zone->updateSensors();
@@ -39,7 +39,7 @@
}
-void PIDControlThread(std::shared_ptr<PIDZone> zone)
+void PIDControlThread(PIDZone* zone)
{
int ms100cnt = 0;
/*
diff --git a/pid/pidthread.hpp b/pid/pidthread.hpp
index 670d558..37c416a 100644
--- a/pid/pidthread.hpp
+++ b/pid/pidthread.hpp
@@ -3,4 +3,4 @@
#include "pid/zone.hpp"
/* Given a zone, run through the loops. */
-void PIDControlThread(std::shared_ptr<PIDZone> zone);
+void PIDControlThread(PIDZone* zone);
diff --git a/pid/thermalcontroller.cpp b/pid/thermalcontroller.cpp
index c44503f..61e5ca9 100644
--- a/pid/thermalcontroller.cpp
+++ b/pid/thermalcontroller.cpp
@@ -20,7 +20,7 @@
std::unique_ptr<PIDController> ThermalController::CreateThermalPid(
- std::shared_ptr<PIDZone> owner,
+ PIDZone* owner,
const std::string& id,
std::vector<std::string>& inputs,
float setpoint,
diff --git a/pid/thermalcontroller.hpp b/pid/thermalcontroller.hpp
index 32616ac..dbfcfc9 100644
--- a/pid/thermalcontroller.hpp
+++ b/pid/thermalcontroller.hpp
@@ -16,7 +16,7 @@
{
public:
static std::unique_ptr<PIDController> CreateThermalPid(
- std::shared_ptr<PIDZone> owner,
+ PIDZone* owner,
const std::string& id,
std::vector<std::string>& inputs,
float setpoint,
@@ -24,7 +24,7 @@
ThermalController(const std::string& id,
std::vector<std::string>& inputs,
- std::shared_ptr<PIDZone> owner)
+ PIDZone* owner)
: PIDController(id, owner),
_inputs(inputs)
{ }
diff --git a/pid/zone.cpp b/pid/zone.cpp
index ed8cd0f..26bff6a 100644
--- a/pid/zone.cpp
+++ b/pid/zone.cpp
@@ -36,9 +36,6 @@
using tstamp = std::chrono::high_resolution_clock::time_point;
using namespace std::literals::chrono_literals;
-static constexpr bool deferSignals = true;
-static constexpr auto objectPath = "/xyz/openbmc_project/settings/fanctrl/zone";
-
float PIDZone::getMaxRPMRequest(void) const
{
return _maximumRPMSetPt;
@@ -335,233 +332,3 @@
{
return getFailSafeMode();
}
-
-static std::string GetControlPath(int64_t zone)
-{
- return std::string(objectPath) + std::to_string(zone);
-}
-
-std::map<int64_t, std::shared_ptr<PIDZone>> BuildZones(
- std::map<int64_t, PIDConf>& ZonePids,
- std::map<int64_t, struct zone>& ZoneConfigs,
- SensorManager& mgr,
- sdbusplus::bus::bus& ModeControlBus)
-{
- std::map<int64_t, std::shared_ptr<PIDZone>> zones;
-
- for (auto& zi : ZonePids)
- {
- auto zoneId = static_cast<int64_t>(zi.first);
- /* The above shouldn't be necessary but is, and I am having trouble
- * locating my notes on why. If I recall correctly it was casting it
- * down to a byte in at least some cases causing weird behaviors.
- */
-
- auto zoneConf = ZoneConfigs.find(zoneId);
- if (zoneConf == ZoneConfigs.end())
- {
- /* The Zone doesn't have a configuration, bail. */
- static constexpr auto err =
- "Bailing during load, missing Zone Configuration";
- std::cerr << err << std::endl;
- throw std::runtime_error(err);
- }
-
- PIDConf& PIDConfig = zi.second;
-
- auto zone = std::make_shared<PIDZone>(
- zoneId,
- zoneConf->second.minthermalrpm,
- zoneConf->second.failsafepercent,
- mgr,
- ModeControlBus,
- GetControlPath(zi.first).c_str(),
- deferSignals);
-
- zones[zoneId] = zone;
-
- std::cerr << "Zone Id: " << zone->getZoneId() << "\n";
-
- // For each PID create a Controller and a Sensor.
- PIDConf::iterator pit = PIDConfig.begin();
- while (pit != PIDConfig.end())
- {
- std::vector<std::string> inputs;
- std::string name = pit->first;
- struct controller_info* info = &pit->second;
-
- std::cerr << "PID name: " << name << "\n";
-
- /*
- * TODO(venture): Need to check if input is known to the
- * SensorManager.
- */
- if (info->type == "fan")
- {
- for (auto i : info->inputs)
- {
- inputs.push_back(i);
- zone->addFanInput(i);
- }
-
- auto pid = FanController::CreateFanPid(
- zone,
- name,
- inputs,
- info->info);
- zone->addFanPID(std::move(pid));
- }
- else if (info->type == "temp" || info->type == "margin")
- {
- for (auto i : info->inputs)
- {
- inputs.push_back(i);
- zone->addThermalInput(i);
- }
-
- auto pid = ThermalController::CreateThermalPid(
- zone,
- name,
- inputs,
- info->setpoint,
- info->info);
- zone->addThermalPID(std::move(pid));
- }
-
- std::cerr << "inputs: ";
- for (auto& i : inputs)
- {
- std::cerr << i << ", ";
- }
- std::cerr << "\n";
-
- ++pit;
- }
-
- zone->emit_object_added();
- }
-
- return zones;
-}
-
-std::map<int64_t, std::shared_ptr<PIDZone>> BuildZonesFromConfig(
- std::string& path,
- SensorManager& mgr,
- sdbusplus::bus::bus& ModeControlBus)
-{
- using namespace libconfig;
- // zone -> pids
- std::map<int64_t, PIDConf> pidConfig;
- // zone -> configs
- std::map<int64_t, struct zone> zoneConfig;
-
- std::cerr << "entered BuildZonesFromConfig\n";
-
- Config cfg;
-
- /* The load was modeled after the example source provided. */
- try
- {
- cfg.readFile(path.c_str());
- }
- catch (const FileIOException& fioex)
- {
- std::cerr << "I/O error while reading file: " << fioex.what() << std::endl;
- throw;
- }
- catch (const ParseException& pex)
- {
- std::cerr << "Parse error at " << pex.getFile() << ":" << pex.getLine()
- << " - " << pex.getError() << std::endl;
- throw;
- }
-
- try
- {
- const Setting& root = cfg.getRoot();
- const Setting& zones = root["zones"];
- int count = zones.getLength();
-
- /* For each zone. */
- for (int i = 0; i < count; ++i)
- {
- const Setting& zoneSettings = zones[i];
-
- int id;
- PIDConf thisZone;
- struct zone thisZoneConfig;
-
- zoneSettings.lookupValue("id", id);
-
- thisZoneConfig.minthermalrpm =
- zoneSettings.lookup("minthermalrpm");
- thisZoneConfig.failsafepercent =
- zoneSettings.lookup("failsafepercent");
-
- const Setting& pids = zoneSettings["pids"];
- int pidCount = pids.getLength();
-
- for (int j = 0; j < pidCount; ++j)
- {
- const Setting& pid = pids[j];
-
- std::string name;
- controller_info info;
-
- /*
- * Mysteriously if you use lookupValue on these, and the type
- * is float. It won't work right.
- *
- * If the configuration file value doesn't look explicitly like
- * a float it won't let you assign it to one.
- */
- name = pid.lookup("name").c_str();
- info.type = pid.lookup("type").c_str();
- /* set-point is only required to be set for thermal. */
- /* TODO(venture): Verify this works optionally here. */
- info.setpoint = pid.lookup("set-point");
- info.info.ts = pid.lookup("pid.sampleperiod");
- info.info.p_c = pid.lookup("pid.p_coefficient");
- info.info.i_c = pid.lookup("pid.i_coefficient");
- info.info.ff_off = pid.lookup("pid.ff_off_coefficient");
- info.info.ff_gain = pid.lookup("pid.ff_gain_coefficient");
- info.info.i_lim.min = pid.lookup("pid.i_limit.min");
- info.info.i_lim.max = pid.lookup("pid.i_limit.max");
- info.info.out_lim.min = pid.lookup("pid.out_limit.min");
- info.info.out_lim.max = pid.lookup("pid.out_limit.max");
- info.info.slew_neg = pid.lookup("pid.slew_neg");
- info.info.slew_pos = pid.lookup("pid.slew_pos");
-
- std::cerr << "out_lim.min: " << info.info.out_lim.min << "\n";
- std::cerr << "out_lim.max: " << info.info.out_lim.max << "\n";
-
- const Setting& inputs = pid["inputs"];
- int icount = inputs.getLength();
-
- for (int z = 0; z < icount; ++z)
- {
- std::string v;
- v = pid["inputs"][z].c_str();
- info.inputs.push_back(v);
- }
-
- thisZone[name] = info;
- }
-
- pidConfig[static_cast<int64_t>(id)] = thisZone;
- zoneConfig[static_cast<int64_t>(id)] = thisZoneConfig;
- }
- }
- catch (const SettingTypeException &setex)
- {
- std::cerr << "Setting '" << setex.getPath() << "' type exception!" << std::endl;
- throw;
- }
- catch (const SettingNotFoundException& snex)
- {
- std::cerr << "Setting '" << snex.getPath() << "' not found!" << std::endl;
- throw;
- }
-
- return BuildZones(pidConfig, zoneConfig, mgr, ModeControlBus);
-}
diff --git a/pid/zone.hpp b/pid/zone.hpp
index 1cf517c..f6ee5e8 100644
--- a/pid/zone.hpp
+++ b/pid/zone.hpp
@@ -111,14 +111,3 @@
std::vector<std::unique_ptr<PIDController>> _fans;
std::vector<std::unique_ptr<PIDController>> _thermals;
};
-
-std::map<int64_t, std::shared_ptr<PIDZone>> BuildZones(
- std::map<int64_t, PIDConf>& ZonePids,
- std::map<int64_t, struct zone>& ZoneConfigs,
- SensorManager& mgmr,
- sdbusplus::bus::bus& ModeControlBus);
-
-std::map<int64_t, std::shared_ptr<PIDZone>> BuildZonesFromConfig(
- std::string& path,
- SensorManager& mgmr,
- sdbusplus::bus::bus& ModeControlBus);