Substituting legacy protocol with uWeave protocol

components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection.cc

Index: components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection.cc
diff --git a/components/proximity_auth/ble/bluetooth_low_energy_connection.cc b/components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection.cc
similarity index 51%
copy from components/proximity_auth/ble/bluetooth_low_energy_connection.cc
copy to components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection.cc
index af6474948da29edac7217010dc358e5315bdb71c..9bf4b43604418eb8643cc44b77e2979b623f4e6e 100644
--- a/components/proximity_auth/ble/bluetooth_low_energy_connection.cc
+++ b/components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection.cc
@@ -1,30 +1,20 @@
-// Copyright 2015 The Chromium Authors. All rights reserved.
+// Copyright 2016 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 "components/proximity_auth/ble/bluetooth_low_energy_connection.h"
+#include "components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/location.h"
-#include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
 #include "base/task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
-#include "base/time/time.h"
-#include "components/proximity_auth/ble/bluetooth_low_energy_characteristics_finder.h"
-#include "components/proximity_auth/ble/fake_wire_message.h"
 #include "components/proximity_auth/bluetooth_throttler.h"
 #include "components/proximity_auth/connection_finder.h"
 #include "components/proximity_auth/logging/logging.h"
 #include "components/proximity_auth/wire_message.h"
-#include "device/bluetooth/bluetooth_adapter.h"
-#include "device/bluetooth/bluetooth_device.h"
 #include "device/bluetooth/bluetooth_gatt_connection.h"
-#include "device/bluetooth/bluetooth_gatt_notify_session.h"
-#include "device/bluetooth/bluetooth_remote_gatt_characteristic.h"
-#include "device/bluetooth/bluetooth_uuid.h"
 
 using device::BluetoothAdapter;
 using device::BluetoothDevice;
@@ -35,41 +25,41 @@ using device::BluetoothGattNotifySession;
 using device::BluetoothUUID;
 
 namespace proximity_auth {
+namespace weave {
 namespace {
 
-// The UUID of the characteristic used to send data to the peripheral.
-const char kToPeripheralCharUUID[] = "977c6674-1239-4e72-993b-502369b8bb5a";
+typedef BluetoothLowEnergyWeavePacketReceiver::State ReceiverState;
 
-// The UUID of the characteristic used to receive data from the peripheral.
-const char kFromPeripheralCharUUID[] = "f4b904a2-a030-43b3-98a8-221c536c03cb";
+// The UUID of the TX characteristic used to transmit data to the server.
+const char kTXCharacteristicUUID[] = "00000100-0004-1000-8000-001A11000101";
 
-// Deprecated signal send as the first byte in send byte operations.
-const int kFirstByteZero = 0;
+// The UUID of the RX characteristic used to receive data from the server.
+const char kRXCharacteristicUUID[] = "00000100-0004-1000-8000-001A11000102";
 
-// The maximum number of bytes written in a remote characteristic with a single
-// write request. This is not the connection MTU, we are assuming that the
-// remote device allows for writes larger than MTU.
-const int kMaxChunkSize = 500;
 }  // namespace
 
-BluetoothLowEnergyConnection::BluetoothLowEnergyConnection(
-    const RemoteDevice& device,
-    scoped_refptr<device::BluetoothAdapter> adapter,
-    const BluetoothUUID remote_service_uuid,
-    BluetoothThrottler* bluetooth_throttler,
-    int max_number_of_write_attempts)
+BluetoothLowEnergyWeaveClientConnection::
+    BluetoothLowEnergyWeaveClientConnection(
+        const RemoteDevice& device,
+        scoped_refptr<device::BluetoothAdapter> adapter,
+        const BluetoothUUID remote_service_uuid,
+        BluetoothThrottler* bluetooth_throttler,
+        int max_number_of_write_attempts)
     : Connection(device),
       adapter_(adapter),
       remote_service_({remote_service_uuid, ""}),
-      to_peripheral_char_({BluetoothUUID(kToPeripheralCharUUID), ""}),
-      from_peripheral_char_({BluetoothUUID(kFromPeripheralCharUUID), ""}),
+      packet_generator_(
+          BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance()),
+      packet_receiver_(
+          BluetoothLowEnergyWeavePacketReceiver::Factory::NewInstance(
+              BluetoothLowEnergyWeavePacketReceiver::ReceiverType::CLIENT)),
+      tx_characteristic_({BluetoothUUID(kTXCharacteristicUUID), ""}),
+      rx_characteristic_({BluetoothUUID(kRXCharacteristicUUID), ""}),
       bluetooth_throttler_(bluetooth_throttler),
       task_runner_(base::ThreadTaskRunnerHandle::Get()),
       sub_status_(SubStatus::DISCONNECTED),
-      receiving_bytes_(false),
       write_remote_characteristic_pending_(false),
       max_number_of_write_attempts_(max_number_of_write_attempts),
-      max_chunk_size_(kMaxChunkSize),
       weak_ptr_factory_(this) {
   DCHECK(adapter_);
   DCHECK(adapter_->IsInitialized());
@@ -77,7 +67,8 @@ BluetoothLowEnergyConnection::BluetoothLowEnergyConnection(
   adapter_->AddObserver(this);
 }
 
-BluetoothLowEnergyConnection::~BluetoothLowEnergyConnection() {
+BluetoothLowEnergyWeaveClientConnection::
+    ~BluetoothLowEnergyWeaveClientConnection() {
   Disconnect();
   if (adapter_) {
     adapter_->RemoveObserver(this);
@@ -85,7 +76,7 @@ BluetoothLowEnergyConnection::~BluetoothLowEnergyConnection() {
   }
 }
 
-void BluetoothLowEnergyConnection::Connect() {
+void BluetoothLowEnergyWeaveClientConnection::Connect() {
   DCHECK(sub_status() == SubStatus::DISCONNECTED);
 
   SetSubStatus(SubStatus::WAITING_GATT_CONNECTION);
@@ -101,8 +92,9 @@ void BluetoothLowEnergyConnection::Connect() {
   if (!throttler_delay.is_zero()) {
     task_runner_->PostDelayedTask(
         FROM_HERE,
-        base::Bind(&BluetoothLowEnergyConnection::CreateGattConnection,
-                   weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(
+            &BluetoothLowEnergyWeaveClientConnection::CreateGattConnection,
+            weak_ptr_factory_.GetWeakPtr()),
         throttler_delay);
     return;
   }
@@ -110,7 +102,7 @@ void BluetoothLowEnergyConnection::Connect() {
   CreateGattConnection();
 }
 
-void BluetoothLowEnergyConnection::CreateGattConnection() {
+void BluetoothLowEnergyWeaveClientConnection::CreateGattConnection() {
   DCHECK(sub_status() == SubStatus::WAITING_GATT_CONNECTION);
 
   BluetoothDevice* remote_device = GetRemoteDevice();
@@ -119,14 +111,16 @@ void BluetoothLowEnergyConnection::CreateGattConnection() {
                  << remote_device->GetAddress();
 
     remote_device->CreateGattConnection(
-        base::Bind(&BluetoothLowEnergyConnection::OnGattConnectionCreated,
-                   weak_ptr_factory_.GetWeakPtr()),
-        base::Bind(&BluetoothLowEnergyConnection::OnCreateGattConnectionError,
+        base::Bind(
+            &BluetoothLowEnergyWeaveClientConnection::OnGattConnectionCreated,
+            weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(&BluetoothLowEnergyWeaveClientConnection::
+                       OnCreateGattConnectionError,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 }
 
-void BluetoothLowEnergyConnection::Disconnect() {
+void BluetoothLowEnergyWeaveClientConnection::Disconnect() {
   if (sub_status() != SubStatus::DISCONNECTED) {
     weak_ptr_factory_.InvalidateWeakPtrs();
     StopNotifySession();
@@ -146,78 +140,45 @@ void BluetoothLowEnergyConnection::Disconnect() {
   }
 }
 
-void BluetoothLowEnergyConnection::SetSubStatus(SubStatus new_sub_status) {
+void BluetoothLowEnergyWeaveClientConnection::SetSubStatus(
+    SubStatus new_sub_status) {
   sub_status_ = new_sub_status;
 
   // Sets the status of parent class proximity_auth::Connection accordingly.
   if (new_sub_status == SubStatus::CONNECTED) {
-    SetStatus(CONNECTED);
+    SetStatus(Status::CONNECTED);
   } else if (new_sub_status == SubStatus::DISCONNECTED) {
-    SetStatus(DISCONNECTED);
+    SetStatus(Status::DISCONNECTED);
   } else {
-    SetStatus(IN_PROGRESS);
+    SetStatus(Status::IN_PROGRESS);
   }
 }
 
-void BluetoothLowEnergyConnection::SetTaskRunnerForTesting(
+void BluetoothLowEnergyWeaveClientConnection::SetTaskRunnerForTesting(
     scoped_refptr<base::TaskRunner> task_runner) {
   task_runner_ = task_runner;
 }
 
-void BluetoothLowEnergyConnection::SendMessageImpl(
+void BluetoothLowEnergyWeaveClientConnection::SendMessageImpl(
     std::unique_ptr<WireMessage> message) {
   PA_LOG(INFO) << "Sending message " << message->Serialize();
   std::string serialized_msg = message->Serialize();
 
-  // [First write]: Build a header with the [send signal] + [size of the
-  // message].
-  WriteRequest write_request = BuildWriteRequest(
-      ToByteVector(static_cast<uint32_t>(ControlSignal::kSendSignal)),
-      ToByteVector(static_cast<uint32_t>(serialized_msg.size())), false);
-
-  // [First write]: Fill the it with a prefix of |serialized_msg| up to
-  // |max_chunk_size_|.
-  size_t first_chunk_size = std::min(
-      max_chunk_size_ - write_request.value.size(), serialized_msg.size());
-  std::vector<uint8_t> bytes(serialized_msg.begin(),
-                             serialized_msg.begin() + first_chunk_size);
-  write_request.value.insert(write_request.value.end(), bytes.begin(),
-                             bytes.end());
-
-  bool is_last_write_request = first_chunk_size == serialized_msg.size();
-  write_request.is_last_write_for_wire_message = is_last_write_request;
-  WriteRemoteCharacteristic(write_request);
-  if (is_last_write_request)
-    return;
+  std::vector<Packet> packets =
+      packet_generator_->EncodeDataMessage(serialized_msg);
 
-  // [Other write requests]: Each chunk has to include a deprecated signal:
-  // |kFirstByteZero| as the first byte.
-  int chunk_size = max_chunk_size_ - 1;
-  std::vector<uint8_t> kFirstByteZeroVector;
-  kFirstByteZeroVector.push_back(static_cast<uint8_t>(kFirstByteZero));
-
-  int message_size = static_cast<int>(serialized_msg.size());
-  int start_index = first_chunk_size;
-  while (start_index < message_size) {
-    int end_index = (start_index + chunk_size) <= message_size
-                        ? (start_index + chunk_size)
-                        : message_size;
-    bool is_last_write_request = (end_index == message_size);
-    write_request = BuildWriteRequest(
-        kFirstByteZeroVector,
-        std::vector<uint8_t>(serialized_msg.begin() + start_index,
-                             serialized_msg.begin() + end_index),
-        is_last_write_request);
-    WriteRemoteCharacteristic(write_request);
-    start_index = end_index;
+  for (uint32_t i = 0; i < packets.size(); ++i) {
+    WriteRequest request = WriteRequest(packets[i], i == packets.size() - 1);
+    WriteRemoteCharacteristic(request);
   }
 }
 
 // Changes in the GATT connection with the remote device should be observed
 // here. If the GATT connection is dropped, we should call Disconnect() anyway,
 // so the object can notify its observers.
-void BluetoothLowEnergyConnection::DeviceChanged(BluetoothAdapter* adapter,
-                                                 BluetoothDevice* device) {
+void BluetoothLowEnergyWeaveClientConnection::DeviceChanged(
+    BluetoothAdapter* adapter,
+    BluetoothDevice* device) {
   DCHECK(device);
   if (sub_status() == SubStatus::DISCONNECTED ||
       device->GetAddress() != GetDeviceAddress())
@@ -234,8 +195,9 @@ void BluetoothLowEnergyConnection::DeviceChanged(BluetoothAdapter* adapter,
   }
 }
 
-void BluetoothLowEnergyConnection::DeviceRemoved(BluetoothAdapter* adapter,
-                                                 BluetoothDevice* device) {
+void BluetoothLowEnergyWeaveClientConnection::DeviceRemoved(
+    BluetoothAdapter* adapter,
+    BluetoothDevice* device) {
   DCHECK(device);
   if (sub_status_ == SubStatus::DISCONNECTED ||
       device->GetAddress() != GetDeviceAddress())
@@ -245,91 +207,74 @@ void BluetoothLowEnergyConnection::DeviceRemoved(BluetoothAdapter* adapter,
   Disconnect();
 }
 
-void BluetoothLowEnergyConnection::GattCharacteristicValueChanged(
+void BluetoothLowEnergyWeaveClientConnection::GattCharacteristicValueChanged(
     BluetoothAdapter* adapter,
     BluetoothRemoteGattCharacteristic* characteristic,
-    const std::vector<uint8_t>& value) {
+    const Packet& value) {
   DCHECK_EQ(adapter, adapter_.get());
-  if (sub_status() != SubStatus::WAITING_RESPONSE_SIGNAL &&
+  if (sub_status() != SubStatus::WAITING_CONNECTION_RESPONSE &&
       sub_status() != SubStatus::CONNECTED)
     return;
 
   PA_LOG(INFO) << "Characteristic value changed: "
                << characteristic->GetUUID().canonical_value();
 
-  if (characteristic->GetIdentifier() == from_peripheral_char_.id) {
-    if (receiving_bytes_) {
-      // Ignoring the first byte, as it contains a deprecated signal.
-      const std::string bytes(value.begin() + 1, value.end());
-      incoming_bytes_buffer_.append(bytes);
-      if (incoming_bytes_buffer_.size() >= expected_number_of_incoming_bytes_) {
-        OnBytesReceived(incoming_bytes_buffer_);
-        receiving_bytes_ = false;
-      }
-      return;
-    }
-
-    if (value.size() < 4) {
-      PA_LOG(WARNING) << "Incoming data corrupted, no signal found.";
-      return;
-    }
-
-    const ControlSignal signal = static_cast<ControlSignal>(ToUint32(value));
-    switch (signal) {
-      case ControlSignal::kInvitationResponseSignal:
-        if (sub_status() == SubStatus::WAITING_RESPONSE_SIGNAL)
-          CompleteConnection();
-        break;
-      case ControlSignal::kInviteToConnectSignal:
+  if (characteristic->GetIdentifier() == rx_characteristic_.id) {
+    ReceiverState state = packet_receiver_->ReceivePacket(value);
+    PA_LOG(INFO) << "\nReceiver State: " << state;
+    switch (state) {
+      case ReceiverState::DATA_READY:
+        OnBytesReceived(packet_receiver_->GetDataMessage());
         break;
-      case ControlSignal::kSendSignal: {
-        if (value.size() < 8) {
-          PA_LOG(WARNING)
-              << "Incoming data corrupted, expected message size not found.";
-          return;
-        }
-        std::vector<uint8_t> size(value.begin() + 4, value.begin() + 8);
-        expected_number_of_incoming_bytes_ =
-            static_cast<size_t>(ToUint32(size));
-        receiving_bytes_ = true;
-        incoming_bytes_buffer_.clear();
-
-        const std::string bytes(value.begin() + 8, value.end());
-        incoming_bytes_buffer_.append(bytes);
-        if (incoming_bytes_buffer_.size() >=
-            expected_number_of_incoming_bytes_) {
-          OnBytesReceived(incoming_bytes_buffer_);
-          receiving_bytes_ = false;
-        }
+      case ReceiverState::CONNECTION_CLOSED:
+        PA_LOG(ERROR) << "Connection closed due to: "
+                      << packet_receiver_->GetReasonForClose();
+        Disconnect();
         break;
-      }
-      case ControlSignal::kDisconnectSignal:
-        PA_LOG(INFO) << "Disconnect signal received.";
+      case ReceiverState::ERROR_DETECTED:
+        // TODO(jingxuy): test this once the design has been confirmed
+        WriteConnectionClose(packet_generator_->CreateConnectionClose(
+            packet_receiver_->GetReasonToClose()));
         Disconnect();
         break;
+      case ReceiverState::WAITING:
+        // Receiver state should have changed from CONNECTING to WAITING if
+        // a proper connection response had been received.
+        // The max packet size selected from the connection response will be
+        // used to generate future data packets.
+        packet_generator_->SetMaxPacketSize(
+            packet_receiver_->GetMaxPacketSize());
+        if (sub_status() == SubStatus::WAITING_CONNECTION_RESPONSE)

[Kyle Horimoto, 2016/06/30 01:29:48]

Should this be an assertion? Is there ever a valid time in which this would not
be the case?

[jingxuy, 2016/06/30 21:59:25]

Done.

+          CompleteConnection();
+        break;
+      case ReceiverState::RECEIVING_DATA:
+        // Normal in between states, so do nothing.
+        break;
+      default:
+        NOTREACHED();
     }
   }
 }
 
-BluetoothLowEnergyConnection::WriteRequest::WriteRequest(
-    const std::vector<uint8_t>& val,
+BluetoothLowEnergyWeaveClientConnection::WriteRequest::WriteRequest(
+    const Packet& val,
     bool flag)
     : value(val),
       is_last_write_for_wire_message(flag),
       number_of_failed_attempts(0) {}
 
-BluetoothLowEnergyConnection::WriteRequest::WriteRequest(
+BluetoothLowEnergyWeaveClientConnection::WriteRequest::WriteRequest(
     const WriteRequest& other) = default;
 
-BluetoothLowEnergyConnection::WriteRequest::~WriteRequest() {}
+BluetoothLowEnergyWeaveClientConnection::WriteRequest::~WriteRequest() {}
 
-void BluetoothLowEnergyConnection::CompleteConnection() {
+void BluetoothLowEnergyWeaveClientConnection::CompleteConnection() {
   PA_LOG(INFO) << "Connection completed. Time elapsed: "
                << base::TimeTicks::Now() - start_time_;
   SetSubStatus(SubStatus::CONNECTED);
 }
 
-void BluetoothLowEnergyConnection::OnCreateGattConnectionError(
+void BluetoothLowEnergyWeaveClientConnection::OnCreateGattConnectionError(
     device::BluetoothDevice::ConnectErrorCode error_code) {
   DCHECK(sub_status_ == SubStatus::WAITING_GATT_CONNECTION);
   PA_LOG(WARNING) << "Error creating GATT connection to "
@@ -338,7 +283,7 @@ void BluetoothLowEnergyConnection::OnCreateGattConnectionError(
   Disconnect();
 }
 
-void BluetoothLowEnergyConnection::OnGattConnectionCreated(
+void BluetoothLowEnergyWeaveClientConnection::OnGattConnectionCreated(
     std::unique_ptr<device::BluetoothGattConnection> gatt_connection) {
   DCHECK(sub_status() == SubStatus::WAITING_GATT_CONNECTION);
   PA_LOG(INFO) << "GATT connection with " << gatt_connection->GetDeviceAddress()
@@ -351,59 +296,62 @@ void BluetoothLowEnergyConnection::OnGattConnectionCreated(
   gatt_connection_ = std::move(gatt_connection);
   SetSubStatus(SubStatus::WAITING_CHARACTERISTICS);
   characteristic_finder_.reset(CreateCharacteristicsFinder(
-      base::Bind(&BluetoothLowEnergyConnection::OnCharacteristicsFound,
-                 weak_ptr_factory_.GetWeakPtr()),
-      base::Bind(&BluetoothLowEnergyConnection::OnCharacteristicsFinderError,
+      base::Bind(
+          &BluetoothLowEnergyWeaveClientConnection::OnCharacteristicsFound,
+          weak_ptr_factory_.GetWeakPtr()),
+      base::Bind(&BluetoothLowEnergyWeaveClientConnection::
+                     OnCharacteristicsFinderError,
                  weak_ptr_factory_.GetWeakPtr())));
 }
 
 BluetoothLowEnergyCharacteristicsFinder*
-BluetoothLowEnergyConnection::CreateCharacteristicsFinder(
+BluetoothLowEnergyWeaveClientConnection::CreateCharacteristicsFinder(
     const BluetoothLowEnergyCharacteristicsFinder::SuccessCallback&
         success_callback,
     const BluetoothLowEnergyCharacteristicsFinder::ErrorCallback&
         error_callback) {
   return new BluetoothLowEnergyCharacteristicsFinder(
-      adapter_, GetRemoteDevice(), remote_service_, to_peripheral_char_,
-      from_peripheral_char_, success_callback, error_callback);
+      adapter_, GetRemoteDevice(), remote_service_, tx_characteristic_,
+      rx_characteristic_, success_callback, error_callback);
 }
 
-void BluetoothLowEnergyConnection::OnCharacteristicsFound(
+void BluetoothLowEnergyWeaveClientConnection::OnCharacteristicsFound(
     const RemoteAttribute& service,
-    const RemoteAttribute& to_peripheral_char,
-    const RemoteAttribute& from_peripheral_char) {
+    const RemoteAttribute& tx_characteristic,
+    const RemoteAttribute& rx_characteristic) {
   PA_LOG(INFO) << "Remote chacteristics found.";
   PrintTimeElapsed();
 
   DCHECK(sub_status() == SubStatus::WAITING_CHARACTERISTICS);
   remote_service_ = service;
-  to_peripheral_char_ = to_peripheral_char;
-  from_peripheral_char_ = from_peripheral_char;
+  tx_characteristic_ = tx_characteristic;
+  rx_characteristic_ = rx_characteristic;
 
   SetSubStatus(SubStatus::CHARACTERISTICS_FOUND);
   StartNotifySession();
 }
 
-void BluetoothLowEnergyConnection::OnCharacteristicsFinderError(
-    const RemoteAttribute& to_peripheral_char,
-    const RemoteAttribute& from_peripheral_char) {
+void BluetoothLowEnergyWeaveClientConnection::OnCharacteristicsFinderError(
+    const RemoteAttribute& tx_characteristic,
+    const RemoteAttribute& rx_characteristic) {
   DCHECK(sub_status() == SubStatus::WAITING_CHARACTERISTICS);
   PA_LOG(WARNING) << "Connection error, missing characteristics for SmartLock "
                      "service.\n"
-                  << (to_peripheral_char.id.empty()
-                          ? to_peripheral_char.uuid.canonical_value()
+                  << (tx_characteristic.id.empty()
+                          ? tx_characteristic.uuid.canonical_value()
+                          : "")
+                  << (rx_characteristic.id.empty()
+                          ? ", " + rx_characteristic.uuid.canonical_value()
                           : "")
-                  << (from_peripheral_char.id.empty()
-                          ? ", " + from_peripheral_char.uuid.canonical_value()
-                          : "") << " not found.";
+                  << " not found.";
 
   Disconnect();
 }
 
-void BluetoothLowEnergyConnection::StartNotifySession() {
+void BluetoothLowEnergyWeaveClientConnection::StartNotifySession() {
   if (sub_status() == SubStatus::CHARACTERISTICS_FOUND) {
     BluetoothRemoteGattCharacteristic* characteristic =
-        GetGattCharacteristic(from_peripheral_char_.id);
+        GetGattCharacteristic(rx_characteristic_.id);
     DCHECK(characteristic);
 
     // This is a workaround for crbug.com/507325. If |characteristic| is already
@@ -413,27 +361,22 @@ void BluetoothLowEnergyConnection::StartNotifySession() {
       PA_LOG(INFO) << characteristic->GetUUID().canonical_value()
                    << " already notifying.";
       SetSubStatus(SubStatus::NOTIFY_SESSION_READY);
-      SendInviteToConnectSignal();
+      SendConnectionRequest();
       return;
     }
 
     SetSubStatus(SubStatus::WAITING_NOTIFY_SESSION);
     characteristic->StartNotifySession(
-        base::Bind(&BluetoothLowEnergyConnection::OnNotifySessionStarted,
-                   weak_ptr_factory_.GetWeakPtr()),
-        base::Bind(&BluetoothLowEnergyConnection::OnNotifySessionError,
-                   weak_ptr_factory_.GetWeakPtr()));
+        base::Bind(
+            &BluetoothLowEnergyWeaveClientConnection::OnNotifySessionStarted,
+            weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(
+            &BluetoothLowEnergyWeaveClientConnection::OnNotifySessionError,
+            weak_ptr_factory_.GetWeakPtr()));
   }
 }
 
-void BluetoothLowEnergyConnection::OnNotifySessionError(
-    BluetoothRemoteGattService::GattErrorCode error) {
-  DCHECK(sub_status() == SubStatus::WAITING_NOTIFY_SESSION);
-  PA_LOG(WARNING) << "Error starting notification session: " << error;
-  Disconnect();
-}
-
-void BluetoothLowEnergyConnection::OnNotifySessionStarted(
+void BluetoothLowEnergyWeaveClientConnection::OnNotifySessionStarted(
     std::unique_ptr<BluetoothGattNotifySession> notify_session) {
   DCHECK(sub_status() == SubStatus::WAITING_NOTIFY_SESSION);
   PA_LOG(INFO) << "Notification session started "
@@ -443,57 +386,101 @@ void BluetoothLowEnergyConnection::OnNotifySessionStarted(
   SetSubStatus(SubStatus::NOTIFY_SESSION_READY);
   notify_session_ = std::move(notify_session);
 
-  SendInviteToConnectSignal();
+  SendConnectionRequest();
+}
+
+void BluetoothLowEnergyWeaveClientConnection::OnNotifySessionError(
+    BluetoothRemoteGattService::GattErrorCode error) {
+  DCHECK(sub_status() == SubStatus::WAITING_NOTIFY_SESSION);
+  PA_LOG(WARNING) << "Error starting notification session: " << error;
+  Disconnect();
 }
 
-void BluetoothLowEnergyConnection::StopNotifySession() {
+void BluetoothLowEnergyWeaveClientConnection::StopNotifySession() {
   if (notify_session_) {
     notify_session_->Stop(base::Bind(&base::DoNothing));
     notify_session_.reset();
   }
 }
 
-void BluetoothLowEnergyConnection::SendInviteToConnectSignal() {
+void BluetoothLowEnergyWeaveClientConnection::SendConnectionRequest() {
   if (sub_status() == SubStatus::NOTIFY_SESSION_READY) {
-    PA_LOG(INFO) << "Sending invite to connect signal";
-    SetSubStatus(SubStatus::WAITING_RESPONSE_SIGNAL);
+    PA_LOG(INFO) << "Sending connection request to the server";
+    SetSubStatus(SubStatus::WAITING_CONNECTION_RESPONSE);
 
-    WriteRequest write_request = BuildWriteRequest(
-        ToByteVector(
-            static_cast<uint32_t>(ControlSignal::kInviteToConnectSignal)),
-        std::vector<uint8_t>(), false);
+    WriteRequest write_request =
+        WriteRequest(packet_generator_->CreateConnectionRequest(), false);
 
     WriteRemoteCharacteristic(write_request);
   }
 }
 
-void BluetoothLowEnergyConnection::WriteRemoteCharacteristic(
-    WriteRequest request) {
+void BluetoothLowEnergyWeaveClientConnection::WriteConnectionClose(
+    const Packet& connection_close) {
+  // TODO(jingxuy): I know this is single threaded, but can it actually still

[Kyle Horimoto, 2016/06/30 00:42:33]

Please don't make two separate paths for this. This is unneeded. Just call
WriteRemoteCharacteristic and send the queued messages in order.

You'll need to handle the disconnect specially, so just add an extra
"disconnect_after_sending" boolean to WriteRequest, then in
OnRemoteCharacteristicWritten(), look at the message that was just written and
check that value. If it is true, then Disconnect().

[jingxuy, 2016/06/30 01:14:55]

This is done because I am presuming that you want to close the connection
immediately. As in that you shouldn't wait for all the writes in the queue to
finish sending once you observe an error. Unless we do want to wait on queue,
I'm not sure how much code reuse can be achieved through reusing
writeRemoteCharacteristic

[Kyle Horimoto, 2016/06/30 02:01:25]

All writes that have been queued should still be written. Otherwise, the remote
device may not receive all of the data it should have. Any writes that are
queued *after* the close connection is queued should not be sent, but any writes
that are queued *before* it is queued should be sent.

For example, consider this case where a client wants to open a connection, send
a large, 100-packet message, then close the connection:

1) Client sends connection request.
2) Server responds with connection response.
3) Client starts sending the large message. The 100 packets are queued, but only
5 have been sent so far.
4) Client enqueues the connection close packet to be sent after all 100 packets
are sent.

In your solution, only 5 packets would be sent, then the close connection packet
would be sent. The server would receive this data and only have 5% of it, so it
wouldn't understand what was actually sent.

Likewise, maybe some other app is utilizing the Bluetooth hardware, so maybe
even the first packet hasn't been sent yet before the connection close packet is
queued. There are many other possibilities in which your implementation would
cause errors.

[jingxuy, 2016/06/30 21:59:25]

I think the misunderstanding is that this connection close is used to kill a
connection in a non-friendly manner. This is not we are done with the
connection, so no more is needed. This is when the client receiver jumped into
error state, which means the server is probably in some weird and non-functional
state as well. It does bring up the point that currently there is no way of
ending the connection in a friendly manner.

[Kyle Horimoto, 2016/06/30 22:36:11]

The connection close packet can also used to close the connection if no error
has occurred. There is even a "reason for close" value called "close without
error". IMO, when Disconnect() is called, we should send a connection close
packet assuming that the following are true:

1) We are the ones initiating the connection being closed (i.e., we have not
received a connection close packet from the remote device).
2) The Bluetooth connection is still valid (i.e., we are able to send things
still).
3) We haven't already sent a connection close packet.

Then, we would do the rest of the disconnection cleanup.

Tim/Gustavo/Mihai, what do you think?

In any case, we should still try to send the rest of the packets that were
already queued for the reasons I said earlier.

[jingxuy, 2016/07/01 00:00:44]

Yes, I think queuing is the proper way to send a normal connection close without
error. It is actually yet not implemented as I mentioned in the last comment,
will be a TODO. (Currently it seems like Disconnect() is the only way to stop a
connection). But this writeConnectionClose is actually only called on error. So
I might want to change name later so there's no confusion.

[sacomoto, 2016/07/06 13:07:07]

This strategy is correct. The close connection packet should be sent (and queued
like any other packet) when this class initiates a clean disconnect (no error).

In the case of a disconnect after an error (e.g. a packet with the wrong packet
counter was received) the close connect packet should be sent right away (no
queueing).

You should probably implement an InternalDisconnect() method, that does exactly
what the public Disconnect() does now. And change the implementation of
Disconnect() to send a connection close packet and then call the
InternalDisconnect().

[jingxuy, 2016/07/07 04:20:57]

Done.

+  // have arbitrary overlay? If so, then this queue clear can race with
+  // ProcessNextWriteRequest.
+  std::queue<WriteRequest> empty;
+  std::swap(write_requests_queue_, empty);
+
+  // Spin wait on the current pending one unless overwritting is okay.
+  while (write_remote_characteristic_pending_) {
+  };
+
+  BluetoothRemoteGattCharacteristic* characteristic =
+      GetGattCharacteristic(tx_characteristic_.id);
+
+  characteristic->WriteRemoteCharacteristic(
+      connection_close,
+      base::Bind(
+          &BluetoothLowEnergyWeaveClientConnection::OnConnectionCloseWritten,
+          weak_ptr_factory_.GetWeakPtr()),
+      base::Bind(
+          &BluetoothLowEnergyWeaveClientConnection::OnConnectionCloseError,
+          weak_ptr_factory_.GetWeakPtr()));
+}
+
+void BluetoothLowEnergyWeaveClientConnection::OnConnectionCloseWritten() {
+  PA_LOG(INFO) << "Connection close written.";
+  Disconnect();
+}
+
+void BluetoothLowEnergyWeaveClientConnection::OnConnectionCloseError(
+    BluetoothRemoteGattService::GattErrorCode error) {
+  PA_LOG(INFO) << "Failed to write connection close due to GATT error: "
+               << error;
+  Disconnect();
+}
+
+void BluetoothLowEnergyWeaveClientConnection::WriteRemoteCharacteristic(
+    const WriteRequest& request) {
   write_requests_queue_.push(request);
   ProcessNextWriteRequest();
 }
 
-void BluetoothLowEnergyConnection::ProcessNextWriteRequest() {
+void BluetoothLowEnergyWeaveClientConnection::ProcessNextWriteRequest() {
   BluetoothRemoteGattCharacteristic* characteristic =
-      GetGattCharacteristic(to_peripheral_char_.id);
+      GetGattCharacteristic(tx_characteristic_.id);
   if (!write_requests_queue_.empty() && !write_remote_characteristic_pending_ &&
       characteristic) {
     write_remote_characteristic_pending_ = true;
     WriteRequest next_request = write_requests_queue_.front();
+
     PA_LOG(INFO) << "Writing characteristic...";
     characteristic->WriteRemoteCharacteristic(
         next_request.value,
-        base::Bind(&BluetoothLowEnergyConnection::OnRemoteCharacteristicWritten,
+        base::Bind(&BluetoothLowEnergyWeaveClientConnection::
+                       OnRemoteCharacteristicWritten,
                    weak_ptr_factory_.GetWeakPtr(),
                    next_request.is_last_write_for_wire_message),
-        base::Bind(
-            &BluetoothLowEnergyConnection::OnWriteRemoteCharacteristicError,
-            weak_ptr_factory_.GetWeakPtr(),
-            next_request.is_last_write_for_wire_message));
+        base::Bind(&BluetoothLowEnergyWeaveClientConnection::
+                       OnWriteRemoteCharacteristicError,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   next_request.is_last_write_for_wire_message));
   }
 }
 
-void BluetoothLowEnergyConnection::OnRemoteCharacteristicWritten(
+void BluetoothLowEnergyWeaveClientConnection::OnRemoteCharacteristicWritten(
     bool run_did_send_message_callback) {
   PA_LOG(INFO) << "Characteristic written.";
   write_remote_characteristic_pending_ = false;
@@ -507,11 +494,11 @@ void BluetoothLowEnergyConnection::OnRemoteCharacteristicWritten(
   ProcessNextWriteRequest();
 }
 
-void BluetoothLowEnergyConnection::OnWriteRemoteCharacteristicError(
+void BluetoothLowEnergyWeaveClientConnection::OnWriteRemoteCharacteristicError(
     bool run_did_send_message_callback,
     BluetoothRemoteGattService::GattErrorCode error) {
   PA_LOG(WARNING) << "Error " << error << " writing characteristic: "
-                  << to_peripheral_char_.uuid.canonical_value();
+                  << tx_characteristic_.uuid.canonical_value();
   write_remote_characteristic_pending_ = false;
   // TODO(sacomoto): Actually pass the current message to the observer.
   if (run_did_send_message_callback)
@@ -527,21 +514,11 @@ void BluetoothLowEnergyConnection::OnWriteRemoteCharacteristicError(
   ProcessNextWriteRequest();
 }
 
-BluetoothLowEnergyConnection::WriteRequest
-BluetoothLowEnergyConnection::BuildWriteRequest(
-    const std::vector<uint8_t>& signal,
-    const std::vector<uint8_t>& bytes,
-    bool is_last_write_for_wire_message) {
-  std::vector<uint8_t> value(signal.begin(), signal.end());
-  value.insert(value.end(), bytes.begin(), bytes.end());
-  return WriteRequest(value, is_last_write_for_wire_message);
-}
-
-void BluetoothLowEnergyConnection::PrintTimeElapsed() {
+void BluetoothLowEnergyWeaveClientConnection::PrintTimeElapsed() {
   PA_LOG(INFO) << "Time elapsed: " << base::TimeTicks::Now() - start_time_;
 }
 
-std::string BluetoothLowEnergyConnection::GetDeviceAddress() {
+std::string BluetoothLowEnergyWeaveClientConnection::GetDeviceAddress() {
   // When the remote device is connected we should rely on the address given by
   // |gatt_connection_|. As the device address may change if the device is
   // paired. The address in |gatt_connection_| is automatically updated in this
@@ -550,7 +527,7 @@ std::string BluetoothLowEnergyConnection::GetDeviceAddress() {
                           : remote_device().bluetooth_address;
 }
 
-BluetoothDevice* BluetoothLowEnergyConnection::GetRemoteDevice() {
+BluetoothDevice* BluetoothLowEnergyWeaveClientConnection::GetRemoteDevice() {
   // It's not possible to simply use
   // |adapter_->GetDevice(GetDeviceAddress())| to find the device with MAC
   // address |GetDeviceAddress()|. For paired devices,
@@ -567,7 +544,8 @@ BluetoothDevice* BluetoothLowEnergyConnection::GetRemoteDevice() {
   return nullptr;
 }
 
-BluetoothRemoteGattService* BluetoothLowEnergyConnection::GetRemoteService() {
+BluetoothRemoteGattService*
+BluetoothLowEnergyWeaveClientConnection::GetRemoteService() {
   BluetoothDevice* remote_device = GetRemoteDevice();
   if (!remote_device) {
     PA_LOG(WARNING) << "Remote device not found.";
@@ -586,7 +564,7 @@ BluetoothRemoteGattService* BluetoothLowEnergyConnection::GetRemoteService() {
 }
 
 BluetoothRemoteGattCharacteristic*
-BluetoothLowEnergyConnection::GetGattCharacteristic(
+BluetoothLowEnergyWeaveClientConnection::GetGattCharacteristic(
     const std::string& gatt_characteristic) {
   BluetoothRemoteGattService* remote_service = GetRemoteService();
   if (!remote_service) {
@@ -596,23 +574,6 @@ BluetoothLowEnergyConnection::GetGattCharacteristic(
   return remote_service->GetCharacteristic(gatt_characteristic);
 }
 
-// TODO(sacomoto): make this robust to byte ordering in both sides of the
-// SmartLock BLE socket.
-uint32_t BluetoothLowEnergyConnection::ToUint32(
-    const std::vector<uint8_t>& bytes) {
-  return bytes[0] | (bytes[1] << 8) | (bytes[2] << 16) | (bytes[3] << 24);
-}
-
-// TODO(sacomoto): make this robust to byte ordering in both sides of the
-// SmartLock BLE socket.
-const std::vector<uint8_t> BluetoothLowEnergyConnection::ToByteVector(
-    const uint32_t value) {
-  std::vector<uint8_t> bytes(4, 0);
-  bytes[0] = static_cast<uint8_t>(value);
-  bytes[1] = static_cast<uint8_t>(value >> 8);
-  bytes[2] = static_cast<uint8_t>(value >> 16);
-  bytes[3] = static_cast<uint8_t>(value >> 24);
-  return bytes;
-}
+}  // namespace weave
 
 }  // namespace proximity_auth