Closed Bug 1171556 Opened 9 years ago Closed 9 years ago

Aries: default should be MTP

Categories

(Firefox OS Graveyard :: Gaia::System::Storage Mgmt, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.5+, b2g-v2.5 verified, b2g-master verified)

VERIFIED FIXED
FxOS-S5 (21Aug)
blocking-b2g 2.5+
Tracking Status
b2g-v2.5 --- verified
b2g-master --- verified

People

(Reporter: hub, Assigned: dhylands)

References

Details

(Keywords: smoketest, Whiteboard: [spark])

Attachments

(2 files, 1 obsolete file)

We should have MTP set by default on Aries since Mass Storage doesn't seem to work.

This is not bug 1061090.
The Mass Storage problem might be a regression making this bug request obsolete.... because if Mass Storage do work then there is no reason to use MTP by default.
See Also: → 1169815
UMS definitely won't work on the aries for the internal storage (same thing for Nexus 4/5).

UMS should work for the external sdcard, but for some reason it isn't (I'll look into that over in bug 1169815)
See Also: → 1172155
Dave, can you take this? This is higher priority within the blocker list. Thanks for your explanations of the situation.
blocking-b2g: --- → spark+
Flags: needinfo?(dhylands)
Blocks: spark-device
Priority: -- → P1
Whiteboard: [spark]
See Also: → 1170244
Note: that OSX doesn't have any support for MTP by default.

A quick google search brought up this: https://www.android.com/intl/en_ca/filetransfer/ which may add MTP support to OSX (I don't have a Mac to test)
Assignee: nobody → dhylands
Flags: needinfo?(dhylands)
(In reply to Dave Hylands [:dhylands] from comment #4)
> Note: that OSX doesn't have any support for MTP by default.
> 
> A quick google search brought up this:
> https://www.android.com/intl/en_ca/filetransfer/ which may add MTP support
> to OSX (I don't have a Mac to test)

Yes this is what people use to copy files out of their MTP Android phone on a Mac.
So it turns out that the default setting for USB Transfer mode is set here:
https://github.com/mozilla-b2g/gaia/blob/master/build/config/common-settings.json#L238

Does anybody know how to do phone specific settings in gaia?
Flags: needinfo?(drs)
Fabrice, I don't know of any way to add device-specific settings. Do you?

If there's no other reasonable option, we could use the Spark distro to set it:
https://github.com/mozilla-b2g/gaia/blob/master/distros/spark/settings.json
Flags: needinfo?(drs) → needinfo?(fabrice)
Also adding Alex for comment 7.
Flags: needinfo?(lissyx+mozillians)
Flags: needinfo?(lissyx+mozillians)
Perhaps another way to do this is rather than make it device specifc, lets make it use a heuristic.

Set the default to some value which means set the default the first time the code runs.

If the internal sdcard is shareable, then we set the default to UMS. This covers all of our shipping devices.

If the internal sdcard is NOT shareable, then we set the default to MTP. This covers the Nexus 4/5 and aries devices.
We only have support for device specific prefs, not settings. Should not be hard to add if we really need it though.

Dave, that looks fine to me. Would that be in the volume management code?
Flags: needinfo?(fabrice)
Actually, I think it would be in the settings app.

The code that reads the setting is here:
https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/panels/usb_storage/usb_transfer.js#L32-L36
Because Bug 1171270 was resolved as WontFix, I am upgrading this issue to a Smoketest blocker, as this blocks case https://moztrap.mozilla.org/manage/case/6072/
Keywords: smoketest
(In reply to Dave Hylands [:dhylands] from comment #11)
> Actually, I think it would be in the settings app.
> 
> The code that reads the setting is here:
> https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/panels/
> usb_storage/usb_transfer.js#L32-L36

Dave, based on where this has gone, are you the correct assignee? If not, who can take it?

If this is going into Settings, I think it would be Arthur Chen (:arthurcc).
Flags: needinfo?(dhylands)
WIP - Automatically set the default protocol based on whether the sdcard storage area can be shared via UMS or not.
Flags: needinfo?(dhylands)
(In reply to Doug Sherk (:drs) (use needinfo?) from comment #13)
> (In reply to Dave Hylands [:dhylands] from comment #11)
> > Actually, I think it would be in the settings app.
> > 
> > The code that reads the setting is here:
> > https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/panels/
> > usb_storage/usb_transfer.js#L32-L36
> 
> Dave, based on where this has gone, are you the correct assignee? If not,
> who can take it?
> 
> If this is going into Settings, I think it would be Arthur Chen (:arthurcc).

Yes - this would be going into settings.

I've coded up a patch which seems to work, but it needs some tests, which I'm not quite sure how best to write. So I've attached my PR for what I've done so far, and changed the assignee to be Arthur.
Assignee: dhylands → arthur.chen
Actually, it went into the system app, not settings, so I'm sure who the right person is. Perhaps Alive?
Flags: needinfo?(drs)
(In reply to Dave Hylands [:dhylands] from comment #16)
> Actually, it went into the system app, not settings, so I'm sure who the
> right person is. Perhaps Alive?

The Systems division page doesn't show any peers or owners for this sub-module:
https://wiki.mozilla.org/Gaia/System/Division#System:_Storage_Mgmt

From a quick |git blame|, it looks like Fred Lin (:gasolin) owns this code. Fred, could you help us take this to completion?
Assignee: arthur.chen → nobody
Flags: needinfo?(drs) → needinfo?(gasolin)
Ian is recently working on mass storage, and has been initiated related API discussions with peripheral team, I think he is the best person to help.

Also ni alphan (gecko api) that we've discussed about such case.
Flags: needinfo?(iliu)
Flags: needinfo?(gasolin)
Flags: needinfo?(alchen)
This is a temporary workaround until Ian cleans up attachment 8617586 [details] [review]. We should back this out once the real patch is ready.
Attachment #8621359 - Flags: review?(dhylands)
Comment on attachment 8621359 [details] [review]
Enable MTP by default, workaround for feature detection.

lgtm
Attachment #8621359 - Flags: review?(dhylands) → review+
I think I don't have no other comments for current status.
Clear ni.
Flags: needinfo?(alchen)
Blocks: 1175162
No longer depends on: 1175162
blocking-b2g: spark+ → 2.5+
I feel hard to understand the ins and outs for the matter. So I have no comment for the workaround solution. Clean ni.
Flags: needinfo?(iliu)
Tim,

I've coded up the changes and attached them to this bug. I'm not sure how to write the tests. This is for the settings app. Can you assign this to someone?
Flags: needinfo?(timdream)
Dave, assigning to you since you already have a patch in-flight?
Assignee: nobody → dhylands
Status: NEW → ASSIGNED
Comment on attachment 8617586 [details] [review]
Default to MTP if UMS not supported on internal sdcard.

Replaced by pull request
Attachment #8617586 - Attachment is obsolete: true
Comment on attachment 8648218 [details] [review]
[gaia] dhylands:bug-1171556-auto-usb > mozilla-b2g:master

Not quite sure who should review this. Please feel free to reassign if there is somebody more appropriate
Attachment #8648218 - Flags: review?(iliu)
Attachment #8648218 - Flags: review?(iliu) → review?(timdream)
Comment on attachment 8648218 [details] [review]
[gaia] dhylands:bug-1171556-auto-usb > mozilla-b2g:master

Looks good with tests asserting what this does.
Attachment #8648218 - Flags: review?(timdream) → review+
https://github.com/dhylands/gaia/commit/2d0511d0cd34af5b0a14803acea7ca48136af45c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Doug - I landed this on master.

Do you just want me to back out the change to  distros/spark/settings.json ?
Flags: needinfo?(drs)
Yes, please.
Flags: needinfo?(drs)
Keywords: leave-open
Target Milestone: --- → FxOS-S5 (21Aug)
This issue is verified on Aries. Confirmed that MTP is the default USB Storage transfer protocol on Aries.

Device: Aries 2.6 Master
BuildID: 20151210122424
Gaia: 7e962276913bd4da7ce5fa7540767107ce322c78
Gecko: 412e4d7ce98ca4dbc37de133d0f26d7e1a59946f
Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56
Version: 45.0a1 (2.6) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Device: Aries 2.5
BuildID: 20151209171644
Gaia: 7ca639a7bb0bacf27f548841c52617bfc0e3b21f
Gecko: a35e8eb98969970d1af28b265bf99a9edd11e9c2
Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56
Version: 44.0a2 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: