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);