817972 - [Bluetooth][File-Transfer] Support multiple files transferring

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

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

There is no definition of sharing multiple files in the spec. Bluetooth V2. Do we need to have the feature in V1?

STR:
==========
  1) Launch Gallery App
  2) Click selection button and go into selected mode
  3) Select multiple files(more than one file)
  4) Click share button

Expected:
==========
  It should show the option "Send To Bluetooth Device" to provide the feature. Then, let the selecting file send by Bluetooth.

Actual:
==========
  No way to share multiple files by Bluetooth.

Comment 1

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

Hi Casey,
I need your definition and confirmation for the feature.
If we want to have the feature, should we show each progress(transferring task) per one file in the notification center?
Thank you.

Comment 2

[Gina Yeh [:gyeh] [:ginayeh]]

Hi Ian,

Bug 817930 is only for receiving multiple files. If we are going to support sending multiple files at one time, then I'll open a new one.

Comment 3

[Walter Chen[:ypwalter][:wachen]]

As far as my checking of v1 application status and other stuffs, I didn't see any user stories or requirement for multiple file transferring.

Comment 4

[David Scravaglieri [:scravag]]

Renominate if needed

Comment 5

[Casey Yee]

cc Clee to confirm but I'm pretty sure there is no requirement for V1 to be able to transfer multiple files using Bluetooth.

You should only be presented with the share option (with Bluetooth) in the individual image view screen and not in the multi-select gallery view screen.

Comment 6

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

Hey developers and QA,
Thanks for all your confirmation.

Comment 9

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 1: v1: Add capability of sending multiple files to a remove device

Add Gecko support for multiple file-trasfering. Users can select more than one photo in Gallery and send those to a paired Bluetooth device. Photos would be sent sequentially, which means that one photo at a time.

Comment 10

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 741220 [details] [diff] [review]
patch 1: v1: Add capability of sending multiple files to a remove device

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

I'd like to review again with nits addressed and question answered.




Question: When shall we broadcast system messages of "bluetooth-opp-transfer-start" and "bluetooth-opp-transfer-complete"? In the case of sending multiple files sequentially, I think we should broadcast a start message right before sending out a put header request and broadcast a complete message right after receiving response code of a put final packet.

So, here's an example of what I expect.
-> Connect request
<- response code of Success
* Broadcast "bluetooth-opp-transfer-start" for the first file
-> PutHeader request
<- response code: Success
-> Put request
<- response code: Continue
-> Put request
<- response code: Continue
-> PutFinal request
<- response code: Success
* Broadcast "bluetooth-opp-transfer-complete" for the first file
* Broadcast "bluetooth-opp-transfer-start" for the second file
-> PutHeader request
<- response code: Success
-> Put request
<- response code: Continue
-> Put request
<- response code: Continue
-> PutFinal request
<- response code: Success
* Broadcast "bluetooth-opp-transfer-complete" for the second file
-> Disconnect request
<- response code: Success


Here's the actual process of this patch.
-> Connect request
* Broadcast "bluetooth-opp-transfer-start" for the first file
<- response code of Success
-> PutHeader request
<- response code: Success
-> Put request
<- response code: Continue
-> Put request
<- response code: Continue
-> PutFinal request
<- response code: Success
* Broadcast "bluetooth-opp-transfer-complete" for the first file
-> PutHeader request
* Broadcast "bluetooth-opp-transfer-start" for the second file
<- response code: Success
-> Put request
<- response code: Continue
-> Put request
<- response code: Continue
-> PutFinal request
<- response code: Success
* Broadcast "bluetooth-opp-transfer-complete" for the second file
-> Disconnect request
<- response code: Success

Since the actual result is different from what I expect, what do you think? Please let me know if I misunderstood something.

::: dom/bluetooth/BluetoothOppManager.cpp
@@ +392,3 @@
>     *  - Create an OPP connection by SendConnectRequest()
>     *  - After receiving the response, start to read file and send.
>     */

These comments are out-of-date, please update them.

@@ +1177,1 @@
>  

For the same reason here, I think we don't have to set mConnected to false in ReplyToDisconnect(), right?

Comment 11

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 1: final: Add capability of sending multiple files to a remove device, r=gyeh

* nits fixed, and send system message "bluetooth-opp-transfer-start" before sending the first PUT header of each file. (SendPutHeaderRequest())

Comment 12

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 741686 [details] [diff] [review]
patch 1: final: Add capability of sending multiple files to a remove device, r=gyeh

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

Looks good to me. 

Just want to point out here that, for the first file, "start" system message is broadcasted before sending Connect request (which is a bit different from what I expect at first but I think it should be ok), and here's the process of current patch:

* Broadcast "bluetooth-opp-transfer-start" for the first file
-> Connect request
<- response code of Success
-> PutHeader request
<- response code: Success
-> Put request
<- response code: Continue
-> Put request
<- response code: Continue
-> PutFinal request
<- response code: Success
* Broadcast "bluetooth-opp-transfer-complete" for the first file
* Broadcast "bluetooth-opp-transfer-start" for the second file
-> PutHeader request
<- response code: Success
-> Put request
<- response code: Continue
-> Put request
<- response code: Continue
-> PutFinal request
<- response code: Success
* Broadcast "bluetooth-opp-transfer-complete" for the second file
-> Disconnect request
<- response code: Success

Comment 13

[Eric Chou [:ericchou] [:echou]]

(In reply to Gina Yeh [:gyeh] [:ginayeh] from comment #12)
> Comment on attachment 741686 [details] [diff] [review]
> patch 1: v2: Add capability of sending multiple files to a remove device
> 
> Review of attachment 741686 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me. 
> 
> Just want to point out here that, for the first file, "start" system message
> is broadcasted before sending Connect request (which is a bit different from
> what I expect at first but I think it should be ok), and here's the process
> of current patch:
> 
> * Broadcast "bluetooth-opp-transfer-start" for the first file
> -> Connect request
> <- response code of Success
> -> PutHeader request
> <- response code: Success
> -> Put request
> <- response code: Continue
> -> Put request
> <- response code: Continue
> -> PutFinal request
> <- response code: Success
> * Broadcast "bluetooth-opp-transfer-complete" for the first file
> * Broadcast "bluetooth-opp-transfer-start" for the second file
> -> PutHeader request
> <- response code: Success
> -> Put request
> <- response code: Continue
> -> Put request
> <- response code: Continue
> -> PutFinal request
> <- response code: Success
> * Broadcast "bluetooth-opp-transfer-complete" for the second file
> -> Disconnect request
> <- response code: Success

Yes, it's a little different but it's easier to implement and I don't think it would be problem.

Comment 14

[Eric Chou [:ericchou] [:echou]]

This bug needs help from Gaia side as well. I'm going to reassign this bug to Ian and will test my patch along with his implementation.

Comment 15

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

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9406

Pointer to Github pull-request

Comment 16

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

Comment on attachment 741775 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9406

* Remove the filter of sharing number.
* Use a for loop to send all sharing files.
* This is a WIP patch.

Comment 17

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

(In reply to Ian Liu [:ianliu] from comment #16)
> Comment on attachment 741775 [details]
> Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9406
> 
> * Remove the filter of sharing number.
> * Use a for loop to send all sharing files.
> * This is a WIP patch.

Eric,
Please use "make reset-gaia" to flash with the patch. Thanks.

Comment 18

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/projects/birch/rev/a609b918e159

Comment 19

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 741775 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9406

Hi Ian,

I've tested the patch. Most of the time it works well, however, once I tried to send a number of photos (say, more than 10), somtimes the operation

  var getRequest = storage.get(file);

wouldn't return any response, which means neither getRequest.onsuccess nor getRequest.onerror would be called. Would you please take a look at this part?

Thank you.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a609b918e159

Comment 21

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

Because of gaia patch is not landed yet, I reopen the issue.

Comment 22

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

Comment on attachment 741775 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9406

Eric,
After refined the coding style of storage.get(file), it looks working fine for me.

Evelyn,
Could you please help to review my pull request? Thank you.

Comment 23

[Evelyn Hung]

(In reply to Ian Liu [:ianliu] from comment #21)
> Because of gaia patch is not landed yet, I reopen the issue.

I think it's better to file another issue for tracking Gaia part implementation. Please file one, and submit your patch there. Thanks.

Comment 24

[Evelyn Hung]

Comment on attachment 741775 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9406

I'll review on the new filed issue. Please request there again. Thanks.

Comment 25

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

Evelyn,
Thanks for your reviewing effort. Let's move the tracking Gaia part implementation to Bug 869287.

Comment 26

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Ian Liu [:ianliu] from comment #21)
> Because of gaia patch is not landed yet, I reopen the issue.

Should we close this bug now the that Gaia part lives in bug 869287?

Comment 27

[Eric Chou [:ericchou] [:echou]]

Close this bug since Gaia side implementation has been moved to bug 869287

Comment 29

[qingluo.wang]

Hi Daniel,

  Refer to bug 874298,I nominate this bug as tef?,please help to decide the priority.

Thank you.

Comment 30

[Alex Keybl [:akeybl]]

https://bugzilla.mozilla.org/show_bug.cgi?id=874298#c1 suggests that this is out of scope for v1.0.1

Comment 31

[Alex Keybl [:akeybl]]

Not blocking, please renominate if this is a blocker for you Daniel.

Comment 32

[Walter Chen[:ypwalter][:wachen]]

we should add a test cases for this.

Comment 33

[leo.bugzilla.gecko]

Dear Mozilla Team,

We should support this feature, so I nominate this bug as leo?.
Please check this.

Thanks.

Comment 34

[David Scravaglieri [:scravag]]

Leo will check with the Bluetooth Team to figure out if this feature is still requested as It will need work in gaia too.

Comment 35

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

Gaia part: Bug 869287 - [Bluetooth][File-Transfer] Remove the transfer restriction on the number of files

Comment 36

[leo.bugzilla.gecko]

OK. I checked the gaia part and nominate bug 869287 too.
Thanks for your help.

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/a83f9ca78c33

This definitely required some fixing up to apply to b2g18. Probably want to look it over to make sure all's well :)

Comment 38

[Ryan VanderMeulen [:RyanVM]]

Yep, so much for that idea. Backed out for bustage.
https://hg.mozilla.org/releases/mozilla-b2g18/rev/756245b5a882

https://tbpl.mozilla.org/php/getParsedLog.php?id=24006613&tree=Mozilla-B2g18
https://tbpl.mozilla.org/php/getParsedLog.php?id=24006566&tree=Mozilla-B2g18

Comment 39

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 1: for b2g18

* Rebased b2g18-specific patch

Comment 40

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/d85aa34a49f2

Comment 41

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/d85aa34a49f2

Comment 42

[Walter Chen[:ypwalter][:wachen]]

Verified fixed in 2013/06/16 V1train pvt build.
Gaia:     f2d6ed54a236e6e3b94f0abad9f0dacb8a1cc7b3
  B-D     2013-06-15 07:33:44
Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18/rev/15d9885034a0
BuildID   20130616070209
Version   18.0

Tried to transfer images and music. 
It worked pretty smooth,