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

tools/battor_agent/battor_agent.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 <iomanip>
 
 #include "base/bind.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "tools/battor_agent/battor_connection_impl.h"
 #include "tools/battor_agent/battor_sample_converter.h"
 
 using std::vector;
 
 namespace battor {
 
 namespace {
 
-// The number of seconds that it takes a BattOr to reset.
-const uint8_t kBattOrResetTimeSeconds = 5;
+// The maximum number of times to retry when init'ing a battor.
+const uint8_t kMaxInitAttempts = 20;
+
+// The number of milliseconds to wait before trying to init again.
+const uint16_t kInitRetryDelayMilliseconds = 100;
 
 // The maximum number of times to retry when reading a message.
 const uint8_t kMaxReadAttempts = 20;
 
 // The number of milliseconds to wait before trying to read a message again.
 const uint8_t kReadRetryDelayMilliseconds = 1;
 
 // The amount of time we need to wait after recording a clock sync marker in
 // order to ensure that the sample we synced to doesn't get thrown out.
 const uint8_t kStopTracingClockSyncDelayMilliseconds = 100;
 
 // The number of seconds allowed for a given action before timing out.
-const uint8_t kBattOrTimeoutSeconds = 10;
+const uint8_t kBattOrTimeoutSeconds = 4;
 
 // Returns true if the specified vector of bytes decodes to a message that is an
 // ack for the specified control message type.
 bool IsAckOfControlCommand(BattOrMessageType message_type,
                            BattOrControlMessageType control_message_type,
                            const vector<char>& msg) {
   if (message_type != BATTOR_MESSAGE_TYPE_CONTROL_ACK)
     return false;
 
   if (msg.size() != sizeof(BattOrControlMessageAck))
@@ skipping 67 matching lines @@
     Listener* listener,
     scoped_refptr<base::SingleThreadTaskRunner> file_thread_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> ui_thread_task_runner)
     : connection_(new BattOrConnectionImpl(path,
                                            this,
                                            file_thread_task_runner,
                                            ui_thread_task_runner)),
       listener_(listener),
       last_action_(Action::INVALID),
       command_(Command::INVALID),
+      num_init_attempts_(0),
       num_read_attempts_(0) {
   // We don't care what thread the constructor is called on - we only care that
   // all of the other method invocations happen on the same thread.
   thread_checker_.DetachFromThread();
 }
 
 BattOrAgent::~BattOrAgent() {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
@@ skipping 35 matching lines @@
     return;
   timeout_callback_.Cancel();
 
   if (!success) {
     CompleteCommand(BATTOR_ERROR_CONNECTION_FAILED);
     return;
   }
 
   switch (command_) {
     case Command::START_TRACING:
-      // TODO(charliea): Ideally, we'd just like to send an init, and the BattOr
-      // firmware can handle whether a reset is necessary or not, sending an
-      // init ack regardless. This reset can be removed once this is true.
-      // https://github.com/aschulm/battor/issues/30 tracks this.
-      PerformAction(Action::SEND_RESET);
+      num_init_attempts_ = 1;
+      PerformAction(Action::SEND_INIT);
       return;
     case Command::STOP_TRACING:
       PerformAction(Action::SEND_EEPROM_REQUEST);
       return;
     case Command::RECORD_CLOCK_SYNC_MARKER:
       PerformAction(Action::SEND_CURRENT_SAMPLE_REQUEST);
       return;
     case Command::INVALID:
       NOTREACHED();
   }
 }
 
 void BattOrAgent::OnBytesSent(bool success) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // Return immediately if whatever action we were trying to perform already
   // timed out.
   if (timeout_callback_.IsCancelled())
     return;
   timeout_callback_.Cancel();
 
   if (!success) {
     CompleteCommand(BATTOR_ERROR_SEND_ERROR);
     return;
   }
 
   switch (last_action_) {
-    case Action::SEND_RESET:
-      // TODO(charliea): Ideally, we'd just like to send an init, and the BattOr
-      // firmware can handle whether a reset is necessary or not, sending an
-      // init ack regardless. This reset can be removed once this is true.
-      // https://github.com/aschulm/battor/issues/30 tracks this.
-
-      // Wait for the reset to happen before sending the init message.
-      PerformDelayedAction(Action::SEND_INIT, base::TimeDelta::FromSeconds(
-                                                  kBattOrResetTimeSeconds));
-      return;
     case Action::SEND_INIT:
       PerformAction(Action::READ_INIT_ACK);
       return;
     case Action::SEND_SET_GAIN:
       PerformAction(Action::READ_SET_GAIN_ACK);
       return;
     case Action::SEND_START_TRACING:
       PerformAction(Action::READ_START_TRACING_ACK);
       return;
     case Action::SEND_EEPROM_REQUEST:
@@ skipping 30 matching lines @@
       case Action::READ_CURRENT_SAMPLE:
         if (++num_read_attempts_ > kMaxReadAttempts) {
           CompleteCommand(BATTOR_ERROR_RECEIVE_ERROR);
           return;
         }
 
         PerformDelayedAction(last_action_, base::TimeDelta::FromMilliseconds(
                                                kReadRetryDelayMilliseconds));
         return;
 
+      // Retry sending an INIT if it an ACK is not received.
+      case Action::SEND_INIT:
+      case Action::READ_INIT_ACK:
+        if (num_init_attempts_++ < kMaxInitAttempts) {
+          PerformDelayedAction(Action::SEND_INIT,
+              base::TimeDelta::FromMilliseconds(kInitRetryDelayMilliseconds));
+        } else {
+          CompleteCommand(BATTOR_ERROR_TOO_MANY_INIT_RETRIES);
+        }
+
+        return;
+
       default:
         CompleteCommand(BATTOR_ERROR_RECEIVE_ERROR);
         return;
     }
   }
 
   switch (last_action_) {
     case Action::READ_INIT_ACK:
       if (!IsAckOfControlCommand(type, BATTOR_CONTROL_MESSAGE_TYPE_INIT,
                                  *bytes)) {
-        CompleteCommand(BATTOR_ERROR_UNEXPECTED_MESSAGE);
+        if (num_init_attempts_++ < kMaxInitAttempts) {
+          PerformDelayedAction(Action::SEND_INIT,
+              base::TimeDelta::FromMilliseconds(kInitRetryDelayMilliseconds));
+        } else {
+          CompleteCommand(BATTOR_ERROR_TOO_MANY_INIT_RETRIES);
+        }
+
         return;
       }
 
       PerformAction(Action::SEND_SET_GAIN);
       return;
 
     case Action::READ_SET_GAIN_ACK:
       if (!IsAckOfControlCommand(type, BATTOR_CONTROL_MESSAGE_TYPE_SET_GAIN,
                                  *bytes)) {
         CompleteCommand(BATTOR_ERROR_UNEXPECTED_MESSAGE);
@@ skipping 103 matching lines @@
       base::TimeDelta::FromSeconds(kBattOrTimeoutSeconds));
 
   last_action_ = action;
 
   switch (action) {
     case Action::REQUEST_CONNECTION:
       BeginConnect();
       return;
 
     // The following actions are required for StartTracing:
-    case Action::SEND_RESET:
-      // Reset the BattOr to clear any preexisting state. After sending the
-      // reset signal, we need to wait for the reset to finish before issuing
-      // further commands.
-      SendControlMessage(BATTOR_CONTROL_MESSAGE_TYPE_RESET, 0, 0);
-      return;
     case Action::SEND_INIT:
-      // After resetting the BattOr, we need to make sure to flush the serial
-      // stream. Strange data may have been written into it during the reset.
+      // Clear out the serial data that may exist from prior init attempts.
       connection_->Flush();
 
       SendControlMessage(BATTOR_CONTROL_MESSAGE_TYPE_INIT, 0, 0);
       return;
     case Action::READ_INIT_ACK:
       connection_->ReadMessage(BATTOR_MESSAGE_TYPE_CONTROL_ACK);
       return;
     case Action::SEND_SET_GAIN:
       // Set the BattOr's gain. Setting the gain tells the BattOr the range of
       // power measurements that we expect to see.
@@ skipping 54 matching lines @@
   }
 }
 
 void BattOrAgent::PerformDelayedAction(Action action, base::TimeDelta delay) {
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE, base::Bind(&BattOrAgent::PerformAction, AsWeakPtr(), action),
       delay);
 }
 
 void BattOrAgent::OnActionTimeout() {
-  CompleteCommand(BATTOR_ERROR_TIMEOUT);
+  switch (last_action_) {
+    case Action::READ_INIT_ACK:
+      if (num_init_attempts_++ < kMaxInitAttempts) {
+        // OnMessageRead() will fail and retry SEND_INIT.
+        connection_->CancelReadMessage();
+      } else {
+        CompleteCommand(BATTOR_ERROR_TOO_MANY_INIT_RETRIES);
+      }
+
+      return;
+
+    default:
+      CompleteCommand(BATTOR_ERROR_TIMEOUT);
+  }
+
   timeout_callback_.Cancel();
 }
 
 void BattOrAgent::SendControlMessage(BattOrControlMessageType type,
                                      uint16_t param1,
                                      uint16_t param2) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   BattOrControlMessage msg{type, param1, param2};
   connection_->SendBytes(BATTOR_MESSAGE_TYPE_CONTROL, &msg, sizeof(msg));
@@ skipping 64 matching lines @@
     if (clock_sync_marker != clock_sync_markers_.end())
       trace_stream << " <" << clock_sync_marker->second << ">";
 
     trace_stream << std::endl;
   }
 
   return trace_stream.str();
 }
 
 }  // namespace battor