[BattOr] Make BattOr able to return firmware version.

tools/battor_agent/battor_agent_unittest.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.
+#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"
 
 #include "base/test/test_simple_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "tools/battor_agent/battor_protocol_types.h"
 
 using namespace testing;
@@ skipping 96 matching lines @@
     is_command_complete_ = true;
     command_error_ = error;
     trace_ = trace;
   }
 
   void OnRecordClockSyncMarkerComplete(BattOrError error) override {
     is_command_complete_ = true;
     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();
   }
 
   void OnMessageRead(bool success,
                      BattOrMessageType type,
                      std::unique_ptr<std::vector<char>> bytes) {
     agent_->OnMessageRead(success, type, std::move(bytes));
     task_runner_->RunUntilIdle();
@@ skipping 22 matching lines @@
 
     // States required to StopTracing.
     EEPROM_REQUEST_SENT,
     EEPROM_RECEIVED,
     SAMPLES_REQUEST_SENT,
     CALIBRATION_FRAME_RECEIVED,
 
     // 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
   // feeding it the callbacks it needs to progress.
   void RunStartTracingTo(BattOrAgentState end_state) {
     is_command_complete_ = false;
 
     GetAgent()->StartTracing();
     GetTaskRunner()->RunUntilIdle();
 
@@ skipping 69 matching lines @@
       return;
 
     DCHECK(end_state == BattOrAgentState::CALIBRATION_FRAME_RECEIVED);
 
     BattOrFrameHeader cal_frame_header{0, sizeof(RawBattOrSample)};
     RawBattOrSample cal_frame[] = {RawBattOrSample{1, 1}};
     OnMessageRead(true, BATTOR_MESSAGE_TYPE_SAMPLES,
                   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) {
     is_command_complete_ = false;
 
     GetAgent()->RecordClockSyncMarker(kClockSyncId);
     GetTaskRunner()->RunUntilIdle();
 
     GetAgent()->OnConnectionOpened(true);
     GetTaskRunner()->RunUntilIdle();
@@ skipping 14 matching lines @@
 
   TestableBattOrAgent* GetAgent() { return agent_.get(); }
 
   scoped_refptr<base::TestSimpleTaskRunner> GetTaskRunner() {
     return task_runner_;
   }
 
   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_;
   // Needed to support ThreadTaskRunnerHandle::Get() in code under test.
   base::ThreadTaskRunnerHandle thread_task_runner_handle_;
 
   std::unique_ptr<TestableBattOrAgent> agent_;
   bool is_command_complete_;
   BattOrError command_error_;
   std::string trace_;
+  std::string git_hash_;
 };
 
 TEST_F(BattOrAgentTest, StartTracing) {
   testing::InSequence s;
   EXPECT_CALL(*GetAgent()->GetConnection(), Open());
 
   EXPECT_CALL(*GetAgent()->GetConnection(), Flush());
   BattOrControlMessage init_msg{BATTOR_CONTROL_MESSAGE_TYPE_INIT, 0, 0};
   EXPECT_CALL(
       *GetAgent()->GetConnection(),
@@ skipping 581 matching lines @@
   RunRecordClockSyncMarkerTo(BattOrAgentState::CURRENT_SAMPLE_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());
 }
 
+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