[b2g-bluetooth] Implement function to register services

RESOLVED FIXED in mozilla17

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ericchou, Assigned: ericchou)

Tracking

Trunk
mozilla17
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Others devices can retrieve what services our device is providing now from SDP Service and Attribute queries. So we need an interface for applications to register/unregister its RFCOMM service to local SDP server.
(Assignee)

Updated

5 years ago
Assignee: nobody → echou
(Assignee)

Comment 1

5 years ago
Sorry, we may not provide unregister to application in order to avoid accidentally unregistering some system built-in services.
(Assignee)

Updated

5 years ago
Summary: B2G Bluetooth: Register/Unregister RFCOMM service → B2G Bluetooth: Register RFCOMM service
(Assignee)

Updated

5 years ago
Summary: B2G Bluetooth: Register RFCOMM service → [b2g-bluetooth] Implement function to register services
(Assignee)

Updated

5 years ago
Blocks: 727618
(Assignee)

Comment 2

5 years ago
Created attachment 649568 [details] [diff] [review]
v1: patch1: Implement function to add reserved services

The word 'Reserved Services' is from BlueZ's implementation. There are 4 reserved services defined in BlueZ, which are PBAP_PSE, HEADSET_AG, HANDSFREE_AG and OBEX_OBJPUSH. 

(For more information, see add_reserved_service_records() in external/bluetooth/bluez/src/adapter.c)
Attachment #649568 - Flags: review?(kyle)
(Assignee)

Comment 3

5 years ago
Created attachment 649570 [details] [diff] [review]
v1: patch 2: Implement function to remove reserved services

We still need a function to remove added reserved services because those services may be added every time after turning on Bluetooth.
Attachment #649570 - Flags: review?(kyle)
Comment on attachment 649568 [details] [diff] [review]
v1: patch1: Implement function to add reserved services

Review of attachment 649568 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/linux/BluetoothDBusService.cpp
@@ +1217,5 @@
> +                            DBUS_TYPE_INVALID)) {
> +    if (!handles) {
> +      LOG("Null array in extract_handles");
> +    } else {
> +      for (int i = 0;i < len;++i) {

Nit: space after ;'s in fors.

@@ +1244,5 @@
> +                                      DBUS_TYPE_ARRAY, DBUS_TYPE_UINT32,
> +                                      &services, length, DBUS_TYPE_INVALID);
> +
> +  if (!reply) {
> +    LOG("Null DBus message. Couldn't extract handles.");

Nit: return early here, kill else block.
Attachment #649568 - Flags: review?(kyle) → review+
Comment on attachment 649570 [details] [diff] [review]
v1: patch 2: Implement function to remove reserved services

Review of attachment 649570 [details] [diff] [review]:
-----------------------------------------------------------------

r+  with nits picked and question addressed.

::: dom/bluetooth/linux/BluetoothDBusService.cpp
@@ +1255,5 @@
>  
>  bool
> +BluetoothDBusService::RemoveReservedServicesInternal(const nsAString& aAdapterPath,
> +                                                     const nsTArray<PRUint32>& aServiceHandles)
> +{

Nit: Blocks, shouldn't be called on Main thread, add thread check.

@@ +1256,5 @@
>  bool
> +BluetoothDBusService::RemoveReservedServicesInternal(const nsAString& aAdapterPath,
> +                                                     const nsTArray<PRUint32>& aServiceHandles)
> +{
> +  const char* adapterPath = NS_ConvertUTF16toUTF8(aAdapterPath).get();

Nit: Intermediate isn't needed, we can just pass this in to dbus_func_args.

@@ +1258,5 @@
> +                                                     const nsTArray<PRUint32>& aServiceHandles)
> +{
> +  const char* adapterPath = NS_ConvertUTF16toUTF8(aAdapterPath).get();
> +
> +  DBusMessage *msg = NULL;

Nit: This isn't used?

@@ +1267,5 @@
> +
> +  const uint32_t* services = aServiceHandles.Elements();
> +
> +  reply = dbus_func_args(mConnection,
> +                         adapterPath,                    

Nit: Remove trailing space

@@ +1272,5 @@
> +                         DBUS_ADAPTER_IFACE, "RemoveReservedServiceRecords",
> +                         DBUS_TYPE_ARRAY, DBUS_TYPE_UINT32,
> +                         &services, length, DBUS_TYPE_INVALID);
> +
> +  return reply ? true : false;

Are we expected to deallocate reply ourselves?
Attachment #649570 - Flags: review?(kyle) → review+
(Assignee)

Comment 6

5 years ago
> @@ +1272,5 @@
> > +                         DBUS_ADAPTER_IFACE, "RemoveReservedServiceRecords",
> > +                         DBUS_TYPE_ARRAY, DBUS_TYPE_UINT32,
> > +                         &services, length, DBUS_TYPE_INVALID);
> > +
> > +  return reply ? true : false;
> 
> Are we expected to deallocate reply ourselves?

Interesting. Looks like a potential memory leak in Android. :P
(Assignee)

Comment 7

5 years ago
Created attachment 650428 [details] [diff] [review]
Final: Patch 1: Implement function to add reserved services, r=qDot

Nits picked.
Attachment #649568 - Attachment is obsolete: true
(Assignee)

Comment 8

5 years ago
Created attachment 650429 [details] [diff] [review]
Final: Patch 2: Implement function to remove reserved services, r=qDot

Nits picked & avoid potential memory leakage.
Attachment #649570 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #650429 - Attachment description: Final: Patch 2: Implement function to remove reserved services → Final: Patch 2: Implement function to remove reserved services, r=qDot
(In reply to Eric Chou [:ericchou] [:echou] from comment #6)

> Interesting. Looks like a potential memory leak in Android. :P

Yeah. A /big/ portion of the kickbacks on bug 740744 were me having to fix all the stuff I'd cut and pasted from the event loop code. :|
(Assignee)

Comment 10

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/c5459232d867
http://hg.mozilla.org/integration/mozilla-inbound/rev/4f31d157cf6b
https://hg.mozilla.org/mozilla-central/rev/c5459232d867
https://hg.mozilla.org/mozilla-central/rev/4f31d157cf6b
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.