Closed
Bug 933195
Opened 11 years ago
Closed 11 years ago
[B2G] [Bluetooth] Object path should be replaced with device address before broadcasting to BluetoothAdpater
Categories
(Firefox OS Graveyard :: Bluetooth, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: gyeh, Assigned: gyeh)
References
Details
Attachments
(1 file, 1 obsolete file)
As Shawn noticed, we send an array of object path to BluetoothAdatper and replace them with device address one by one in BluetoothAdapter. I think we should do these operations right after receiving signals from BlueZ rather than in the child process.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → gyeh
Assignee | ||
Updated•11 years ago
|
blocking-b2g: --- → 1.3?
Assignee | ||
Comment 1•11 years ago
|
||
It's quite short. Should be an easy one for you, Eric. ;)
Attachment #825828 -
Flags: review?(echou)
Comment 2•11 years ago
|
||
Comment on attachment 825828 [details] [diff] [review] Patch 1(v1): Replace object path with device address Review of attachment 825828 [details] [diff] [review]: ----------------------------------------------------------------- r=me with nits addressed. ::: dom/bluetooth/linux/BluetoothDBusService.cpp @@ +621,5 @@ > */ > bool convert = false; > if (propertyName.EqualsLiteral("Connected") && > receivedType == DBUS_TYPE_ARRAY) { > + MOZ_ASSERT(aPropertyTypes == sDeviceProperties); Not sure if we need to add this assertion. Interface AudioSink, Control and Input have "Connected" property, too. It seems to be possible that the type of "Connected" property of these interface may change from boolean to array as well (I know it's really a disaster, but it could happen). :( @@ +690,5 @@ > MOZ_ASSERT(propertyValue.type() == BluetoothValue::TArrayOfuint8_t); > > bool b = propertyValue.get_ArrayOfuint8_t()[0]; > propertyValue = BluetoothValue(b); > + } else if (propertyName.EqualsLiteral("Devices")){ super-nit: please insert an extra blank between ')' and '{'. Furthermore, how about leaving a few comments to describe why we did this (translate from object path to address)? @@ +695,5 @@ > + MOZ_ASSERT(aPropertyTypes == sAdapterProperties); > + MOZ_ASSERT(propertyValue.type() == BluetoothValue::TArrayOfnsString); > + > + uint32_t length = propertyValue.get_ArrayOfnsString().Length(); > + for (int i = 0; i < length; i++) { nit: please make sure that 'i' has the same type with 'length' to avoid warnings.
Attachment #825828 -
Flags: review?(echou) → review+
Assignee | ||
Comment 3•11 years ago
|
||
Thanks for your quick review. (In reply to Eric Chou [:ericchou] [:echou] from comment #2) > ::: dom/bluetooth/linux/BluetoothDBusService.cpp > @@ +621,5 @@ > > */ > > bool convert = false; > > if (propertyName.EqualsLiteral("Connected") && > > receivedType == DBUS_TYPE_ARRAY) { > > + MOZ_ASSERT(aPropertyTypes == sDeviceProperties); > > Not sure if we need to add this assertion. Interface AudioSink, Control and > Input have "Connected" property, too. It seems to be possible that the type > of "Connected" property of these interface may change from boolean to array > as well (I know it's really a disaster, but it could happen). :( If it happens again, I think that this assertion helps us to observe the change in debug build. For release build, there should be no difference. So, I'd like to keep it.
Assignee | ||
Comment 4•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=3c472526062d https://tbpl.mozilla.org/?tree=Try&rev=2347cdfa57e9
Attachment #825828 -
Attachment is obsolete: true
Assignee | ||
Comment 5•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&3c472526062d
Assignee | ||
Comment 6•11 years ago
|
||
Please ignore the above links. Sorry for bothering. Try server links: https://tbpl.mozilla.org/?tree=Try&rev=9f1b81f23108 https://tbpl.mozilla.org/?tree=Try&rev=0ad566f884dc
Assignee | ||
Comment 7•11 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/ff8eecaf65e4
Blocks: 932770
Comment 8•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ff8eecaf65e4
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•