Add support for local services/characteristics/descriptors.

device/bluetooth/bluetooth_gatt_characteristic_bluez.h

+// Copyright 2014 The Chromium Authors. All rights reserved.

[ortuno, 2016/04/14 17:42:34]

It's 2016!

[rkc, 2016/04/14 19:27:26]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef DEVICE_BLUETOOTH_BLUETOOTH_GATT_CHARACTERISTIC_BLUEZ_H_
+#define DEVICE_BLUETOOTH_BLUETOOTH_GATT_CHARACTERISTIC_BLUEZ_H_
+
+#include <stddef.h>

[ortuno, 2016/04/14 17:42:34]

Don't see any NULLs or size_ts.

[rkc, 2016/04/14 19:27:26]

Done.

+#include <stdint.h>

[ortuno, 2016/04/14 17:42:35]

Same here no ints.

[rkc, 2016/04/14 19:27:26]

Done.

+
+#include <map>

[ortuno, 2016/04/14 17:42:34]

Some of these are no longer needed as well. Please clean up.

[rkc, 2016/04/14 19:27:26]

Done.

+#include <queue>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "dbus/object_path.h"
+#include "device/bluetooth/bluetooth_gatt_characteristic.h"
+#include "device/bluetooth/bluetooth_uuid.h"

[ortuno, 2016/04/14 17:42:34]

No UUIDs.

[rkc, 2016/04/14 19:27:26]

Done.

+
+namespace device {
+
+class BluetoothGattDescriptor;
+class BluetoothGattService;
+
+}  // namespace device
+
+namespace bluez {
+
+class BluetoothGattDescriptorBlueZ;
+class BluetoothGattServiceBlueZ;
+
+// The BluetoothGattCharacteristicBlueZ class implements
+// BluetoothGattCharacteristic for GATT characteristics for platforms
+// that use BlueZ.

[ortuno, 2016/04/14 17:42:35]

Could you mention that this class implements common methods between Remote And
Local Characteristics.

[rkc, 2016/04/14 19:27:26]

Seems like a superfluous comment. To implement common functionality between all
derived classes is one of the main purposes of a base class.

+class BluetoothGattCharacteristicBlueZ
+    : public device::BluetoothGattCharacteristic {
+ public:
+  // device::BluetoothGattCharacteristic overrides.
+  std::string GetIdentifier() const override;
+  device::BluetoothGattService* GetService() const override;
+  Permissions GetPermissions() const override;
+  std::vector<device::BluetoothGattDescriptor*> GetDescriptors() const override;
+  device::BluetoothGattDescriptor* GetDescriptor(
+      const std::string& identifier) const override;
+
+  // Object path of the underlying D-Bus characteristic.
+  const dbus::ObjectPath& object_path() const { return object_path_; }
+
+ protected:
+  BluetoothGattCharacteristicBlueZ(BluetoothGattServiceBlueZ* service,
+                                   const dbus::ObjectPath& object_path);
+  ~BluetoothGattCharacteristicBlueZ() override;
+
+  // Mapping from GATT descriptor object paths to descriptor objects owned by
+  // this characteristic. Since the BlueZ implementation uses object paths
+  // as unique identifiers, we also use this mapping to return descriptors by
+  // identifier.
+  using DescriptorMap =
+      std::map<dbus::ObjectPath, BluetoothGattDescriptorBlueZ*>;

[ortuno, 2016/04/14 17:42:34]

optional: Any reason why you leave the deletion of descriptors up to the local
and remote implementations? If not please consider using unique_ptr here.

If end up not using unique_ptrs please document that any classes that add
objects to this map
are in charge of manually freeing them.

[rkc, 2016/04/14 19:27:26]

This is a refactor. The purpose is to try to move code around and only change
what really needs to be.

[ortuno, 2016/04/14 21:17:38]

OK. I still think a comment that points out that if classes add to this map they
need to free them manually is needed, but up to you.

[rkc, 2016/04/14 21:59:38]

Done.

+  DescriptorMap descriptors_;
+
+  // The GATT service this GATT characteristic belongs to.

[ortuno, 2016/04/14 17:42:34]

nit: The GATT service that owns this characteristic.

[rkc, 2016/04/14 19:27:26]

Done.

+  BluetoothGattServiceBlueZ* service_;
+
+ private:
+  friend class BluetoothRemoteGattServiceBlueZ;
+
+  // Object path of the D-Bus characteristic object.
+  dbus::ObjectPath object_path_;
+
+  // Note: This should remain the last member so it'll be destroyed and
+  // invalidate its weak pointers before any other members are destroyed.
+  base::WeakPtrFactory<BluetoothGattCharacteristicBlueZ> weak_ptr_factory_;

[ortuno, 2016/04/14 17:42:34]

Do you still need this for the general implementation? You also have one in each
implementation.

[rkc, 2016/04/14 19:27:27]

I do not, thanks for catching this.
Done.

+
+  DISALLOW_COPY_AND_ASSIGN(BluetoothGattCharacteristicBlueZ);
+};
+
+}  // namespace bluez
+
+#endif  // DEVICE_BLUETOOTH_BLUETOOTH_GATT_CHARACTERISTIC_BLUEZ_H_