Improve battor_agent interactive mode

The battor_agent StopTracing command, if no file name is specified,
printed the raw sampling data which can easily be 100,000 lines of
numbers. This is not helpful. This change improves this case,
especially when in interactive mode.

Now a BattorResults object is returned which contains a detailed
string representation of the results and enough information to allow
creating a summary. When in battor_agent's interactive mode the
summary is printed and the details are saved to a file (using a
generated file name if none is supplied). The summary result format
is for human consumption and is subject to change but currently it
looks like this:

StopTracing
Second  0 average power:  9.20 W
Second  1 average power:  8.65 W
Second  2 average power: 11.38 W
Average power over 3.14 s :  9.74 W
Summary of power-by-seconds:
Minimum:  8.65
Median:   9.20
Maximum: 11.38
Saving detailed results to c:\users\brucedawson\desktop\trace_data.txt
Done.

This lets the developer easily see the average power and the variability
of power, which is helpful for getting a sense of how consistent the
results are and therefore (in steady-state scenarios) how meaningful
they are. This format has been tested for a few weeks on a Battor
equipped SurfaceBook.

Bug: 751863
Change-Id: I57af162f183a4d65388c88c901573cd6720ae2b8
Reviewed-on: https://chromium-review.googlesource.com/609385
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Charlie Andrews <charliea@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500344}

content/browser/tracing/power_tracing_agent.cc

@@ -115,13 +115,14 @@ void PowerTracingAgent::StopAgentTracingOnIOThread(
   battor_agent_->StopTracing();
 }
 
-void PowerTracingAgent::OnStopTracingComplete(const std::string& trace,
-                                              battor::BattOrError error) {
+void PowerTracingAgent::OnStopTracingComplete(
+    const battor::BattOrResults& results,
+    battor::BattOrError error) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   scoped_refptr<base::RefCountedString> result(new base::RefCountedString());
   if (error == battor::BATTOR_ERROR_NONE)
-    result->data() = trace;
+    result->data() = results.ToString();
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,

content/browser/tracing/power_tracing_agent.h

@@ -43,7 +43,7 @@ class PowerTracingAgent : public base::trace_event::TracingAgent,
 
   // BattOrAgent::Listener implementation.
   void OnStartTracingComplete(battor::BattOrError error) override;
-  void OnStopTracingComplete(const std::string& trace,
+  void OnStopTracingComplete(const battor::BattOrResults& results,
                              battor::BattOrError error) override;
   void OnRecordClockSyncMarkerComplete(battor::BattOrError error) override;
   void OnGetFirmwareGitHashComplete(const std::string& version,

tools/battor_agent/battor_agent.cc

@@ -3,7 +3,9 @@
 // found in the LICENSE file.
 #include "tools/battor_agent/battor_agent.h"
 
+#include <algorithm>
 #include <iomanip>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -102,6 +104,19 @@ bool ParseSampleFrame(BattOrMessageType type,
 }
 }  // namespace
 
+BattOrResults::BattOrResults() {}
+
+BattOrResults::BattOrResults(std::string details,
+                             std::vector<float> power_samples_W,
+                             uint32_t sample_rate)
+    : details_(std::move(details)),
+      power_samples_W_(std::move(power_samples_W)),
+      sample_rate_(sample_rate) {}
+
+BattOrResults::BattOrResults(const BattOrResults&) = default;
+
+BattOrResults::~BattOrResults() {}
+
 BattOrAgent::BattOrAgent(
     const std::string& path,
     Listener* listener,
@@ -541,7 +556,7 @@ void BattOrAgent::CompleteCommand(BattOrError error) {
       base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE,
           base::Bind(&Listener::OnStopTracingComplete,
-                     base::Unretained(listener_), SamplesToString(), error));
+                     base::Unretained(listener_), SamplesToResults(), error));
       break;
     case Command::RECORD_CLOCK_SYNC_MARKER:
       base::ThreadTaskRunnerHandle::Get()->PostTask(
@@ -569,9 +584,9 @@ void BattOrAgent::CompleteCommand(BattOrError error) {
   num_command_attempts_ = 0;
 }
 
-std::string BattOrAgent::SamplesToString() {
+BattOrResults BattOrAgent::SamplesToResults() {
   if (calibration_frame_.empty() || samples_.empty() || !battor_eeprom_)
-    return "";
+    return BattOrResults();
 
   BattOrSampleConverter converter(*battor_eeprom_, calibration_frame_);
 
@@ -606,7 +621,13 @@ std::string BattOrAgent::SamplesToString() {
     trace_stream << std::endl;
   }
 
-  return trace_stream.str();
+  // Convert to a vector of power in watts.
+  std::vector<float> samples(samples_.size());
+  for (size_t i = 0; i < samples_.size(); i++)
+    samples[i] = converter.ToWatts(samples_[i]);
+
+  return BattOrResults(trace_stream.str(), samples,
+                       battor_eeprom_->sd_sample_rate);
 }
 
 void BattOrAgent::SetActionTimeout(uint16_t timeout_seconds) {

tools/battor_agent/battor_agent.h

@@ -6,6 +6,7 @@
 #define TOOLS_BATTOR_AGENT_BATTOR_AGENT_H_
 
 #include <map>
+#include <vector>
 
 #include "base/cancelable_callback.h"
 #include "base/macros.h"
@@ -18,6 +19,29 @@
 
 namespace battor {
 
+// A BattOrResults object contains the results of BattOr tracing, including a
+// summary and sample data in watts.
+class BattOrResults {
+ public:
+  BattOrResults();
+  BattOrResults(std::string details,
+                std::vector<float> power_samples_W,
+                uint32_t sample_rate);
+  BattOrResults(const BattOrResults&);
+  ~BattOrResults();
+
+  // Get a detailed textual representation of the data recorded.
+  const std::string& ToString() const { return details_; }
+  // Returns a vector of power samples (in watts).
+  const std::vector<float>& GetPowerSamples() const { return power_samples_W_; }
+  uint32_t GetSampleRate() const { return sample_rate_; }
+
+ private:
+  std::string details_;
+  std::vector<float> power_samples_W_;
+  uint32_t sample_rate_ = 0;
+};
+
 // A BattOrAgent is a class used to asynchronously communicate with a BattOr for
 // the purpose of collecting power samples. A BattOr is an external USB device
 // that's capable of recording accurate, high-frequency (2000Hz) power samples.
@@ -41,7 +65,7 @@ class BattOrAgent : public BattOrConnection::Listener,
   class Listener {
    public:
     virtual void OnStartTracingComplete(BattOrError error) = 0;
-    virtual void OnStopTracingComplete(const std::string& trace,
+    virtual void OnStopTracingComplete(const BattOrResults& trace,
                                        BattOrError error) = 0;
     virtual void OnRecordClockSyncMarkerComplete(BattOrError error) = 0;
     virtual void OnGetFirmwareGitHashComplete(const std::string& version,
@@ -141,7 +165,7 @@ class BattOrAgent : public BattOrConnection::Listener,
   void CompleteCommand(BattOrError error);
 
   // Returns a formatted version of samples_ with timestamps and real units.
-  std::string SamplesToString();
+  BattOrResults SamplesToResults();
 
   // Sets and restarts the action timeout timer.
   void SetActionTimeout(uint16_t timeout_seconds);

tools/battor_agent/battor_agent_bin.cc

@@ -34,15 +34,18 @@
 #include <stdint.h>
 
 #include <fstream>
+#include <iomanip>
 #include <iostream>
 
 #include "base/at_exit.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
+#include "base/files/file_path.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
+#include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_tokenizer.h"
@@ -289,25 +292,97 @@ class BattOrAgentBin : public BattOrAgent::Listener {
         base::Bind(&BattOrAgent::StopTracing, base::Unretained(agent_.get())));
   }
 
-  void OnStopTracingComplete(const std::string& trace,
+  std::string BattOrResultsToSummary(const BattOrResults& results) {
+    const uint32_t samples_per_second = results.GetSampleRate();
+
+    // Print a summary of a BattOr trace. These summaries are intended for human
+    // consumption and are subject to change at any moment. The summary is
+    // printed when using interactive mode.
+    std::stringstream trace_summary;
+    // Display floating-point numbers without exponents, in a five-character
+    // field, with two digits of precision. ie;
+    // 12.39
+    //  8.40
+    trace_summary << std::fixed << std::setw(5) << std::setprecision(2);
+
+    // Scan through the sample data to summarize it. Report on average power and
+    // second-by-second power including min-second, median-second, and
+    // max-second.
+    double total_power = 0.0;
+    int num_seconds = 0;
+    std::vector<double> power_by_seconds;
+    const std::vector<float>& samples = results.GetPowerSamples();
+    for (size_t i = 0; i < samples.size(); i += samples_per_second) {
+      size_t loop_count = samples.size() - i;
+      if (loop_count > samples_per_second)
+        loop_count = samples_per_second;
+
+      double second_power = 0.0;
+      for (size_t j = i; j < i + loop_count; ++j) {
+        total_power += samples[i];
+        second_power += samples[i];
+      }
+
+      // Print/store results for full seconds.
+      if (loop_count == samples_per_second) {
+        // Calculate power for one second in watts.
+        second_power /= samples_per_second;
+        trace_summary << "Second " << std::setw(2) << num_seconds
+                      << " average power: " << std::setw(5) << second_power
+                      << " W" << std::endl;
+        ++num_seconds;
+        power_by_seconds.push_back(second_power);
+      }
+    }
+    // Calculate average power in watts.
+    const double average_power_W = total_power / samples.size();
+    const double duration_sec =
+        static_cast<double>(samples.size()) / samples_per_second;
+    trace_summary << "Average power over " << duration_sec
+                  << " s : " << average_power_W << " W" << std::endl;
+    std::sort(power_by_seconds.begin(), power_by_seconds.end());
+    if (power_by_seconds.size() >= 3) {
+      trace_summary << "Summary of power-by-seconds:" << std::endl
+                    << "Minimum: " << power_by_seconds[0] << std::endl
+                    << "Median:  "
+                    << power_by_seconds[power_by_seconds.size() / 2]
+                    << std::endl
+                    << "Maximum: "
+                    << power_by_seconds[power_by_seconds.size() - 1]
+                    << std::endl;
+    } else {
+      trace_summary << "Too short a trace to generate per-second summary.";
+    }
+
+    return trace_summary.str();
+  }
+
+  void OnStopTracingComplete(const BattOrResults& results,
                              BattOrError error) override {
     if (error == BATTOR_ERROR_NONE) {
+      std::string output_file = trace_output_file_;
       if (trace_output_file_.empty()) {
-        if (interactive_) {
-          // Printing of summary statistics will happen here.
-          std::cout << trace.size() << " bytes of output collected." << endl;
-        } else {
-          std::cout << trace;
-        }
-      } else {
-        std::ofstream trace_stream(trace_output_file_);
-        if (!trace_stream.is_open()) {
-          std::cout << "Tracing output file could not be opened." << endl;
-          exit(1);
-        }
-        trace_stream << trace;
-        trace_stream.close();
+        // Save the detailed results in case they are needed.
+        base::FilePath default_path;
+        PathService::Get(base::DIR_USER_DESKTOP, &default_path);
+        default_path = default_path.Append(FILE_PATH_LITERAL("trace_data.txt"));
+        output_file = default_path.AsUTF8Unsafe().c_str();
+        std::cout << "Saving detailed results to " << output_file << std::endl;
+      }
+
+      if (interactive_) {
+        // Print a summary of the trace.
+        std::cout << BattOrResultsToSummary(results) << endl;
+      }
+
+      std::ofstream trace_stream(output_file);
+      if (!trace_stream.is_open()) {
+        std::cout << "Tracing output file \"" << output_file
+                  << "\" could not be opened." << endl;
+        exit(1);
       }
+      trace_stream << results.ToString();
+      trace_stream.close();
       std::cout << "Done." << endl;
     } else {
       HandleError(error);

tools/battor_agent/battor_agent_unittest.cc

@@ -107,11 +107,11 @@ class BattOrAgentTest : public testing::Test, public BattOrAgent::Listener {
     command_error_ = error;
   }
 
-  void OnStopTracingComplete(const std::string& trace,
+  void OnStopTracingComplete(const BattOrResults& results,
                              BattOrError error) override {
     is_command_complete_ = true;
     command_error_ = error;
-    trace_ = trace;
+    trace_ = results.ToString();
   }
 
   void OnRecordClockSyncMarkerComplete(BattOrError error) override {

tools/battor_agent/battor_sample_converter.cc

@@ -91,6 +91,12 @@ BattOrSample BattOrSampleConverter::ToSample(const RawBattOrSample& sample,
   return BattOrSample{time_ms, voltage, current};
 }
 
+float BattOrSampleConverter::ToWatts(const RawBattOrSample& raw_sample) const {
+  BattOrSample sample = ToSample(raw_sample, 0);
+
+  return sample.current_mA * sample.voltage_mV * 1e-6f;
+}
+
 BattOrSample BattOrSampleConverter::MinSample() const {
   // Create a minimum raw sample.
   RawBattOrSample sample_raw = {kAnalogDigitalConverterMinValue,

tools/battor_agent/battor_sample_converter.h

@@ -33,6 +33,9 @@ class BattOrSampleConverter {
   BattOrSample ToSample(const RawBattOrSample& sample,
                         size_t sample_number) const;
 
+  // Converts a raw sample to watts.
+  float ToWatts(const RawBattOrSample& sample) const;
+
   // Returns the lowest magnitude sample that the BattOr can collect.
   BattOrSample MinSample() const;