[BattOr] Make BattOr able to return firmware version.

tools/battor_agent/battor_agent_unittest.cc

Index: tools/battor_agent/battor_agent_unittest.cc
diff --git a/tools/battor_agent/battor_agent_unittest.cc b/tools/battor_agent/battor_agent_unittest.cc
index 0239649d2c972dfdf8de289657476e957dbdb38b..a0ac2165066354566c52e255cba13de15ed0657b 100644
--- a/tools/battor_agent/battor_agent_unittest.cc
+++ b/tools/battor_agent/battor_agent_unittest.cc
@@ -1,6 +1,7 @@
 // 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.
+#include <memory>

[charliea (OOO until 10-5), 2016/10/20 12:23:28]

I think there should probably be one empty line after the header

[rnephew (Reviews Here), 2016/10/20 18:03:44]

Done.

 
 #include "tools/battor_agent/battor_agent.h"
 
@@ -117,6 +118,13 @@ class BattOrAgentTest : public testing::Test, public BattOrAgent::Listener {
     command_error_ = error;
   }
 
+  void OnGetFirmwareGitHashComplete(const std::string& git_hash,
+                            BattOrError error) override {
+    is_command_complete_ = true;
+    command_error_ = error;
+    git_hash_ = git_hash;
+  }
+
   void OnBytesSent(bool success) {
     agent_->OnBytesSent(success);
     task_runner_->RunUntilIdle();
@@ -159,6 +167,10 @@ class BattOrAgentTest : public testing::Test, public BattOrAgent::Listener {
     // States required to RecordClockSyncMarker.
     CURRENT_SAMPLE_REQUEST_SENT,
     RECORD_CLOCK_SYNC_MARKER_COMPLETE,
+
+    // States required to GetFirmwareGitHash.
+    READ_GIT_HASH_REQUEST_SENT,

[charliea (OOO until 10-5), 2016/10/20 12:23:28]

I think probably GIT_FIRMWARE_HASH_REQUEST_[SENT,RECEIVED] is more consistent
with the names of the other enums

[rnephew (Reviews Here), 2016/10/20 18:03:44]

Done.

+    READ_GIT_HASH_RECEIVED,
   };
 
   // Runs BattOrAgent::StartTracing until it reaches the specified state by
@@ -248,6 +260,32 @@ class BattOrAgentTest : public testing::Test, public BattOrAgent::Listener {
                   CreateFrame(cal_frame_header, cal_frame, 1));
   }
 
+  // Runs BattOrAgent::GetFirmwareGitHash until it reaches the specified

[charliea (OOO until 10-5), 2016/10/20 12:23:28]

I think it probably makes sense to move this below RunRecordClockSyncMarkerTo...
in order to have a consistent ordering about the commands in these files. Right
now, StartTracing appears first, then StopTracing, then RecordClockSyncMarker,
so it probably makes sense to put all GitHash functions under
RecordClockSyncMarker functions

[rnephew (Reviews Here), 2016/10/20 18:03:44]

Done.

+  // state by feeding it the callbacks it needs to progress.
+  void RunGetFirmwareGitHashTo(BattOrAgentState end_state) {
+    is_command_complete_ = false;
+
+    GetAgent()->GetFirmwareGitHash();
+    GetTaskRunner()->RunUntilIdle();
+
+    GetAgent()->OnConnectionOpened(true);
+    GetTaskRunner()->RunUntilIdle();
+
+    if (end_state == BattOrAgentState::CONNECTED)
+      return;
+
+    OnBytesSent(true);
+    if (end_state == BattOrAgentState::READ_GIT_HASH_REQUEST_SENT)
+      return;
+
+    DCHECK(end_state == BattOrAgentState::READ_GIT_HASH_RECEIVED);
+
+    std::unique_ptr<std::vector<char>> git_hash_vector(
+        new std::vector<char>{'G', 'I', 'T', 'H', 'A', 'S', 'H'});
+    OnMessageRead(true, BATTOR_MESSAGE_TYPE_CONTROL_ACK,
+                  std::move(git_hash_vector));
+  }
+
   // Runs BattOrAgent::RecordClockSyncMarker until it reaches the specified
   // state by feeding it the callbacks it needs to progress.
   void RunRecordClockSyncMarkerTo(BattOrAgentState end_state) {
@@ -282,6 +320,7 @@ class BattOrAgentTest : public testing::Test, public BattOrAgent::Listener {
   bool IsCommandComplete() { return is_command_complete_; }
   BattOrError GetCommandError() { return command_error_; }
   std::string GetTrace() { return trace_; }
+  std::string GetGitHash() { return git_hash_; }
 
  private:
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
@@ -292,6 +331,7 @@ class BattOrAgentTest : public testing::Test, public BattOrAgent::Listener {
   bool is_command_complete_;
   BattOrError command_error_;
   std::string trace_;
+  std::string git_hash_;
 };
 
 TEST_F(BattOrAgentTest, StartTracing) {
@@ -893,4 +933,32 @@ TEST_F(BattOrAgentTest,
   EXPECT_EQ(BATTOR_ERROR_UNEXPECTED_MESSAGE, GetCommandError());
 }
 
+TEST_F(BattOrAgentTest, GetFirmwareGitHash) {
+  RunGetFirmwareGitHashTo(BattOrAgentState::READ_GIT_HASH_RECEIVED);
+  EXPECT_TRUE(IsCommandComplete());
+  EXPECT_EQ(BATTOR_ERROR_NONE, GetCommandError());
+  EXPECT_EQ("GITHASH", GetGitHash());
+}
+
+TEST_F(BattOrAgentTest, GetFirmwareGitHashFailsWithoutConnection) {
+  GetAgent()->GetFirmwareGitHash();
+  GetTaskRunner()->RunUntilIdle();
+
+  GetAgent()->OnConnectionOpened(false);
+  GetTaskRunner()->RunUntilIdle();
+
+  EXPECT_TRUE(IsCommandComplete());
+  EXPECT_EQ(BATTOR_ERROR_CONNECTION_FAILED, GetCommandError());
+}
+
+TEST_F(BattOrAgentTest, GetFirmwareGitHashFailsIfReadHasWrongType) {
+  RunGetFirmwareGitHashTo(BattOrAgentState::READ_GIT_HASH_REQUEST_SENT);
+
+  uint32_t current_sample = 1;
+  OnMessageRead(true, BATTOR_MESSAGE_TYPE_CONTROL,
+                ToCharVector(current_sample));
+
+  EXPECT_TRUE(IsCommandComplete());
+  EXPECT_EQ(BATTOR_ERROR_UNEXPECTED_MESSAGE, GetCommandError());
+}
 }  // namespace battor