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)
Tracking
(blocking-b2g:2.5+, 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.
Reporter | ||
Comment 1•9 years ago
|
||
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.
Assignee | ||
Comment 2•9 years ago
|
||
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)
Comment 3•9 years ago
|
||
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)
Updated•9 years ago
|
Assignee | ||
Comment 4•9 years ago
|
||
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)
Reporter | ||
Comment 5•9 years ago
|
||
(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.
Assignee | ||
Comment 6•9 years ago
|
||
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)
Comment 7•9 years ago
|
||
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)
Updated•9 years ago
|
Flags: needinfo?(lissyx+mozillians)
Assignee | ||
Comment 9•9 years ago
|
||
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.
Comment 10•9 years ago
|
||
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)
Assignee | ||
Comment 11•9 years ago
|
||
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
Comment 12•9 years ago
|
||
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
Comment 13•9 years ago
|
||
(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)
Assignee | ||
Comment 14•9 years ago
|
||
WIP - Automatically set the default protocol based on whether the sdcard storage area can be shared via UMS or not.
Flags: needinfo?(dhylands)
Assignee | ||
Comment 15•9 years ago
|
||
(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
Assignee | ||
Comment 16•9 years ago
|
||
Actually, it went into the system app, not settings, so I'm sure who the right person is. Perhaps Alive?
Flags: needinfo?(drs)
Comment 17•9 years ago
|
||
(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)
Comment 18•9 years ago
|
||
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)
Comment 19•9 years ago
|
||
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)
Updated•9 years ago
|
Keywords: leave-open
Assignee | ||
Comment 20•9 years ago
|
||
Comment on attachment 8621359 [details] [review] Enable MTP by default, workaround for feature detection. lgtm
Attachment #8621359 -
Flags: review?(dhylands) → review+
Comment 21•9 years ago
|
||
Comment on attachment 8621359 [details] [review] Enable MTP by default, workaround for feature detection. https://github.com/mozilla-b2g/gaia/commit/05240fe5bcd785eab4f009c90527c87040503812
Comment 22•9 years ago
|
||
I think I don't have no other comments for current status. Clear ni.
Flags: needinfo?(alchen)
Updated•9 years ago
|
blocking-b2g: spark+ → 2.5+
Comment 24•9 years ago
|
||
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)
Assignee | ||
Comment 25•9 years ago
|
||
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)
Comment 26•9 years ago
|
||
You can assert the behavior by writing unit tests https://github.com/mozilla-b2g/gaia/blob/master/apps/system/test/unit/usb_storage_test.js https://github.com/mozilla-b2g/gaia#unit-tests
Flags: needinfo?(timdream)
Comment 27•9 years ago
|
||
Dave, assigning to you since you already have a patch in-flight?
Assignee: nobody → dhylands
Status: NEW → ASSIGNED
Comment 28•9 years ago
|
||
Assignee | ||
Comment 29•9 years ago
|
||
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
Assignee | ||
Comment 30•9 years ago
|
||
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)
Assignee | ||
Updated•9 years ago
|
Attachment #8648218 -
Flags: review?(iliu) → review?(timdream)
Comment 31•9 years ago
|
||
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+
Assignee | ||
Comment 32•9 years ago
|
||
https://github.com/dhylands/gaia/commit/2d0511d0cd34af5b0a14803acea7ca48136af45c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 33•9 years ago
|
||
Doug - I landed this on master. Do you just want me to back out the change to distros/spark/settings.json ?
Flags: needinfo?(drs)
Updated•9 years ago
|
Comment 35•8 years ago
|
||
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?]
status-b2g-v2.5:
--- → verified
Flags: needinfo?(jmercado)
Updated•8 years ago
|
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.
Description
•