[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) {
+TEST_F(BattOrAgentTest, StartTracingFailsIfInitSendFails) {
   RunStartTracingTo(BattOrAgentState::CONNECTED);
   OnBytesSent(false);
 
   EXPECT_TRUE(IsCommandComplete());
-  EXPECT_EQ(BATTOR_ERROR_SEND_ERROR, GetCommandError());
-}
-
-TEST_F(BattOrAgentTest, StartTracingFailsIfInitSendFails) {
-  RunStartTracingTo(BattOrAgentState::RESET_SENT);
-  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);
 
   EXPECT_TRUE(IsCommandComplete());
   EXPECT_EQ(BATTOR_ERROR_RECEIVE_ERROR, GetCommandError());
 }
 
 TEST_F(BattOrAgentTest, StartTracingFailsIfStartTracingWrongAckRead) {
   RunStartTracingTo(BattOrAgentState::START_TRACING_SENT);
   OnMessageRead(true, BATTOR_MESSAGE_TYPE_CONTROL_ACK, ToCharVector(kInitAck));
 
   EXPECT_TRUE(IsCommandComplete());
   EXPECT_EQ(BATTOR_ERROR_UNEXPECTED_MESSAGE, GetCommandError());
 }
 
+TEST_F(BattOrAgentTest, StartTracingSucceedsWithOneInitFailure) {
+  RunStartTracingTo(BattOrAgentState::INIT_SENT);
+
+  // Instead of an INIT ACK get some samples. This will force an init retry.

[charliea (OOO until 10-5), 2016/06/15 18:56:34]

Maybe "Send some samples instead of an INIT ACK. This will force an init
retry."?

[aschulman, 2016/06/15 23:37:09]

Done.

+  BattOrFrameHeader frame_header{1, 3 * sizeof(RawBattOrSample)};
+  RawBattOrSample frame[] = {
+      RawBattOrSample{1, 1}, RawBattOrSample{2, 2}, RawBattOrSample{3, 3},
+  };
+  OnMessageRead(true, BATTOR_MESSAGE_TYPE_SAMPLES,
+                CreateFrame(frame_header, frame, 3));
+
+  OnBytesSent(true);
+  OnMessageRead(true, BATTOR_MESSAGE_TYPE_CONTROL_ACK, ToCharVector(kInitAck));
+
+  OnBytesSent(true);
+  OnMessageRead(true, BATTOR_MESSAGE_TYPE_CONTROL_ACK,
+                ToCharVector(kSetGainAck));
+
+  OnBytesSent(true);
+  OnMessageRead(true, BATTOR_MESSAGE_TYPE_CONTROL_ACK,
+                ToCharVector(kStartTracingAck));
+
+  EXPECT_TRUE(IsCommandComplete());
+  EXPECT_EQ(BATTOR_ERROR_NONE, GetCommandError());
+}
+
 TEST_F(BattOrAgentTest, StopTracing) {
   testing::InSequence s;
   EXPECT_CALL(*GetAgent()->GetConnection(), Open());
 
   BattOrControlMessage request_eeprom_msg{
       BATTOR_CONTROL_MESSAGE_TYPE_READ_EEPROM, sizeof(BattOrEEPROM), 0};
   EXPECT_CALL(
       *GetAgent()->GetConnection(),
       SendBytes(BATTOR_MESSAGE_TYPE_CONTROL,
                 BufferEq(&request_eeprom_msg, sizeof(request_eeprom_msg)),
@@ skipping 418 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