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 53%
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..6d1acfeb95b04814adfc1e1738c205685421e93c 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;
@@ -34,42 +24,45 @@ using device::BluetoothRemoteGattCharacteristic;
 using device::BluetoothGattNotifySession;
 using device::BluetoothUUID;
 
-namespace proximity_auth {
 namespace {
 
-// The UUID of the characteristic used to send data to the peripheral.
-const char kToPeripheralCharUUID[] = "977c6674-1239-4e72-993b-502369b8bb5a";
+typedef proximity_auth::BluetoothLowEnergyWeavePacketReceiver::State
+    ReceiverState;
 
-// The UUID of the characteristic used to receive data from the peripheral.
-const char kFromPeripheralCharUUID[] = "f4b904a2-a030-43b3-98a8-221c536c03cb";
+// TODO(jingxy): figure out the right UUID to use
+// The UUID of the TX characteristic used to send data to the server.

[Kyle Horimoto, 2016/06/27 18:06:02]

s/send/transmit/

[jingxuy, 2016/06/30 00:27:20]

Done.

+const char kTXCharacteristicUUID[] = "977c6674-1239-4e72-993b-502369b8bb5a";

[Kyle Horimoto, 2016/06/27 18:06:02]

00000100-0004-1000-8000-001A11000101

[sacomoto, 2016/06/29 14:26:58]

+1

[jingxuy, 2016/06/30 00:27:20]

Done.

 
-// Deprecated signal send as the first byte in send byte operations.
-const int kFirstByteZero = 0;
+// TODO(jingxy): figure out the right UUID to use
+// The UUID of the RX characteristic used to receiver data from the server.

[Kyle Horimoto, 2016/06/27 18:06:02]

s/receiver/receive/

[jingxuy, 2016/06/30 00:27:20]

Done.

+const char kRXCharacteristicUUID[] = "f4b904a2-a030-43b3-98a8-221c536c03cb";

[Kyle Horimoto, 2016/06/27 18:06:02]

00000100-0004-1000-8000-001A11000102

[sacomoto, 2016/06/29 14:26:57]

+1

[jingxuy, 2016/06/30 00:27:20]

Done.

 
-// 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)
+namespace proximity_auth {
+
+BluetoothLowEnergyWeaveClientConnection::
+    BluetoothLowEnergyWeaveClientConnection(

[Kyle Horimoto, 2016/06/27 18:06:02]

Your constructor should take a generator and receiver as parameters. In your
test, you can pass in a fake generator/receiver which each generate stub values
which are easy to test against.

If you do not do this, your tests for this class must implicitly know how the
uWeave protocol works, meaning that you are essentially testing both this class
and the generator/receiver classes. Providing mock classes isolates your unit
test to testing only this class, providing greater test isolation and ensuring
that a bug in one of the generator/receiver classes does not cause an incorrect
test to pass or a correct test to fail.

In your mocks, you should do something like the following: define several
constant values to be used as messages. For example, the "connection request"
packet could just be std::vector<uint8_t>(20, 'a'). It doesn't really matter
what the contents of the packets are as long as they are different. Then you
could create several fake "data messages" (name them message 1, message 2, etc).
Then, for example, your CreateConnectionRequest() would just return the constant
value, and your mock receiver would only change states if the mock value for a
connection request was received.

Likewise, for functions which we expect to be called, we would just set a value
which can be observed later. So, for example, SetMaxPacketSize() could just
store the packet sized passed as an instance variable for the generator class.
Then, you could add a GetLastMaxPacketSizeSet() function to your mock class
which could be called after the fact to ensure that the correct value was passed
via EXPECT_EQ(expected_value, mock_generator->GetLastMaxPacketSizeSet()).

[jingxuy, 2016/06/30 00:27:20]

Done.

+        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 +70,8 @@ BluetoothLowEnergyConnection::BluetoothLowEnergyConnection(
   adapter_->AddObserver(this);
 }
 
-BluetoothLowEnergyConnection::~BluetoothLowEnergyConnection() {
+BluetoothLowEnergyWeaveClientConnection::
+    ~BluetoothLowEnergyWeaveClientConnection() {
   Disconnect();
   if (adapter_) {
     adapter_->RemoveObserver(this);
@@ -85,7 +79,7 @@ BluetoothLowEnergyConnection::~BluetoothLowEnergyConnection() {
   }
 }
 
-void BluetoothLowEnergyConnection::Connect() {
+void BluetoothLowEnergyWeaveClientConnection::Connect() {
   DCHECK(sub_status() == SubStatus::DISCONNECTED);
 
   SetSubStatus(SubStatus::WAITING_GATT_CONNECTION);
@@ -101,8 +95,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 +105,7 @@ void BluetoothLowEnergyConnection::Connect() {
   CreateGattConnection();
 }
 
-void BluetoothLowEnergyConnection::CreateGattConnection() {
+void BluetoothLowEnergyWeaveClientConnection::CreateGattConnection() {
   DCHECK(sub_status() == SubStatus::WAITING_GATT_CONNECTION);
 
   BluetoothDevice* remote_device = GetRemoteDevice();
@@ -119,14 +114,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 +143,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<std::vector<uint8_t>> 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 +198,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,7 +210,7 @@ void BluetoothLowEnergyConnection::DeviceRemoved(BluetoothAdapter* adapter,
   Disconnect();
 }
 
-void BluetoothLowEnergyConnection::GattCharacteristicValueChanged(
+void BluetoothLowEnergyWeaveClientConnection::GattCharacteristicValueChanged(
     BluetoothAdapter* adapter,
     BluetoothRemoteGattCharacteristic* characteristic,
     const std::vector<uint8_t>& value) {
@@ -257,79 +222,56 @@ void BluetoothLowEnergyConnection::GattCharacteristicValueChanged(
   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:
+        // TODO(jingxuy): what to do with the reason for close?

[sacomoto, 2016/06/29 14:26:57]

Just log the error for now. 

This class doesn't pass any errors to its users in the case of a disconnect. 

[jingxuy, 2016/06/30 00:27:20]

Done.

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

Add a function which converts a ReasonForClose to a string so that the logs are
readable.

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

Done.

+        Disconnect();
         break;
-      }
-      case ControlSignal::kDisconnectSignal:
-        PA_LOG(INFO) << "Disconnect signal received.";
+      case ReceiverState::ERROR:
+        // TODO(jingxuy): send a control close over the channel

[Kyle Horimoto, 2016/06/27 18:06:02]

Can you do this in this CL please?

[sacomoto, 2016/06/29 14:26:58]

+1

You'll need to change the logic here. If try to send the error packet and then
immediately call the |Disconnect()|, the error packet will never be sent.  

[jingxuy, 2016/06/30 00:27:20]

Done.

         Disconnect();
         break;
+      case ReceiverState::WAITING:
+        packet_generator_->SetMaxPacketSize(

[Kyle Horimoto, 2016/06/27 18:06:02]

Please add a comment about what the WAITING state means for the client and why
this is done at this time.

[jingxuy, 2016/06/30 00:27:20]

Done.

+            packet_receiver_->GetMaxPacketSize());
+        if (sub_status() == SubStatus::WAITING_RESPONSE_SIGNAL)
+          CompleteConnection();
+        break;
+      case ReceiverState::CONNECTING:

[Kyle Horimoto, 2016/06/27 18:06:02]

Is CONNECTING a valid state? If we have already received a packet, shouldn't we
have transitioned to another state?

[jingxuy, 2016/06/30 00:27:20]

Done.

+      case ReceiverState::RECEIVING_DATA:
+        // Normal in between states, so do nothing.
+        break;
+      default:
+        NOTREACHED();
     }
   }
 }
 
-BluetoothLowEnergyConnection::WriteRequest::WriteRequest(
+BluetoothLowEnergyWeaveClientConnection::WriteRequest::WriteRequest(
     const std::vector<uint8_t>& 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 +280,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 +293,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()
                           : "")
-                  << (from_peripheral_char.id.empty()
-                          ? ", " + from_peripheral_char.uuid.canonical_value()
-                          : "") << " not found.";
+                  << (rx_characteristic.id.empty()
+                          ? ", " + rx_characteristic.uuid.canonical_value()
+                          : "")
+                  << " 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
@@ -419,21 +364,16 @@ void BluetoothLowEnergyConnection::StartNotifySession() {
 
     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 "
@@ -446,36 +386,41 @@ void BluetoothLowEnergyConnection::OnNotifySessionStarted(
   SendInviteToConnectSignal();
 }
 
-void BluetoothLowEnergyConnection::StopNotifySession() {
+void BluetoothLowEnergyWeaveClientConnection::OnNotifySessionError(
+    BluetoothRemoteGattService::GattErrorCode error) {
+  DCHECK(sub_status() == SubStatus::WAITING_NOTIFY_SESSION);
+  PA_LOG(WARNING) << "Error starting notification session: " << error;
+  Disconnect();
+}
+
+void BluetoothLowEnergyWeaveClientConnection::StopNotifySession() {
   if (notify_session_) {
     notify_session_->Stop(base::Bind(&base::DoNothing));
     notify_session_.reset();
   }
 }
 
-void BluetoothLowEnergyConnection::SendInviteToConnectSignal() {
+void BluetoothLowEnergyWeaveClientConnection::SendInviteToConnectSignal() {
   if (sub_status() == SubStatus::NOTIFY_SESSION_READY) {
     PA_LOG(INFO) << "Sending invite to connect signal";
     SetSubStatus(SubStatus::WAITING_RESPONSE_SIGNAL);
 
-    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::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;
@@ -483,17 +428,18 @@ void BluetoothLowEnergyConnection::ProcessNextWriteRequest() {
     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 +453,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 +473,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 +486,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 +503,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 +523,7 @@ BluetoothRemoteGattService* BluetoothLowEnergyConnection::GetRemoteService() {
 }
 
 BluetoothRemoteGattCharacteristic*
-BluetoothLowEnergyConnection::GetGattCharacteristic(
+BluetoothLowEnergyWeaveClientConnection::GetGattCharacteristic(
     const std::string& gatt_characteristic) {
   BluetoothRemoteGattService* remote_service = GetRemoteService();
   if (!remote_service) {
@@ -596,23 +533,4 @@ 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 proximity_auth