[battor agent] Fix the init sequence so it retries inits instead of resets.

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 "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"
@@ skipping 49 matching lines @@
       : BattOrConnection(listener) {}
   ~MockBattOrConnection() override {}
 
   MOCK_METHOD0(Open, void());
   MOCK_METHOD0(Close, void());
   MOCK_METHOD3(SendBytes,
                void(BattOrMessageType type,
                     const void* buffer,
                     size_t bytes_to_send));
   MOCK_METHOD1(ReadMessage, void(BattOrMessageType type));
+  MOCK_METHOD0(CancelReadMessage, void());
   MOCK_METHOD0(Flush, void());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockBattOrConnection);
 };
 
 }  // namespace
 
 // TestableBattOrAgent uses a fake BattOrConnection to be testable.
 class TestableBattOrAgent : public BattOrAgent {
@@ skipping 55 matching lines @@
     is_command_complete_ = false;
     command_error_ = BATTOR_ERROR_NONE;
   }
 
   // Possible states that the BattOrAgent can be in.
   enum class BattOrAgentState {
     // States required to connect to a BattOr.
     CONNECTED,
 
     // States required to StartTracing.
-    RESET_SENT,
     INIT_SENT,
     INIT_ACKED,
     SET_GAIN_SENT,
     GAIN_ACKED,
     START_TRACING_SENT,
     START_TRACING_COMPLETE,
 
     // States required to StopTracing.
     EEPROM_REQUEST_SENT,
     EEPROM_RECEIVED,
@@ skipping 13 matching lines @@
     GetAgent()->StartTracing();
     GetTaskRunner()->RunUntilIdle();
 
     GetAgent()->OnConnectionOpened(true);
     GetTaskRunner()->RunUntilIdle();
 
     if (end_state == BattOrAgentState::CONNECTED)
       return;
 
     OnBytesSent(true);
-    if (end_state == BattOrAgentState::RESET_SENT)
-      return;
-
-    OnBytesSent(true);
     if (end_state == BattOrAgentState::INIT_SENT)
       return;
 
     OnMessageRead(true, BATTOR_MESSAGE_TYPE_CONTROL_ACK,
                   ToCharVector(kInitAck));
     if (end_state == BattOrAgentState::INIT_ACKED)
       return;
 
     OnBytesSent(true);
     if (end_state == BattOrAgentState::SET_GAIN_SENT)
@@ skipping 102 matching lines @@
   std::unique_ptr<TestableBattOrAgent> agent_;
   bool is_command_complete_;
   BattOrError command_error_;
   std::string trace_;
 };
 
 TEST_F(BattOrAgentTest, StartTracing) {
   testing::InSequence s;
   EXPECT_CALL(*GetAgent()->GetConnection(), Open());
 
-  BattOrControlMessage reset_msg{BATTOR_CONTROL_MESSAGE_TYPE_RESET, 0, 0};
-  EXPECT_CALL(
-      *GetAgent()->GetConnection(),
-      SendBytes(BATTOR_MESSAGE_TYPE_CONTROL,
-                BufferEq(&reset_msg, sizeof(reset_msg)), sizeof(reset_msg)));
-
   EXPECT_CALL(*GetAgent()->GetConnection(), Flush());
   BattOrControlMessage init_msg{BATTOR_CONTROL_MESSAGE_TYPE_INIT, 0, 0};
   EXPECT_CALL(
       *GetAgent()->GetConnection(),
       SendBytes(BATTOR_MESSAGE_TYPE_CONTROL,
                 BufferEq(&init_msg, sizeof(init_msg)), sizeof(init_msg)));
 
   EXPECT_CALL(*GetAgent()->GetConnection(),
               ReadMessage(BATTOR_MESSAGE_TYPE_CONTROL_ACK));
 
@@ skipping 26 matching lines @@
   GetAgent()->StartTracing();
   GetTaskRunner()->RunUntilIdle();
 
   GetAgent()->OnConnectionOpened(false);
   GetTaskRunner()->RunUntilIdle();
 
   EXPECT_TRUE(IsCommandComplete());
   EXPECT_EQ(BATTOR_ERROR_CONNECTION_FAILED, GetCommandError());
 }
 
 TEST_F(BattOrAgentTest, StartTracingFailsIfResetSendFails) {

[charliea (OOO until 10-5), 2016/06/15 14:28:50]

I think that this test should be deleted.

   RunStartTracingTo(BattOrAgentState::CONNECTED);
   OnBytesSent(false);
 
   EXPECT_TRUE(IsCommandComplete());
   EXPECT_EQ(BATTOR_ERROR_SEND_ERROR, GetCommandError());
 }
 
 TEST_F(BattOrAgentTest, StartTracingFailsIfInitSendFails) {
-  RunStartTracingTo(BattOrAgentState::RESET_SENT);
+  RunStartTracingTo(BattOrAgentState::CONNECTED);
   OnBytesSent(false);
 
   EXPECT_TRUE(IsCommandComplete());
   EXPECT_EQ(BATTOR_ERROR_SEND_ERROR, GetCommandError());
 }
 
 TEST_F(BattOrAgentTest, StartTracingFailsIfInitAckReadFails) {
   RunStartTracingTo(BattOrAgentState::INIT_SENT);
-  OnMessageRead(false, BATTOR_MESSAGE_TYPE_CONTROL_ACK, nullptr);
+
+  for (int i = 0; i < 20; i++) {
+    OnMessageRead(false, BATTOR_MESSAGE_TYPE_CONTROL_ACK, nullptr);
+
+    // Bytes will be sent because INIT will be retried.
+    OnBytesSent(true);
+   }
 
   EXPECT_TRUE(IsCommandComplete());
-  EXPECT_EQ(BATTOR_ERROR_RECEIVE_ERROR, GetCommandError());
+  EXPECT_EQ(BATTOR_ERROR_TOO_MANY_INIT_RETRIES, GetCommandError());
 }
 
 TEST_F(BattOrAgentTest, StartTracingFailsIfInitWrongAckRead) {
   RunStartTracingTo(BattOrAgentState::INIT_SENT);
-  OnMessageRead(true, BATTOR_MESSAGE_TYPE_CONTROL_ACK,
-                ToCharVector(kStartTracingAck));
+
+  for (int i = 0; i < 20; i++) {
+    OnMessageRead(true, BATTOR_MESSAGE_TYPE_CONTROL_ACK,
+                  ToCharVector(kStartTracingAck));
+
+    // Bytes will be sent because INIT will be retried.
+    OnBytesSent(true);
+  }
 
   EXPECT_TRUE(IsCommandComplete());
-  EXPECT_EQ(BATTOR_ERROR_UNEXPECTED_MESSAGE, GetCommandError());
+  EXPECT_EQ(BATTOR_ERROR_TOO_MANY_INIT_RETRIES, GetCommandError());
 }
 
 TEST_F(BattOrAgentTest, StartTracingFailsIfSetGainSendFails) {
-  RunStartTracingTo(BattOrAgentState::RESET_SENT);
+  RunStartTracingTo(BattOrAgentState::INIT_SENT);
   OnBytesSent(false);
 
   EXPECT_TRUE(IsCommandComplete());
   EXPECT_EQ(BATTOR_ERROR_SEND_ERROR, GetCommandError());
 }
 
 TEST_F(BattOrAgentTest, StartTracingFailsIfSetGainAckReadFails) {
   RunStartTracingTo(BattOrAgentState::SET_GAIN_SENT);
   OnMessageRead(false, BATTOR_MESSAGE_TYPE_CONTROL_ACK, nullptr);
 
   EXPECT_TRUE(IsCommandComplete());
   EXPECT_EQ(BATTOR_ERROR_RECEIVE_ERROR, GetCommandError());
 }
 
 TEST_F(BattOrAgentTest, StartTracingFailsIfSetGainWrongAckRead) {
   RunStartTracingTo(BattOrAgentState::SET_GAIN_SENT);
   OnMessageRead(true, BATTOR_MESSAGE_TYPE_CONTROL_ACK,
                 ToCharVector(kStartTracingAck));
 
   EXPECT_TRUE(IsCommandComplete());
   EXPECT_EQ(BATTOR_ERROR_UNEXPECTED_MESSAGE, GetCommandError());
 }
 
 TEST_F(BattOrAgentTest, StartTracingFailsIfStartTracingSendFails) {
-  RunStartTracingTo(BattOrAgentState::RESET_SENT);
+  RunStartTracingTo(BattOrAgentState::INIT_SENT);
   OnBytesSent(false);
 
   EXPECT_TRUE(IsCommandComplete());
   EXPECT_EQ(BATTOR_ERROR_SEND_ERROR, GetCommandError());
 }
 
 TEST_F(BattOrAgentTest, StartTracingFailsIfStartTracingAckReadFails) {
   RunStartTracingTo(BattOrAgentState::START_TRACING_SENT);
   OnMessageRead(false, BATTOR_MESSAGE_TYPE_CONTROL_ACK, nullptr);
 
@@ skipping 139 matching lines @@
 }
 
 TEST_F(BattOrAgentTest, StopTracingFailsIfRequestSamplesFails) {
   RunStopTracingTo(BattOrAgentState::EEPROM_RECEIVED);
   OnBytesSent(false);
 
   EXPECT_TRUE(IsCommandComplete());
   EXPECT_EQ(BATTOR_ERROR_SEND_ERROR, GetCommandError());
 }
 
 TEST_F(BattOrAgentTest, StopTracingSucceedsWithOneCalibrationFrameReadFailure) {

[charliea (OOO until 10-5), 2016/06/15 14:28:50]

I would add a test (up by the StartTracing tests) that's similar to this one and
verifies that the overall StartTracing command can succeed even if there's an
init failure.

[aschulman, 2016/06/15 16:47:41]

Done.

   RunStopTracingTo(BattOrAgentState::SAMPLES_REQUEST_SENT);
 
   // Make a read fail in order to make sure that the agent will retry.
   OnMessageRead(false, BATTOR_MESSAGE_TYPE_SAMPLES, nullptr);
 
   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));
 
@@ skipping 280 matching lines @@
 
   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