Closed Bug 817972 Opened 11 years ago Closed 11 years ago

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

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:leo+, blocking-basecamp:-, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

VERIFIED FIXED
1.1 QE3 (26jun)
blocking-b2g leo+
blocking-basecamp -
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- fixed

People

(Reporter: iliu, Assigned: echou)

References

()

Details

(Whiteboard: [fixed-in-birch])

Attachments

(3 files, 1 obsolete file)

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.
Flags: needinfo?(kyee)
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.
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.
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.
Renominate if needed
blocking-basecamp: ? → -
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.
Flags: needinfo?(kyee)
Hey developers and QA,
Thanks for all your confirmation.
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.
Assignee: iliu → echou
Attachment #741220 - Flags: review?(gyeh)
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?
Attachment #741220 - Flags: review?(gyeh)
* nits fixed, and send system message "bluetooth-opp-transfer-start" before sending the first PUT header of each file. (SendPutHeaderRequest())
Attachment #741220 - Attachment is obsolete: true
Attachment #741686 - Flags: review?(gyeh)
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
Attachment #741686 - Flags: review?(gyeh) → review+
(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.
Attachment #741686 - Attachment description: patch 1: v2: Add capability of sending multiple files to a remove device → patch 1: final: Add capability of sending multiple files to a remove device, r=gyeh
Attachment #741686 - Flags: review+
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.
Assignee: echou → iliu
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.
Attachment #741775 - Flags: feedback?(echou)
(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.
Depends on: 830551
Whiteboard: [fixed-in-birch]
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.
Attachment #741775 - Flags: feedback?(echou)
https://hg.mozilla.org/mozilla-central/rev/a609b918e159
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Because of gaia patch is not landed yet, I reopen the issue.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Attachment #741775 - Flags: review?(ehung)
(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 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.
Attachment #741775 - Flags: review?(ehung)
Evelyn,
Thanks for your reviewing effort. Let's move the tracking Gaia part implementation to Bug 869287.
Depends on: 869287
(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?
Close this bug since Gaia side implementation has been moved to bug 869287
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Hi Daniel,

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

Thank you.
blocking-b2g: --- → tef?
Flags: needinfo?(dcoloma)
https://bugzilla.mozilla.org/show_bug.cgi?id=874298#c1 suggests that this is out of scope for v1.0.1
Not blocking, please renominate if this is a blocker for you Daniel.
blocking-b2g: tef? → -
Assignee: iliu → echou
Component: Gaia::Bluetooth File Transfer → Bluetooth
we should add a test cases for this.
Flags: in-moztrap?
Flags: needinfo?(dcoloma)
Dear Mozilla Team,

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

Thanks.
blocking-b2g: - → leo?
Leo will check with the Bluetooth Team to figure out if this feature is still requested as It will need work in gaia too.
Flags: needinfo?(leo.bugzilla.gecko)
Gaia part: Bug 869287 - [Bluetooth][File-Transfer] Remove the transfer restriction on the number of files
OK. I checked the gaia part and nominate bug 869287 too.
Thanks for your help.
Flags: needinfo?(leo.bugzilla.gecko)
blocking-b2g: leo? → leo+
Target Milestone: --- → 1.1 QE3
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 :)
Flags: in-moztrap? → in-moztrap+
* Rebased b2g18-specific patch
Flags: needinfo?(echou)
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,
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.