add tuning enable variable
Add a variable that when set, enables tuning logging. This variable is
set to false.
Tuning can be enabled via "-t" "--tuning" on the command line.
With a parameter is the path where to write the logging information.
Change-Id: I6eb8035d56cc3301face21e9375c02fc9fcc5b31
Signed-off-by: Patrick Venture <venture@google.com>
diff --git a/Makefile.am b/Makefile.am
index 9a31124..3a1b52b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -60,6 +60,7 @@
pid/zone.cpp \
pid/util.cpp \
pid/pidthread.cpp \
+ pid/tuning.cpp \
threads/busthread.cpp \
build/buildjson.cpp \
experiments/drive.cpp
diff --git a/README.md b/README.md
index e029d71..17465b2 100644
--- a/README.md
+++ b/README.md
@@ -254,14 +254,8 @@
### Enabling Logging
-By default, swampd isn't compiled to log information. To compile it for tuning,
-you'll need to add:
-
-```
-EXTRA_OEMAKE_append_YOUR_MACHINE = " CXXFLAGS='${CXXFLAGS} -D__TUNING_LOGGING__'"
-```
-
-To the recipe.
+By default, swampd won't log tuning information. To enable this pass "-t" on
+the command line with a pareter of the path to write the log file.
## Project Information
diff --git a/main.cpp b/main.cpp
index d896300..cec3789 100644
--- a/main.cpp
+++ b/main.cpp
@@ -22,6 +22,7 @@
#include "pid/builder.hpp"
#include "pid/buildjson.hpp"
#include "pid/pidthread.hpp"
+#include "pid/tuning.hpp"
#include "pid/zone.hpp"
#include "sensors/builder.hpp"
#include "sensors/buildjson.hpp"
@@ -65,12 +66,13 @@
// clang-format off
static struct option long_options[] = {
{"conf", required_argument, 0, 'c'},
+ {"tuning", required_argument, 0, 't'},
{0, 0, 0, 0}
};
// clang-format on
int option_index = 0;
- int c = getopt_long(argc, argv, "c:", long_options, &option_index);
+ int c = getopt_long(argc, argv, "t:c:", long_options, &option_index);
if (c == -1)
{
@@ -82,6 +84,10 @@
case 'c':
configPath = std::string{optarg};
break;
+ case 't':
+ tuningLoggingEnabled = true;
+ tuningLoggingPath = std::string{optarg};
+ break;
default:
/* skip garbage. */
continue;
diff --git a/pid/fancontroller.cpp b/pid/fancontroller.cpp
index 4a61def..4c1b56b 100644
--- a/pid/fancontroller.cpp
+++ b/pid/fancontroller.cpp
@@ -16,6 +16,7 @@
#include "fancontroller.hpp"
+#include "tuning.hpp"
#include "util.hpp"
#include "zone.hpp"
@@ -115,16 +116,17 @@
double percent = value;
/* If doing tuning logging, don't go into failsafe mode. */
-#ifndef __TUNING_LOGGING__
- if (_owner->getFailSafeMode())
+ if (!tuningLoggingEnabled)
{
- /* In case it's being set to 100% */
- if (percent < _owner->getFailSafePercent())
+ if (_owner->getFailSafeMode())
{
- percent = _owner->getFailSafePercent();
+ /* In case it's being set to 100% */
+ if (percent < _owner->getFailSafePercent())
+ {
+ percent = _owner->getFailSafePercent();
+ }
}
}
-#endif
// value and kFanFailSafeDutyCycle are 10 for 10% so let's fix that.
percent /= 100;
diff --git a/pid/pidthread.cpp b/pid/pidthread.cpp
index 08fb513..f8d7fd9 100644
--- a/pid/pidthread.cpp
+++ b/pid/pidthread.cpp
@@ -17,6 +17,7 @@
#include "pidthread.hpp"
#include "pid/pidcontroller.hpp"
+#include "pid/tuning.hpp"
#include "sensors/sensor.hpp"
#include <chrono>
@@ -63,9 +64,11 @@
* TODO(venture): If the fan value is 0 should that loop just be skipped?
* Right now, a 0 value is ignored in FanController::inputProc()
*/
-#ifdef __TUNING_LOGGING__
- zone->initializeLog();
-#endif
+ if (tuningLoggingEnabled)
+ {
+ zone->initializeLog();
+ }
+
zone->initializeCache();
processThermals(zone);
@@ -93,10 +96,11 @@
// Run the fan PIDs every iteration.
zone->processFans();
-#ifdef __TUNING_LOGGING__
- zone->getLogHandle() << "," << zone->getFailSafeMode();
- zone->getLogHandle() << std::endl;
-#endif
+ if (tuningLoggingEnabled)
+ {
+ zone->getLogHandle() << "," << zone->getFailSafeMode();
+ zone->getLogHandle() << std::endl;
+ }
ms100cnt += 1;
}
diff --git a/pid/tuning.cpp b/pid/tuning.cpp
new file mode 100644
index 0000000..c3f21ed
--- /dev/null
+++ b/pid/tuning.cpp
@@ -0,0 +1,20 @@
+/**
+ * Copyright 2019 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 <string>
+
+bool tuningLoggingEnabled = false;
+std::string tuningLoggingPath;
diff --git a/pid/tuning.hpp b/pid/tuning.hpp
new file mode 100644
index 0000000..511c797
--- /dev/null
+++ b/pid/tuning.hpp
@@ -0,0 +1,9 @@
+#pragma once
+
+#include <string>
+
+/** Boolean variable controlling whether tuning logging output is enabled
+ * during this run.
+ */
+extern bool tuningLoggingEnabled;
+extern std::string tuningLoggingPath;
diff --git a/pid/zone.cpp b/pid/zone.cpp
index 7433d30..928aef5 100644
--- a/pid/zone.cpp
+++ b/pid/zone.cpp
@@ -23,6 +23,7 @@
#include "pid/fancontroller.hpp"
#include "pid/stepwisecontroller.hpp"
#include "pid/thermalcontroller.hpp"
+#include "pid/tuning.hpp"
#include <algorithm>
#include <chrono>
@@ -138,38 +139,38 @@
*/
max = std::max(getMinThermalRPMSetpoint(), max);
-#ifdef __TUNING_LOGGING__
- /*
- * We received no setpoints from thermal sensors.
- * This is a case experienced during tuning where they only specify
- * fan sensors and one large fan PID for all the fans.
- */
- static constexpr auto setpointpath = "/etc/thermal.d/setpoint";
- try
+ if (tuningLoggingEnabled)
{
- std::ifstream ifs;
- ifs.open(setpointpath);
- if (ifs.good())
+ /*
+ * We received no setpoints from thermal sensors.
+ * This is a case experienced during tuning where they only specify
+ * fan sensors and one large fan PID for all the fans.
+ */
+ static constexpr auto setpointpath = "/etc/thermal.d/setpoint";
+ try
{
- int value;
- ifs >> value;
+ std::ifstream ifs;
+ ifs.open(setpointpath);
+ if (ifs.good())
+ {
+ int value;
+ ifs >> value;
- /* expecting RPM setpoint, not pwm% */
- max = static_cast<double>(value);
+ /* expecting RPM setpoint, not pwm% */
+ max = static_cast<double>(value);
+ }
+ }
+ catch (const std::exception& e)
+ {
+ /* This exception is uninteresting. */
+ std::cerr << "Unable to read from '" << setpointpath << "'\n";
}
}
- catch (const std::exception& e)
- {
- /* This exception is uninteresting. */
- std::cerr << "Unable to read from '" << setpointpath << "'\n";
- }
-#endif
_maximumRPMSetPt = max;
return;
}
-#ifdef __TUNING_LOGGING__
void PIDZone::initializeLog(void)
{
/* Print header for log file:
@@ -196,7 +197,6 @@
{
return _log;
}
-#endif
/*
* TODO(venture) This is effectively updating the cache and should check if the
@@ -217,13 +217,14 @@
* is disabled? I think it's a waste to try and log things even if the
* data is just being dropped though.
*/
-#ifdef __TUNING_LOGGING__
- tstamp now = std::chrono::high_resolution_clock::now();
- _log << std::chrono::duration_cast<std::chrono::milliseconds>(
- now.time_since_epoch())
- .count();
- _log << "," << _maximumRPMSetPt;
-#endif
+ if (tuningLoggingEnabled)
+ {
+ tstamp now = std::chrono::high_resolution_clock::now();
+ _log << std::chrono::duration_cast<std::chrono::milliseconds>(
+ now.time_since_epoch())
+ .count();
+ _log << "," << _maximumRPMSetPt;
+ }
for (const auto& f : _fanInputs)
{
@@ -236,17 +237,19 @@
* However, these are the fans, so if I'm not getting updated values
* for them... what should I do?
*/
-#ifdef __TUNING_LOGGING__
- _log << "," << r.value;
-#endif
+ if (tuningLoggingEnabled)
+ {
+ _log << "," << r.value;
+ }
}
-#ifdef __TUNING_LOGGING__
- for (const auto& t : _thermalInputs)
+ if (tuningLoggingEnabled)
{
- _log << "," << _cachedValuesByName[t];
+ for (const auto& t : _thermalInputs)
+ {
+ _log << "," << _cachedValuesByName[t];
+ }
}
-#endif
return;
}
diff --git a/pid/zone.hpp b/pid/zone.hpp
index 94082e9..a458011 100644
--- a/pid/zone.hpp
+++ b/pid/zone.hpp
@@ -5,6 +5,7 @@
#include "pidcontroller.hpp"
#include "sensors/manager.hpp"
#include "sensors/sensor.hpp"
+#include "tuning.hpp"
#include <fstream>
#include <map>
@@ -51,9 +52,10 @@
_minThermalOutputSetPt(minThermalOutput),
_failSafePercent(failSafePercent), _mgr(mgr)
{
-#ifdef __TUNING_LOGGING__
- _log.open("/tmp/swampd.log");
-#endif
+ if (tuningLoggingEnabled && !tuningLoggingPath.empty())
+ {
+ _log.open(tuningLoggingPath);
+ }
}
double getMaxRPMRequest(void) const override;
@@ -87,10 +89,8 @@
void addFanInput(const std::string& fan);
void addThermalInput(const std::string& therm);
-#ifdef __TUNING_LOGGING__
void initializeLog(void);
std::ofstream& getLogHandle(void);
-#endif
/* Method for setting the manual mode over dbus */
bool manual(bool value) override;
@@ -98,9 +98,7 @@
bool failSafe() const override;
private:
-#ifdef __TUNING_LOGGING__
std::ofstream _log;
-#endif
const int64_t _zoneId;
double _maximumRPMSetPt = 0;
diff --git a/test/Makefile.am b/test/Makefile.am
index 04cf2df..07c96ce 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -37,7 +37,7 @@
pid_zone_unittest_SOURCES = pid_zone_unittest.cpp
pid_zone_unittest_LDADD = $(top_builddir)/pid/ec/pid.o \
$(top_builddir)/sensors/manager.o $(top_builddir)/pid/pidcontroller.o \
- $(top_builddir)/pid/zone.o
+ $(top_builddir)/pid/zone.o $(top_builddir)/pid/tuning.o
pid_thermalcontroller_unittest_SOURCES = pid_thermalcontroller_unittest.cpp
pid_thermalcontroller_unittest_LDADD = $(top_builddir)/pid/ec/pid.o \
@@ -51,7 +51,7 @@
pid_fancontroller_unittest_SOURCES = pid_fancontroller_unittest.cpp
pid_fancontroller_unittest_LDADD = $(top_builddir)/pid/ec/pid.o \
$(top_builddir)/pid/util.o $(top_builddir)/pid/pidcontroller.o \
- $(top_builddir)/pid/fancontroller.o
+ $(top_builddir)/pid/fancontroller.o $(top_builddir)/pid/tuning.o
dbus_passive_unittest_SOURCES = dbus_passive_unittest.cpp
dbus_passive_unittest_LDADD = $(top_builddir)/dbus/util.o \