Prototype of fix for mac FTDI crash where the entire trace is read even if there is an error (e.g.,…

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 <iostream>
 
 #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 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 = 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,
@@ skipping 42 matching lines @@
 
   // Each frame should contain a header and an integer number of BattOr samples.
   if ((msg.size() - sizeof(BattOrFrameHeader)) % sizeof(RawBattOrSample) != 0)
     return false;
 
   // The first bytes in the frame contain the frame header.
   const char* frame_ptr = reinterpret_cast<const char*>(msg.data());
   memcpy(frame_header, frame_ptr, sizeof(BattOrFrameHeader));
   frame_ptr += sizeof(BattOrFrameHeader);
 
-  if (frame_header->sequence_number != expected_sequence_number) {
-    LOG(WARNING) << "Unexpected sequence number: wanted "
-                 << expected_sequence_number << ", but got "
-                 << frame_header->sequence_number << ".";
+  if (frame_header->sequence_number != expected_sequence_number)
     return false;
-  }
 
   size_t remaining_bytes = msg.size() - sizeof(BattOrFrameHeader);
   if (remaining_bytes != frame_header->length)
     return false;
 
   samples->resize(remaining_bytes / sizeof(RawBattOrSample));
   memcpy(samples->data(), frame_ptr, remaining_bytes);
 
   return true;
 }
 
 }  // namespace
 
 BattOrAgent::BattOrAgent(
     const std::string& path,
     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) {
+      num_init_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());
 }
 
 void BattOrAgent::StartTracing() {
@@ skipping 73 matching lines @@
     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:
-      num_read_attempts_ = 1;
       PerformAction(Action::READ_EEPROM);
       return;
     case Action::SEND_SAMPLES_REQUEST:
-      num_read_attempts_ = 1;
       PerformAction(Action::READ_CALIBRATION_FRAME);
       return;
     case Action::SEND_CURRENT_SAMPLE_REQUEST:
-      num_read_attempts_ = 1;
       PerformAction(Action::READ_CURRENT_SAMPLE);
       return;
     default:
       CompleteCommand(BATTOR_ERROR_UNEXPECTED_MESSAGE);
   }
 }
 
 void BattOrAgent::OnMessageRead(bool success,
                                 BattOrMessageType type,
                                 std::unique_ptr<vector<char>> bytes) {
   // Return immediately if whatever action we were trying to perform already
   // timed out.
   if (timeout_callback_.IsCancelled())
     return;
   timeout_callback_.Cancel();
 
   if (!success) {
     switch (last_action_) {
       case Action::READ_EEPROM:
+      case Action::READ_CURRENT_SAMPLE:
+        CompleteCommand(BATTOR_ERROR_RECEIVE_ERROR);
+        return;
       case Action::READ_CALIBRATION_FRAME:
       case Action::READ_DATA_FRAME:
-      case Action::READ_CURRENT_SAMPLE:
-        if (num_read_attempts_++ > kMaxReadAttempts) {
-          CompleteCommand(BATTOR_ERROR_RECEIVE_ERROR);
-          return;
-        }
-
-        PerformDelayedAction(last_action_, base::TimeDelta::FromMilliseconds(
-                                               kReadRetryDelayMilliseconds));
+        CompleteCommand(BATTOR_ERROR_RECEIVE_ERROR);
         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);
         }
 
@@ skipping 58 matching lines @@
       base::TimeDelta request_samples_delay = std::max(
           min_request_samples_time - base::TimeTicks::Now(), base::TimeDelta());
 
       PerformDelayedAction(Action::SEND_SAMPLES_REQUEST, request_samples_delay);
       return;
     }
     case Action::READ_CALIBRATION_FRAME: {
       BattOrFrameHeader frame_header;
       if (!ParseSampleFrame(type, *bytes, next_sequence_number_++,
                             &frame_header, &calibration_frame_)) {
-        CompleteCommand(BATTOR_ERROR_UNEXPECTED_MESSAGE);
-        return;
+        if (read_frame_error_ == BATTOR_ERROR_NONE)
+          read_frame_error_ = BATTOR_ERROR_UNEXPECTED_MESSAGE;
+      } else {
+        // Make sure that the calibration frame has actual samples in it.
+        if (calibration_frame_.empty()) {
+          if (read_frame_error_ == BATTOR_ERROR_NONE)
+            read_frame_error_ = BATTOR_ERROR_UNEXPECTED_MESSAGE;
+        }
       }
 
-      // Make sure that the calibration frame has actual samples in it.
-      if (calibration_frame_.empty()) {
-        CompleteCommand(BATTOR_ERROR_UNEXPECTED_MESSAGE);
-        return;
-      }
-
-      num_read_attempts_ = 1;
       PerformAction(Action::READ_DATA_FRAME);
       return;
     }
 
     case Action::READ_DATA_FRAME: {
       BattOrFrameHeader frame_header;
       vector<RawBattOrSample> frame;
       if (!ParseSampleFrame(type, *bytes, next_sequence_number_++,
                             &frame_header, &frame)) {
-        CompleteCommand(BATTOR_ERROR_UNEXPECTED_MESSAGE);
-        return;
+        if (read_frame_error_ == BATTOR_ERROR_NONE)
+          read_frame_error_ = BATTOR_ERROR_UNEXPECTED_MESSAGE;
+      } else {
+        // Check for the empty frame the BattOr uses to indicate it's done
+        // streaming samples.
+        if (frame.empty()) {
+          CompleteCommand(BATTOR_ERROR_NONE);
+          return;
+        }
+
+        samples_.insert(samples_.end(), frame.begin(), frame.end());
       }
 
-      // Check for the empty frame the BattOr uses to indicate it's done
-      // streaming samples.
-      if (frame.empty()) {
-        CompleteCommand(BATTOR_ERROR_NONE);
-        return;
-      }
-
-      samples_.insert(samples_.end(), frame.begin(), frame.end());
-
-      num_read_attempts_ = 1;
       PerformAction(Action::READ_DATA_FRAME);
       return;
     }
 
     case Action::READ_CURRENT_SAMPLE:
       if (type != BATTOR_MESSAGE_TYPE_CONTROL_ACK ||
           bytes->size() != sizeof(uint32_t)) {
         CompleteCommand(BATTOR_ERROR_UNEXPECTED_MESSAGE);
         return;
       }
@@ skipping 65 matching lines @@
     case Action::SEND_SAMPLES_REQUEST:
       // Send a request to the BattOr to tell it to start streaming the samples
       // that it's stored on its SD card over the serial connection.
       SendControlMessage(BATTOR_CONTROL_MESSAGE_TYPE_READ_SD_UART, 0, 0);
       return;
     case Action::READ_CALIBRATION_FRAME:
       // Data frames are numbered starting at zero and counting up by one each
       // data frame. We keep track of the next frame sequence number we expect
       // to see to ensure we don't miss any data.
       next_sequence_number_ = 0;
+
+      // Initialize error that could be encountered while reading data.
+      read_frame_error_ = BATTOR_ERROR_NONE;
     case Action::READ_DATA_FRAME:
       // The first frame sent back from the BattOr contains voltage and current
       // data that excludes whatever device is being measured from the
       // circuit. We use this first frame to establish a baseline voltage and
       // current.
       //
       // All further frames contain real (but uncalibrated) voltage and current
       // data.
       connection_->ReadMessage(BATTOR_MESSAGE_TYPE_SAMPLES);
       return;
@@ skipping 19 matching lines @@
 
 void BattOrAgent::OnActionTimeout() {
   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;
+    case Action::READ_CALIBRATION_FRAME:
+    case Action::READ_DATA_FRAME:
+      // Fail with the first frame read error error if one was encountered.
+      if (read_frame_error_ != BATTOR_ERROR_NONE)
+        CompleteCommand(read_frame_error_);
+      else
+        CompleteCommand(BATTOR_ERROR_TIMEOUT);
       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));
 }
 
 void BattOrAgent::CompleteCommand(BattOrError error) {
   switch (command_) {
     case Command::START_TRACING:
       base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::Bind(&Listener::OnStartTracingComplete,
                                 base::Unretained(listener_), error));
       break;
     case Command::STOP_TRACING:
+      if (read_frame_error_ != BATTOR_ERROR_NONE)
+        error = read_frame_error_;
+
       base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE,
           base::Bind(&Listener::OnStopTracingComplete,
                      base::Unretained(listener_), SamplesToString(), error));
       break;
     case Command::RECORD_CLOCK_SYNC_MARKER:
       base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::Bind(&Listener::OnRecordClockSyncMarkerComplete,
                                 base::Unretained(listener_), error));
       break;
@@ skipping 44 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