tools/battor_agent: Changes the BattOr Agent binary to be interactive

tools/battor_agent/battor_agent_bin.cc

Index: tools/battor_agent/battor_agent_bin.cc
diff --git a/tools/battor_agent/battor_agent_bin.cc b/tools/battor_agent/battor_agent_bin.cc
index 7ba898fa175dd26b27480b8630cc567e77dd7b89..3a27793eb79ae0e6ffb3d82d39f80ec01ac3fb66 100644
--- a/tools/battor_agent/battor_agent_bin.cc
+++ b/tools/battor_agent/battor_agent_bin.cc
@@ -5,7 +5,7 @@
 // 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..
+// tracing commands to the BattOr through an interactive shell.
 
 #include <stdint.h>
 
@@ -23,7 +23,6 @@
 #include "tools/battor_agent/battor_error.h"
 #include "tools/battor_agent/battor_finder.h"
 
-using std::cout;
 using std::endl;
 
 namespace {
@@ -34,38 +33,25 @@ const char kUiThreadName[] = "BattOr UI Thread";
 const int32_t kBattOrCommandTimeoutSeconds = 10;
 
 void PrintUsage() {
-  cout << "Usage: battor_agent <command> <arguments> <switches>" << endl
+  std::cout << "Start the battor_agent shell with:" << endl
        << endl
-       << "Commands:" << endl
+       << "  battor_agent <switches>" << endl
+       << endl
+       << "Switches: " << endl
+       << "  --battor-path=<path> Uses the specified BattOr path." << endl
+       << endl
+       << "Once in the shell, you can issue the following commands:" << endl
        << endl
        << "  StartTracing" << endl
        << "  StopTracing" << endl
        << "  SupportsExplicitClockSync" << endl
        << "  RecordClockSyncMarker <marker>" << endl
-       << "  IssueClockSyncMarker" << endl
-       << "  Help" << endl
-       << endl
-       << "Switches:" << endl
-       << endl
-       << "  --battor-path=<path> Uses the specified BattOr path." << endl;
+       << "  Exit" << endl
+       << "  Help" << endl;
 }
 
 void PrintSupportsExplicitClockSync() {
-  cout << battor::BattOrAgent::SupportsExplicitClockSync() << endl;
-}
-
-// Retrieves argument argnum from the argument list, or an empty string if the
-// argument doesn't exist.
-std::string GetArg(size_t argnum, base::CommandLine::StringVector args) {
-  if (argnum >= args.size()) {
-    return std::string();
-  }
-
-#if defined(OS_WIN)
-  return base::WideToUTF8(args[argnum]);
-#else
-  return args[argnum];
-#endif
+  std::cout << battor::BattOrAgent::SupportsExplicitClockSync() << endl;
 }
 
 // Checks if an error occurred and, if it did, prints the error and exits
@@ -96,43 +82,36 @@ class BattOrAgentBin : public BattOrAgent::Listener {
 
   ~BattOrAgentBin() { DCHECK(!agent_); }
 
-  // Runs the BattOr binary and returns the exit code.
+  // Starts the interactive BattOr agent shell and eventually returns an exit
+  // code.
   int Run(int argc, char* argv[]) {
-    base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-    std::string cmd = GetArg(0, command_line->GetArgs());
-    if (cmd.empty()) {
-      PrintUsage();
-      exit(1);
-    }
-
-    // SupportsExplicitClockSync doesn't need to use the serial connection, so
-    // handle it separately.
-    if (cmd == "SupportsExplicitClockSync") {
-      PrintSupportsExplicitClockSync();
-      return 0;
-    }
-
     // If we don't have any BattOr to use, exit.
     std::string path = BattOrFinder::FindBattOr();

[alexandermont, 2016/02/24 22:01:48]

Where is the "battor-path" switch actually read? It looks like it tries to find
a path using FindBattOr, then uses that path in the SetUp, and doesn't seem to
use the path that you passed in on battor-path. Is that somewhere else in the
code, not in this file?

[Zhen Wang, 2016/02/24 22:35:45]

Yes, it is done in FindBattOr. base::CommandLine::Init() called in main()
initialized the command line switches.

     if (path.empty()) {
-      cout << "Unable to find a BattOr." << endl;
+      std::cout << "Unable to find a BattOr." << endl;
       exit(1);
     }
 
     SetUp(path);
 
-    if (cmd == "StartTracing") {
-      StartTracing();
-    } else if (cmd == "StopTracing") {
-      StopTracing();
-    } else if (cmd == "RecordClockSyncMarker") {
-      // TODO(charliea): Write RecordClockSyncMarker.
-    } else if (cmd == "IssueClockSyncMarker") {
-      // TODO(charliea): Write IssueClockSyncMarker.
-    } else {
-      TearDown();
-      PrintUsage();
-      return 1;
+    std::string cmd;
+    for (;;) {

[alexandermont, 2016/02/24 22:01:47]

maybe while(true) instead?

+      std::getline(std::cin, cmd);
+
+      if (cmd == "StartTracing") {
+        StartTracing();
+      } else if (cmd == "StopTracing") {

[alexandermont, 2016/02/24 22:01:48]

documentation should mention that StopTracing makes the binary exit

+        StopTracing();
+        break;
+      } else if (cmd == "SupportsExplicitClockSync") {
+        PrintSupportsExplicitClockSync();
+      } else if (cmd == "RecordClockSyncMarker") {
+        // TODO(charliea): Write RecordClockSyncMarker.
+      } else if (cmd == "Exit") {
+        break;
+      } else {
+        PrintUsage();
+      }
     }
 
     TearDown();
@@ -172,6 +151,7 @@ class BattOrAgentBin : public BattOrAgent::Listener {
 
   void OnStartTracingComplete(BattOrError error) override {
     error_ = error;
+    std::cout << "Done." << endl;
     done_.Signal();
   }
 
@@ -187,7 +167,7 @@ class BattOrAgentBin : public BattOrAgent::Listener {
     error_ = error;
 
     if (error == BATTOR_ERROR_NONE)
-      cout << trace << endl;
+      std::cout << trace;
 
     done_.Signal();
   }