1122327 - [System][Bluetooth] Wrap current Bluetooth module to instanciable format

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[[e-mail]]

Current BT related status icon depends on window.Bluetooth to get profileConnection stat, which will cause problem when we switch to bluetooth APIv2

The affect files include:

media_playback
screen_manager
sound_manager
statusbar
system_nfc_connect_dialog

Besides, we'd better follow the naming convention in Settings/Bluetooth to rename current Bluetooth.js/BluetoothTransfer.js to Bluetooth1.js/BluetoothTransfer1.js
 and make APIv2 version use the origin name.

The work need to based on icon per module bug 1098168

Comment 1

[[e-mail]]

@alive I'd like wrap current Bluetooth module to instanciable format, and implement same profileConnection api on Bluetooth v2. Therefore we only have to change file reference to non-capital 'window.bluetooth'.  Is it make sense to you?


// pseudo code in bluetooth_core
if (APIv1) {
  window.bluetooth = new Bluetooth1(); (v1)
  ...
else { // APIv2
  window.bluetooth = new Bluetooth(); (v2);
  ..
}

Comment 2

[[e-mail]]

Found after rename window.Bluetooth in Bluetooth.js to window.Bluetooth1, the follow code simply works in bluetooth_core

window.Bluetooth = window.Bluetooth1;
window.BluetoothTransfer = window.BluetoothTransfer1; 
window.Bluetooth.init();
window.BluetoothTransfer.init();

So we don't have to change affect files and not have to change the existing Bluetooth v1 file too much.

Comment 3

[GitHub Autolander Bot]

Attachment: [PullReq] gasolin:issue-1122327 to mozilla-b2g:master

Comment 4

[[e-mail]]

Comment on attachment 8550039 [details] [review]
[PullReq] gasolin:issue-1122327 to mozilla-b2g:master

* rename module to fit the naming convention in Settings/Bluetooth
* fix lint error of Bluetooth Transfer test

nfc/BT paring transfer test fine

The current approach also make API change independent to statusbar and other system part.

Comment 5

[[e-mail]]

Comment on attachment 8550039 [details] [review]
[PullReq] gasolin:issue-1122327 to mozilla-b2g:master

gij is fail, will fix it before review. Change to feedback? instead.

Comment 6

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8550039 [details] [review]
[PullReq] gasolin:issue-1122327 to mozilla-b2g:master

I don't see big value before there is a Bluetooth2.js but anyway.

Comment 7

[[e-mail], [e-mail]]

Comment on attachment 8550039 [details] [review]
[PullReq] gasolin:issue-1122327 to mozilla-b2g:master

Feedback + for me. The major change is to fit the old v1 code base. And we no need to do too much refinement since we also want to refactor the code base for v2. We would like to make Bluetooth() module be new-able in system app in the feature.

Comment 8

[[e-mail]]

Thanks for feedback. I will send another PR based on bug 1098168

Comment 9

[GitHub Autolander Bot]

Attachment: [gaia] gasolin:issue-1122327-3 > mozilla-b2g:master

Comment 10

[[e-mail]]

Comment on attachment 8574533 [details] [review]
[gaia] gasolin:issue-1122327-3 > mozilla-b2g:master

The patch wrap BTv1 via Lazyload and fix bluetooth_transfer.js jshint error.
Transfer and quicksettings works fine on v1 device.

In future patch I'd like to reuse bluetooth_transfer for both v1/v2, so the naming is unchanged. Will try to move bluetooth_transfer to instanciable format in future patch.

Comment 11

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8574533 [details] [review]
[gaia] gasolin:issue-1122327-3 > mozilla-b2g:master

nits

Comment 12

[[e-mail], [e-mail]]

Comment on attachment 8574533 [details] [review]
[gaia] gasolin:issue-1122327-3 > mozilla-b2g:master

r+ with me! Thanks for your effort here.

Comment 13

[[e-mail]]

Thanks for review! Will rename this._debug to this.debug for consistency. Will land after gij + gip test cases fixed

Comment 14

[[e-mail]]

treeherfer green after bug 1141442 fixed, merged to master https://github.com/mozilla-b2g/gaia/commit/f1bcd41e881a42b75336a48942c59a767782e0ee

Since the indent fix make followup PR harder to merge, would like to fix indent after related patch landed.

Thanks!