[BattOr] Make BattOr able to return firmware version.

tools/battor_agent/battor_agent.h

 // 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.
 
 #ifndef TOOLS_BATTOR_AGENT_BATTOR_AGENT_H_
 #define TOOLS_BATTOR_AGENT_BATTOR_AGENT_H_
 
 #include <map>
 
 #include "base/cancelable_callback.h"
@@ skipping 25 matching lines @@
                     public base::SupportsWeakPtr<BattOrAgent> {
  public:
   // The listener interface that must be implemented in order to interact with
   // the BattOrAgent.
   class Listener {
    public:
     virtual void OnStartTracingComplete(BattOrError error) = 0;
     virtual void OnStopTracingComplete(const std::string& trace,
                                        BattOrError error) = 0;
     virtual void OnRecordClockSyncMarkerComplete(BattOrError error) = 0;
+    virtual void OnGetVersionComplete(const int version, BattOrError error) = 0;
   };
 
   BattOrAgent(
       const std::string& path,
       Listener* listener,
       scoped_refptr<base::SingleThreadTaskRunner> file_thread_task_runner,
       scoped_refptr<base::SingleThreadTaskRunner> ui_thread_task_runner);
   virtual ~BattOrAgent();
 
   void StartTracing();
   void StopTracing();
   void RecordClockSyncMarker(const std::string& marker);
 

[charliea (OOO until 10-5), 2016/10/04 15:15:48]

Rather than `int Version()` below, I think you want

`void GetVersion()` here, with the assumption that `void
OnGetVersionComplete(bool success, int version)` will be called when the command
finishes. Basically, you want to change this from being a function that returns
its value via a return value to a function that returns its value via a
callback.

The reason for this is that you actually need to do some communication with the
BattOr in order to get that version (over serial), and that communication takes
time. There's also an assumption that the thread that these functions
(StartTracing, StopTracing, etc.) are run on won't block. These two facts
together make it impossible to return our version via return value.

The reason that SupportsExplicitClockSync *isn't* like this is because we know
the answer (true) without having to communicate with the BattOr.

   // Returns whether the BattOr is able to record clock sync markers in its own
   // trace log.
   static bool SupportsExplicitClockSync() { return true; }
 
   // BattOrConnection::Listener implementation.
   void OnConnectionOpened(bool success) override;
   void OnBytesSent(bool success) override;
   void OnMessageRead(bool success,
                      BattOrMessageType type,
                      std::unique_ptr<std::vector<char>> bytes) override;
+  int Version();
 
  protected:
   // The connection that knows how to communicate with the BattOr in terms of
   // protocol primitives. This is protected so that it can be replaced with a
   // fake in testing.
   std::unique_ptr<BattOrConnection> connection_;
 
   // Timeout for when an action isn't completed within the allotted time. This
   // is virtual and protected so that timeouts can be disabled in testing. The
   // testing task runner that runs delayed tasks immediately deals poorly with
   // timeouts posted as future tasks.
   virtual void OnActionTimeout();
 
  private:
   enum class Command {
     INVALID,
     START_TRACING,
     STOP_TRACING,
     RECORD_CLOCK_SYNC_MARKER,
+    VERSION,
   };
 
   enum class Action {
     INVALID,
 
     // Actions required to connect to a BattOr.
     REQUEST_CONNECTION,
 
     // Actions required for starting tracing.
     SEND_INIT,
@@ skipping 82 matching lines @@
 
   // The timeout that's run when an action times out.
   base::CancelableClosure timeout_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(BattOrAgent);
 };
 
 }  // namespace battor
 
 #endif  // TOOLS_BATTOR_AGENT_BATTOR_AGENT_H_