[BattOr] Make BattOr able to return firmware version.

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;
@@ skipping 86 matching lines @@
   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;
 }
 
+bool ParseGitHashFrame(BattOrMessageType type,

[charliea (OOO until 10-5), 2016/10/18 23:46:29]

I think it's probably okay to do this inline in OnMessageRead() rather than in a
separate function. The only reason that I ended up creating a separate function
for ParseSampleFrame is because the logic is so extensive for that.

[charliea (OOO until 10-5), 2016/10/18 23:46:29]

I think that, if you use an std::string, this entire translation can be written
as:

return std::string(msg.begin(), msg.end());

My reasoning is that strings have a constructor that accepts a begin and end
iterator, and a vector provides that.

[rnephew (Reviews Here), 2016/10/19 18:23:47]

Done.

+                       const vector<char>& msg,
+                       char* git_hash){
+  if (type !=  BATTOR_MESSAGE_TYPE_CONTROL_ACK)
+    return false;
+  memcpy(git_hash, msg.data(), msg.size());
+  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,
@@ skipping 31 matching lines @@
 }
 
 void BattOrAgent::RecordClockSyncMarker(const std::string& marker) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   command_ = Command::RECORD_CLOCK_SYNC_MARKER;
   pending_clock_sync_marker_ = marker;
   PerformAction(Action::REQUEST_CONNECTION);
 }
 
+void BattOrAgent::GetVersion() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  command_ = Command::GET_VERSION;
+  PerformAction(Action::REQUEST_CONNECTION);
+}
+
 void BattOrAgent::BeginConnect() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   connection_->Open();
 }
 
 void BattOrAgent::OnConnectionOpened(bool success) {
   // Return immediately if opening the connection already timed out.
   if (timeout_callback_.IsCancelled())
     return;
   timeout_callback_.Cancel();
 
   if (!success) {
     CompleteCommand(BATTOR_ERROR_CONNECTION_FAILED);
     return;
   }
 
   switch (command_) {
     case Command::START_TRACING:
       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::GET_VERSION:
+      PerformAction(Action::SEND_GIT_HASH_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.
@@ skipping 21 matching lines @@
       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;
+    case Action::SEND_GIT_HASH_REQUEST:
+      num_read_attempts_ = 1;
+      PerformAction(Action::READ_GIT_HASH);
+      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_GIT_HASH:
       case Action::READ_EEPROM:
       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(
@@ skipping 123 matching lines @@
         return;
       }
 
       uint32_t sample_num;
       memcpy(&sample_num, bytes->data(), sizeof(uint32_t));
       clock_sync_markers_[sample_num] = pending_clock_sync_marker_;
       last_clock_sync_time_ = base::TimeTicks::Now();
       CompleteCommand(BATTOR_ERROR_NONE);
       return;
 
+    case Action::READ_GIT_HASH:
+      // Get size to allocate space for git_hash_.
+      if (bytes->size() <= 0 ){

[charliea (OOO until 10-5), 2016/10/18 23:46:29]

Also don't think this should be necessary once we use the iterator-based
constructor.

[rnephew (Reviews Here), 2016/10/19 18:23:47]

Done.

+        CompleteCommand(BATTOR_ERROR_UNEXPECTED_MESSAGE);
+        return;
+      }
+      git_hash_ = (char*) malloc(bytes->size());
+
+      if(!ParseGitHashFrame(type, *bytes, git_hash_)) {
+        CompleteCommand(BATTOR_ERROR_UNEXPECTED_MESSAGE);
+        return;
+      }
+      CompleteCommand(BATTOR_ERROR_NONE);
+      return;
+
     default:
       CompleteCommand(BATTOR_ERROR_UNEXPECTED_MESSAGE);
   }
 }
 
 void BattOrAgent::PerformAction(Action action) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   timeout_callback_.Reset(
       base::Bind(&BattOrAgent::OnActionTimeout, AsWeakPtr()));
@@ skipping 66 matching lines @@
       return;
 
     // The following actions are required for RecordClockSyncMarker:
     case Action::SEND_CURRENT_SAMPLE_REQUEST:
       SendControlMessage(BATTOR_CONTROL_MESSAGE_TYPE_READ_SAMPLE_COUNT, 0, 0);
       return;
     case Action::READ_CURRENT_SAMPLE:
       connection_->ReadMessage(BATTOR_MESSAGE_TYPE_CONTROL_ACK);
       return;
 
+    case Action::SEND_GIT_HASH_REQUEST:
+      SendControlMessage(BATTOR_CONTROL_MESSAGE_TYPE_GET_GIT_HASH, 0, 0);
+      return;
+
+    case Action::READ_GIT_HASH:
+      connection_->ReadMessage(BATTOR_MESSAGE_TYPE_CONTROL_ACK);
+      return;
+
     case Action::INVALID:
       NOTREACHED();
   }
 }
 
 void BattOrAgent::PerformDelayedAction(Action action, base::TimeDelta delay) {
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE, base::Bind(&BattOrAgent::PerformAction, AsWeakPtr(), action),
       delay);
 }
@@ skipping 37 matching lines @@
       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;
+    case Command::GET_VERSION:
+      base::ThreadTaskRunnerHandle::Get()->PostTask(
+          FROM_HERE, base::Bind(&Listener::OnGetVersionComplete,
+                                base::Unretained(listener_),
+                                git_hash_, error));
+      break;
     case Command::INVALID:
       NOTREACHED();
   }
 
   last_action_ = Action::INVALID;
   command_ = Command::INVALID;
   pending_clock_sync_marker_.clear();
   battor_eeprom_.reset();
   calibration_frame_.clear();
   samples_.clear();
@@ skipping 34 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