999053 - The FTU ping fails to read ICC & Network SIM data from a DSDS device that has the SIM in the second slot with no SIM in the first slot

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Jason Smith [:jsmith]]

Build - 4/21/2014 Tarako 1.3T

STR

1. Setup a Tarako device with a SIM with mobile data in the second slot with no SIM the first slot
2. Flash the Tarako device
3. Establish a data connection using the 2nd SIM
4. Check the ping that gets sent out

Expected

The ping should use the ICC/network data from the SIM in the second slot, as it's currently the primary SIM if no SIM was included in the first slot.

Actual

The ping is sent out without ICC/network data from the SIM in the second slot.

Comment 1

[Jason Smith [:jsmith]]

This needs to be fixed for 1.3T & 1.4 DSDS devices to ensure that pings send out SIM data even if no SIM was included in the first slot, but the 2nd slot had the SIM.

Comment 2

[Jason Smith [:jsmith]]

Marshall - Can you take a look?

Comment 3

[Marshall Culpepper [:marshall_law]]

Bhavana just sent me a Tarako device, I'll try to repro once I have it (should be in a day or two)

Comment 4

[Joe Cheng [:jcheng] (please needinfo)]

hi Marshall, wonder if you have further update on this bug? Thanks

Comment 5

[Marshall Culpepper [:marshall_law]]

Hey Joe, sorry for lack of updates

I received the Tarako device from Bhavana last week and have a patch that I'm trying to test, but I haven't been able to see my device over ADB on my laptop. I'll make this my top priority today and try to get a patch posted for review ASAP.

Comment 6

[Jason Smith [:jsmith]]

If you can post a WIP as a github branch, then Naoki, myself, or John could test it.

Comment 7

[Hema Koka [:hema]]

(In reply to Marshall Culpepper [:marshall_law] from comment #5)
> Hey Joe, sorry for lack of updates
> 
> I received the Tarako device from Bhavana last week and have a patch that
> I'm trying to test, but I haven't been able to see my device over ADB on my
> laptop. I'll make this my top priority today and try to get a patch posted
> for review ASAP.

Marshall, any update? 

Thanks
Hema

Comment 8

[Marshall Culpepper [:marshall_law]]

(In reply to Jason Smith [:jsmith] from comment #6)
> If you can post a WIP as a github branch, then Naoki, myself, or John could
> test it.

Argh, I missed this. I just pushed this up on my remote here:
https://github.com/marshall/gaia/tree/bug995434_ftuPingSimLocked

I ended up giving up on the fight for Tarako and switched to the Flame so I could test this in a dual sim device.. I'm pulling from the active / connected SIM slot now instead of just the first connection object (which was apparently causing some uncaught exceptions).

Comment 9

[Jason Smith [:jsmith]]

Leaving needinfo on myself to test this with the custom branch.

Comment 10

[Faramarz Rashed]

:jsmith, can you test this and see if we can close it or not please?

Comment 11

[Joe Cheng [:jcheng] (please needinfo)]

adding QAWANTED

Comment 12

[Jason Smith [:jsmith]]

Sorry for the long delay. The problem with the patch included here is that it's against master, not 1.3T. I need this patch to be against 1.3T to be able to test this on Tarako.

The other option we could do here to test this on master is to apply the Flame RIL workaround to get the RIL working on Flame & test it on master.

Comment 13

[Mike Lien[:mlien]]

hi jason, could you help to provide more information about how to check the ping?

Comment 14

[Jason Smith [:jsmith]]

(In reply to Mike Lien[:mlien] from comment #13)
> hi jason, could you help to provide more information about how to check the
> ping?

You need to flash a custom Gaia build on top of a normal gaia build that changes https://github.com/mozilla-b2g/gaia/blob/master/Makefile#L133 to point to a mock HTTP server (e.g. https://github.com/jds2501/mockmetricsserver, which is ran on Google App Engine). Then, after FTE starts up, you need to establish a network connection and wait for the ping to be sent out. Last, you'll need to check server side that the ping was received and has the correct JSON information included according to https://wiki.mozilla.org/FirefoxOS/Metrics#FTU_ping.

Note #1 - you'll probably want to change the ping interval to make this easier to see this work vs. not work, so you'll need to change https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/ftu_ping.js#L27 to 15 * 1000 to see the ping fire every 15 seconds.

Note #2 - you'll probably want to turn on debug to get more information in logcat. You can do that by changing this line - https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/ftu_ping.js#L12 to true for DEBUG.

Comment 15

[Mike Lien[:mlien]]

hi Jason, if this only happen on SIM 1 is empty?
Input both SIM or using SIM 1 + leave SIM 2 empty are fine or not?
If only one case will encounter FTU ping fail, I don't think it's a blocker

Comment 16

[Jason Smith [:jsmith]]

(In reply to Mike Lien[:mlien] from comment #15)
> hi Jason, if this only happen on SIM 1 is empty?
> Input both SIM or using SIM 1 + leave SIM 2 empty are fine or not?
> If only one case will encounter FTU ping fail, I don't think it's a blocker

Per discussion with product, we can't lose any in country data at all for measuring country statistics when the phone is released. That means any case that causes us to have no way to detect that information is considered a blocker, as the metrics team will be unable to accurately determine the number of phones shipped in our target markets. We've made the mistake historically of shipping a ping with inaccurate way of measuring in country statistics before which resulted in a lot of frustration at the top of Mozilla organization, so I don't think it makes sense to me to take any risk on any inaccuracies on country-specific metrics.

Comment 17

[Mike Lien[:mlien]]

per comment 12, ni Marshall to update new patch for v1.3t
hi Marshall, could we have the patch for v1.3t?

Comment 18

[Marshall Culpepper [:marshall_law]]

apologies, let me rebase against 1.3t and give a new link

Comment 19

[Gregor Wagner [:gwagner]]

any updates here?

Comment 20

[Marshall Culpepper [:marshall_law]]

Jason, I've rebased and pushed to my remote here:
https://github.com/marshall/gaia/tree/bug995434_ftuPingSimLocked_v1.3t

I re-tested locally and confirm that this works for only having a SIM in slot #2 on my tarako

Comment 21

[Gregor Wagner [:gwagner]]

(In reply to Marshall Culpepper [:marshall_law] from comment #20)
> Jason, I've rebased and pushed to my remote here:
> https://github.com/marshall/gaia/tree/bug995434_ftuPingSimLocked_v1.3t
> 
> I re-tested locally and confirm that this works for only having a SIM in
> slot #2 on my tarako

Can we get this reviewed and landed asap?

Comment 22

[Jason Smith [:jsmith]]

(In reply to Gregor Wagner [:gwagner] from comment #21)
> (In reply to Marshall Culpepper [:marshall_law] from comment #20)
> > Jason, I've rebased and pushed to my remote here:
> > https://github.com/marshall/gaia/tree/bug995434_ftuPingSimLocked_v1.3t
> > 
> > I re-tested locally and confirm that this works for only having a SIM in
> > slot #2 on my tarako
> 
> Can we get this reviewed and landed asap?

Yes, let's do that. I'm not going to be able to test this from where I'm at right now since I've only got a T-Mobile SIM with me that doesn't work in a second slot right now (known issue). I'll leave needinfo on Mike here to test this though following these directions:

1. Flash the latest 1.3T Tarako build
2. Pull Gaia from https://github.com/mozilla-b2g/gaia/
3. Modify https://github.com/mozilla-b2g/gaia/blob/v1.3t/Makefile#L119 to point to http://mockmetricsserver.appspot.com/
4. Modify https://github.com/mozilla-b2g/gaia/blob/v1.3t/apps/system/js/ftu_ping.js#L27 to 15 * 1000 (i.e. 15 seconds)
5. Flash Gaia using "MOZILLA_OFFICIAL=1 make production"
6. After the FTE comes up, enable your data connection
7. Wait patiently for 15 seconds
8. Comment here that the testing is complete & I'll look to see if the ping came through

Comment 23

[Jason Smith [:jsmith]]

Actually one change to the above directions - you need to pull from https://github.com/marshall/gaia/tree/bug995434_ftuPingSimLocked_v1.3t, not the gaia tree, as that has the patch here.

Comment 24

[Mike Lien[:mlien]]

test 20140527164004 build with patch from comment 20
below are FtuPing logs

I/GeckoDump(  495): [FtuPing] http://mockmetricsserver.appspot.com//b3a32c3f-fee8-4fa1-9937-0e06656cdd44/ftu/FirefoxOS/28.1/tarako%2F1.3.0t%2Fnightly/20140527164004
I/GeckoDump(  495): [FtuPing] {"ver":3,"screenHeight":480,"screenWidth":320,"devicePixelRatio":1,"locale":"en-US","pingID":"b3a32c3f-fee8-4fa1-9937-0e06656cdd44","activationTime":1401249795031,"deviceinfo.os":"1.3.0.0-prerelease","deviceinfo.software":"Boot2Gecko 1.3.0.0-prerelease","deviceinfo.platform_build_id":"20140527164004","deviceinfo.platform_version":"28.1","deviceinfo.product_model":"sp6821a","deviceinfo.firmware_revision":"","deviceinfo.hardware":"sp8810","deviceinfo.update_channel":"tarako/1.3.0t/nightly","network":{"operator":"CHT","mnc":"92","mcc":"466"},"icc":{"mnc":"92","mcc":"466","spn":null},"pingTime":1401255361804}

Comment 25

[Jason Smith [:jsmith]]

Here's what I see server-side:

 {"ver":3,"screenHeight":480,"screenWidth":320,"devicePixelRatio":1,"locale":"en-US","pingID":"b3a32c3f-fee8-4fa1-9937-0e06656cdd44","activationTime":1401249795031,"deviceinfo.os":"1.3.0.0-prerelease","deviceinfo.software":"Boot2Gecko 1.3.0.0-prerelease","deviceinfo.platform_build_id":"20140527164004","deviceinfo.platform_version":"28.1","deviceinfo.product_model":"sp6821a","deviceinfo.firmware_revision":"","deviceinfo.hardware":"sp8810","deviceinfo.update_channel":"tarako/1.3.0t/nightly","network":{"operator":"CHT","mnc":"92","mcc":"466"},"icc":{"mnc":"92","mcc":"466","spn":null},"pingTime":1401255361804} 

Which means that this patch does indeed work. Let's get this in review!

Comment 26

[Hema Koka [:hema]]

Marshall, lets get the patch reviewed and get a closure on this.

Thanks
Hema

Comment 27

[Marshall Culpepper [:marshall_law]]

Attachment: Multisim fixes - v1

Updated with new unit tests

Comment 28

[[:fabrice] Fabrice Desré]

Marshall, Travis is complaining...

Comment 29

[Marshall Culpepper [:marshall_law]]

Thanks

Comment 30

[Marshall Culpepper [:marshall_law]]

These failures look to be in the tests for the contacts app. I've retriggered them to see if they are intermitent

Comment 31

[Marshall Culpepper [:marshall_law]]

One of the failures went away after a retrigger, but the other stayed. I did a rebase to grab the latest commits in case I was missing any, and have a new build queued here:

https://travis-ci.org/mozilla-b2g/gaia/builds/26404624

Comment 32

[Marshall Culpepper [:marshall_law]]

It looks like another test was monkey patching the MockSIMSlotManager similar to the way I modified it for my testing purposes. I've removed those changed and switched to the same method in my latest push (should know soon if this makes Travis happy, but the test passes locally now)

Comment 33

[Marshall Culpepper [:marshall_law]]

https://github.com/mozilla-b2g/gaia/commit/335486c42498fa7a93c21e4d6121199728602ab8

Comment 34

[Gregor Wagner [:gwagner]]

Marshall, are we done here? Do we have to merge to other branches as well?

Comment 35

[Jason Smith [:jsmith]]

(In reply to Gregor Wagner [:gwagner] from comment #34)
> Marshall, are we done here? Do we have to merge to other branches as well?

This needs to land on master & 1.4 since both of those branches have DSDS support.

Comment 36

[Gregor Wagner [:gwagner]]

(In reply to Jason Smith [:jsmith] from comment #35)
> (In reply to Gregor Wagner [:gwagner] from comment #34)
> > Marshall, are we done here? Do we have to merge to other branches as well?
> 
> This needs to land on master & 1.4 since both of those branches have DSDS
> support.

Ok can we land on master please so we can close it?

Comment 37

[Ivan Tsay (:ITsay)]

> Ok can we land on master please so we can close it?

Please also land it in v1.4 since this bug also affects DSDS in v1.4.

Comment 38

[Ivan Tsay (:ITsay)]

Hi Marshall,

May I know if this will be landed soon? This is one of the patch that our partner is waiting for preparing their v1.4 dolphin build. Thanks.

Comment 39

[Marshall Culpepper [:marshall_law]]

I'll post 1.4 and master patches today ASAP

Comment 40

[Marshall Culpepper [:marshall_law]]

PRs are open, I'm monitoring Travis and Try builds:
https://github.com/mozilla-b2g/gaia/pull/20104
https://github.com/mozilla-b2g/gaia/pull/20106

https://tbpl.mozilla.org/?tree=Try&rev=ba3d8fcbb9b4

Comment 41

[Marshall Culpepper [:marshall_law]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/812f2f24df7b02e6daae33b02d1749058b977b56
master: https://github.com/mozilla-b2g/gaia/commit/e7db1765f5895d6baa163affdcefe133f469c31b

Comment 42

[Gregor Wagner [:gwagner]]

Fabrice reviewed here. I think we can count this as 1.4 approval from him.