[Battor] Add ability to dump trace log to file.

tools/battor_agent/battor_agent_bin.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This file provides a thin binary wrapper around the BattOr Agent
 // library. This binary wrapper provides a means for non-C++ tracing
 // controllers, such as Telemetry and Android Systrace, to issue high-level
 // tracing commands to the BattOr through an interactive shell.
 //
 // Example usage of how an external trace controller might use this binary:
@@ skipping 15 matching lines @@
 //    sync end timestamp and sends the subprocess the StopTracing message via
 //    STDIN
 // 9) PowerTracingAgent continues to read trace output lines from STDOUT until
 //    the binary exits with an exit code of 1 (indicating failure) or the
 //    'Done.' line is printed to STDOUT, signaling the last line of the trace
 // 10) PowerTracingAgent returns the battery trace to the Telemetry trace
 //     controller
 
 #include <stdint.h>
 
+#include <fstream>
 #include <iostream>
 
 #include "base/at_exit.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/strings/string_tokenizer.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 17 matching lines @@
     "Start the battor_agent shell with:\n"
     "\n"
     "  battor_agent <switches>\n"
     "\n"
     "Switches: \n"
     "  --battor-path=<path> Uses the specified BattOr path.\n"
     "\n"
     "Once in the shell, you can issue the following commands:\n"
     "\n"
     "  StartTracing\n"
-    "  StopTracing\n"
+    "  StopTracing <optional file path>\n"
     "  SupportsExplicitClockSync\n"
     "  RecordClockSyncMarker <marker>\n"
     "  Exit\n"
     "  Help\n"
     "\n";
 
 void PrintSupportsExplicitClockSync() {
   std::cout << BattOrAgent::SupportsExplicitClockSync() << endl;
 }
 
 // Logs the error and exits with an error code.
 void HandleError(battor::BattOrError error) {
   if (error != BATTOR_ERROR_NONE)
     LOG(FATAL) << "Fatal error when communicating with the BattOr: "
                << BattOrErrorToString(error);
 }
 
 // Prints an error message and exits due to a required thread failing to start.
 void ExitFromThreadStartFailure(const std::string& thread_name) {
   LOG(FATAL) << "Failed to start " << thread_name;
 }
 
+std::vector<std::string> TokenizeCommand(std::string cmd) {

[alexandermont, 2016/03/23 22:56:53]

The name of this function could be more general, like "TokenizeString" - it
works on any string, not just a string representing a command.

[rnephew (Reviews Here), 2016/03/24 00:11:26]

This will only ever be used to tokenize command inputs though, so it makes the
use of it more easily understood; I'll let Charlie weigh in and do whatever
popular consensus is.

[charliea (OOO until 10-5), 2016/03/24 15:05:21]

I don't have any strong opinion here. I'm fine with TokenizeString, although I
think they're both clear.

[rnephew (Reviews Here), 2016/03/24 15:15:47]

Switched to String

+  base::StringTokenizer tokenizer(cmd, " ");
+  std::vector<std::string> tokens;
+  while (tokenizer.GetNext())
+    tokens.push_back(tokenizer.token());
+  return tokens;
+}
+
 }  // namespace
 
 // Wrapper class containing all state necessary for an independent binary to
 // use a BattOrAgent to communicate with a BattOr.
 class BattOrAgentBin : public BattOrAgent::Listener {
  public:
   BattOrAgentBin()
       : done_(false, false),
         io_thread_(kIoThreadName),
         file_thread_(kFileThreadName),
@@ skipping 12 matching lines @@
     }
 
     SetUp(path);
 
     std::string cmd;
     for (;;) {
       std::getline(std::cin, cmd);
 
       if (cmd == "StartTracing") {
         StartTracing();
-      } else if (cmd == "StopTracing") {
-        StopTracing();
+      } else if (cmd.find("StopTracing") != std::string::npos) {

[alexandermont, 2016/03/23 22:56:53]

This just tests if "StopTracing" is in the string somewhere. I think you want to
test if "StopTracing" is at the beginning of the string.

[rnephew (Reviews Here), 2016/03/24 00:11:26]

Later it does make sure the first word is STopTracing and if it is not issues a
warning that it is an invalid command.

+        std::vector<std::string> tokens = TokenizeCommand(cmd);
+        if (tokens.size() == 1 && tokens[0] == "StopTracing") {
+          // No path given.
+          StopTracing();
+        } else if (tokens.size() == 2 && tokens[0] == "StopTracing") {
+          // Path given.
+          StopTracing(tokens[1]);
+        } else {
+          std::cout << "Invalid StopTracing command." << endl;
+          std::cout << kUsage << endl;
+          continue;
+        }
         break;
       } else if (cmd == "SupportsExplicitClockSync") {
         PrintSupportsExplicitClockSync();
       } else if (cmd.find("RecordClockSyncMarker") != std::string::npos) {

[alexandermont, 2016/03/23 22:56:53]

Same here; this just tests if "RecordClockSyncMarker" is somewhere in the
string, not necessarily at the beginning.

[rnephew (Reviews Here), 2016/03/24 00:11:26]

Same as above.

-        base::StringTokenizer tokenizer(cmd, " ");
-
-        std::vector<std::string> tokens;
-        while (tokenizer.GetNext())
-          tokens.push_back(tokenizer.token());
-
+        std::vector<std::string> tokens = TokenizeCommand(cmd);
         if (tokens.size() != 2 || tokens[0] != "RecordClockSyncMarker") {
           std::cout << "Invalid RecordClockSyncMarker command." << endl;
           std::cout << kUsage << endl;
           continue;
         }
 
         RecordClockSyncMarker(tokens[1]);
       } else if (cmd == "Exit") {
         break;
       } else {
@@ skipping 38 matching lines @@
 
   void OnStartTracingComplete(BattOrError error) override {
     if (error == BATTOR_ERROR_NONE)
       std::cout << "Done." << endl;
     else
       HandleError(error);
 
     done_.Signal();
   }
 
-  void StopTracing() {
+  void StopTracing(const std::string& path = "") {
+    trace_output_file_ = path;
     io_thread_.task_runner()->PostTask(
         FROM_HERE,
         base::Bind(&BattOrAgent::StopTracing, base::Unretained(agent_.get())));
     AwaitResult();

[charliea (OOO until 10-5), 2016/03/24 15:05:20]

After AwaitResult(), I'd do:

trace_output_file_ = std::string();

[rnephew (Reviews Here), 2016/03/24 15:15:47]

Done.

   }
 
   void OnStopTracingComplete(const std::string& trace,
                              BattOrError error) override {
     if (error == BATTOR_ERROR_NONE) {
-      std::cout << trace;
+      if (trace_output_file_.empty()) {
+        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();
+      }
       std::cout << "Done." << endl;
     } else {
       HandleError(error);
     }
 
     done_.Signal();
   }
 
   void RecordClockSyncMarker(const std::string& marker) {
     io_thread_.task_runner()->PostTask(
@@ skipping 50 matching lines @@
   // Event signaled when an async task has finished executing.
   base::WaitableEvent done_;
 
   // Threads needed for serial communication.
   base::Thread io_thread_;
   base::Thread file_thread_;
   base::Thread ui_thread_;
 
   // The agent capable of asynchronously communicating with the BattOr.
   scoped_ptr<BattOrAgent> agent_;
+
+  std::string trace_output_file_;
+
 };
 
 }  // namespace battor
 
 int main(int argc, char* argv[]) {
   base::AtExitManager exit_manager;
   base::CommandLine::Init(argc, argv);
   battor::BattOrAgentBin bin;
   return bin.Run(argc, argv);
 }