1170509 - Work around broken function |get_remote_service_record|

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Calls to |get_remote_service_record| return incomplete data in the notification callback. See the TODOs in |btif_dm_remote_service_record_evt| in [1]. This bug has been present since the first version of Bluedroid, so every recent Android phone is affected.

We need to work around the problem to make SDP lookups work in Gecko. There's also bug 1142007 to keep track of a real fix in Bluedroid.

[1] https://android.googlesource.com/platform/external/bluetooth/bluedroid/+/master/btif/src/btif_dm.c

Comment 1

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: Github pull request

Shawn,

This pull request contains a workaround for the broken Bluedroid function. I'd like to get this merged in preparation of bug 1141616 and bug 1088574.

In case you want to test this, you can do so by

 1) applying the patches of bug 1141616, and
 2) sending an image to a connected device.

This should invoke the SDP service discovery in Gecko and trigger the workaround.

Comment 2

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Thomas,
I'm so sorry to review this patch so late. I will review it tomorrow.

Comment 3

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Hey, no problem! I'm thinking about fixing bug 1088574 during Q3. So there's no urgency.

Comment 4

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Hi Shawn,

can you give an ETA for reviewing the pull request or hand it over to someone else if you're busy. (Bruce?) Thanks!

Comment 5

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #4)
> Hi Shawn,
> 
> can you give an ETA for reviewing the pull request or hand it over to
> someone else if you're busy. (Bruce?) Thanks!

Hi Thomas,
Sorry for delay here. I have some concerns and I've updated comments on github.

Comment 6

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Comment on attachment 8614006 [details] [review]
Github pull request

I've updated comments for this PR.

Comment 7

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 8614006 [details] [review]
Github pull request

Updated Github pull request:

  - added comment in sdpsrvc.c
  - fixed comment for workaround
  - use |ARRAY_LENGTH| when sanitizing service name
  - return an empty string for unknown service records
  - added comment about service names

I also submitted the bug to Google: https://code.google.com/p/android/issues/detail?id=179772

Comment 8

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #7)
> Comment on attachment 8614006 [details] [review]
> Github pull request
> 
> Updated Github pull request:
> I also submitted the bug to Google:
> https://code.google.com/p/android/issues/detail?id=179772

Is it correct title? Should be invalid response?

Comment 9

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Right, but there seems to be no way of changing it now. I left a comment in the bug report. Maybe someone at Google can fix this.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/platform_system_bluetoothd/commit/b0f9410d2f7111d2fc9b30e2f7bb14dd89af0259