Closed Bug 1086880 Opened 6 years ago Closed 6 years ago

Mobile ID is broken with v188 RIL

Categories

(Firefox OS Graveyard :: RIL, defect)

All
Gonk (Firefox OS)
defect
Not set
blocker

Tracking

(blocking-b2g:2.0+, firefox34 wontfix, firefox35 wontfix, firefox36 fixed, b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

RESOLVED FIXED
2.1 S7 (24Oct)
blocking-b2g 2.0+
Tracking Status
firefox34 --- wontfix
firefox35 --- wontfix
firefox36 --- fixed
b2g-v2.0 --- fixed
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: ferjm, Assigned: ferjm)

References

Details

(Keywords: regression, Whiteboard: ft:Loop [blocking][platform][tef-triage])

Attachments

(2 files)

It seems that the RIL that goes with the latest v188 is reporting empty SIM slots as available and this breaks the Mobile ID API.

Attached is a small log with the Mobile ID API debug output. Note the "iccInfo {"iccId":null,"mcc":"","mnc":"","msisdn":null,"operator":null,"serviceId":0,"roaming":false}" line. That shouldn't be appearing if no SIM is inserted in that SIM slot.
Attached file Mobile ID log
Blocks: mobileid
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → All
[Blocking Requested - why for this release]:

Fernando, did this work on previous images like v184 and v180?  if yes, this should be blocking.
blocking-b2g: --- → 2.1?
Keywords: regression
(In reply to Tony Chung [:tchung] from comment #2)
> [Blocking Requested - why for this release]:
> 
> Fernando, did this work on previous images like v184 and v180?  if yes, this
> should be blocking.

yes, it was working fine with those previous images.

It seems to be a vendor issue because with v188 base and removing QC RIL so using Mozilla one we have not reproduced this issue. 

In that case, Tony, I am not sure about the blocking flag to nominate... but be aware that we need to have this fixed also in 2.0 as Loop is going to be landed with that version and with that base image or higher.
Can you share the output of this command on the v188 build?

  adb shell cat /firmware/verinfo/ver_info.txt
Flags: needinfo?(ferjmoreno)
(In reply to Diego Wilson [:diego] from comment #4)
> Can you share the output of this command on the v188 build?
> 
>   adb shell cat /firmware/verinfo/ver_info.txt

M8610AAAAANFYD1530.1
Flags: needinfo?(ferjmoreno)
(In reply to Maria Angeles Oteo (:oteo) from comment #5)
> (In reply to Diego Wilson [:diego] from comment #4)
> > Can you share the output of this command on the v188 build?
> > 
> >   adb shell cat /firmware/verinfo/ver_info.txt
> 
> M8610AAAAANFYD1530.1

Anshul,

Could the RIL update be causing this issue?
Flags: needinfo?(anshulj)
(In reply to Fernando Jiménez Moreno [:ferjm] from comment #0)
> It seems that the RIL that goes with the latest v188 is reporting empty SIM
> slots as available and this breaks the Mobile ID API.
> 
> Attached is a small log with the Mobile ID API debug output. Note the
> "iccInfo
> {"iccId":null,"mcc":"","mnc":"","msisdn":null,"operator":null,"serviceId":0,
> "roaming":false}" line. That shouldn't be appearing if no SIM is inserted in
> that SIM slot.
Fernando, where do you see the SIM is reported present, on the UI or using some API? Note that in the above message iccId is null indicating that the SIM is absent. In addition to providing build info as per comment #4 please also provide the build info for the build in which this scenario was working for you.
Flags: needinfo?(anshulj)
Flags: needinfo?(ferjmoreno)
> In addition to providing build info as per comment #4 please
> also provide the build info for the build in which this scenario was working
> for you.

I've tested this with many different base builds as coming from the vendor (180, 184 and 187) and the failure can be reproduced in all of them. Unfortunately, we have not detected this issue before as we normally flash new builds on top of the vendor ones.
Sorry I'm no expert so would like to double confirm:

1. this is really an issue but not false alarm? (per comment#7 I guess might related to the test method)
2. If it's an issue, the flow will be: a QCT patch, then T2M to integrate, then release v189, is this right understanding?
3. what's the user impact of this problem?

Just trying to understand if this is a blocker for v188 to public. (ie make v188 available to all Flame users in the world) And the possible way forward. Thanks.
Flags: needinfo?(oteo)
(In reply to Wesly Huang from comment #9)
> Sorry I'm no expert so would like to double confirm:
> 
> 1. this is really an issue but not false alarm? (per comment#7 I guess might
> related to the test method)

Yes, this is a real issue

> 2. If it's an issue, the flow will be: a QCT patch, then T2M to integrate,
> then release v189, is this right understanding?

The issue is that Commercial and Mozilla RIL behaves differently. Our tests have been focused in Mozilla RIL but with commercial RIL this issue appears. The alternative is doing a workaround in Gecko to handle properly the Commercial RIL behaviour.

> 3. what's the user impact of this problem?
> 

This is a blocker for Loop, this feature (MSISDN Verification) should be able to be used when no SIM Card is inserted, and without a solution for this issue, it won't be possible.

> Just trying to understand if this is a blocker for v188 to public. (ie make
> v188 available to all Flame users in the world) And the possible way
> forward. Thanks.

As I said, afaik this issue is currently only blocking Loop, not sure if any other app might be affected, I doubt so.
Flags: needinfo?(oteo)
[Blocking Requested - why for this release]:

This is a blocker for Loop, this feature (MSISDN Verification) should be able to be used when no SIM Card is inserted, and without a solution for this issue, it won't be possible.

Fernando will prepare the workaround patch in Gecko (I think it will be quicker than requesting the RIL change), so assigning the bug to him and nominating the bug to 2.0 as this is a must-have for Loop.
Assignee: nobody → ferjmoreno
Blocks: 1036490
Status: NEW → ASSIGNED
blocking-b2g: 2.1? → 2.0?
Flags: needinfo?(ferjmoreno)
Severity: normal → blocker
Whiteboard: ft:Loop [blocking][platform]
(In reply to Anshul from comment #7)
> (In reply to Fernando Jiménez Moreno [:ferjm] from comment #0)
> > It seems that the RIL that goes with the latest v188 is reporting empty SIM
> > slots as available and this breaks the Mobile ID API.
> > 
> > Attached is a small log with the Mobile ID API debug output. Note the
> > "iccInfo
> > {"iccId":null,"mcc":"","mnc":"","msisdn":null,"operator":null,"serviceId":0,
> > "roaming":false}" line. That shouldn't be appearing if no SIM is inserted in
> > that SIM slot.
> Fernando, where do you see the SIM is reported present, on the UI or using
> some API? Note that in the above message iccId is null indicating that the
> SIM is absent.

In this case I am considering that the SIM is present if it is listed in the iccInfo array. Which now I see that it's an incorrect assumption.

I believe the root of this issue is that MOZ RIL and QC RIL behaves differently to report absent SIMs. On the MOZ RIL, if there is no SIM we don't list it within IccInfo, but with MOZ QC, it seems that we do list it with null or empty values.

As mentioned above, I'll write a patch for the Mobile ID API to make the API work with both RILs. I'm traveling today, but I'll try to upload the patch for review tonight.
Whiteboard: ft:Loop [blocking][platform] → ft:Loop [blocking][platform][tef-triage]
Jonas: I'm told daniel/maria needinfo'd you on this, but I don't see it here (email?).  Hit me up in IRC if you have any questions.  Thanks
Flags: needinfo?(jonas)
(In reply to Randell Jesup [:jesup] from comment #13)
> Jonas: I'm told daniel/maria needinfo'd you on this, but I don't see it here
> (email?).  Hit me up in IRC if you have any questions.  Thanks

Hey Randell! it was not for this bug but for bug 1079974 and it seems Jonas already provided the required feedback. Sorry for the confusion... and thanks!
Flags: needinfo?(jonas)
triaged to block 2.0+.  ni? bhavana to follow up with vendor
blocking-b2g: 2.0? → 2.0+
Whiteboard: ft:Loop [blocking][platform][tef-triage] → ft:Loop [blocking][platform][tef-triage][NPOTB]
(In reply to Fernando Jiménez Moreno [:ferjm] from comment #12)
> (In reply to Anshul from comment #7)
> > (In reply to Fernando Jiménez Moreno [:ferjm] from comment #0)
> > > It seems that the RIL that goes with the latest v188 is reporting empty SIM
> > > slots as available and this breaks the Mobile ID API.
> > > 
> > > Attached is a small log with the Mobile ID API debug output. Note the
> > > "iccInfo
> > > {"iccId":null,"mcc":"","mnc":"","msisdn":null,"operator":null,"serviceId":0,
> > > "roaming":false}" line. That shouldn't be appearing if no SIM is inserted in
> > > that SIM slot.
> > Fernando, where do you see the SIM is reported present, on the UI or using
> > some API? Note that in the above message iccId is null indicating that the
> > SIM is absent.
> 
> In this case I am considering that the SIM is present if it is listed in the
> iccInfo array. Which now I see that it's an incorrect assumption.
> 
Fernando, you should look at how Gaia handles this scenario as Gaia can properly show or hide sim absent icon based on the availability of IccId. What you encountered is really an issue due to lack of proper documentation of Gaia APIs or unclear APIs.

> I believe the root of this issue is that MOZ RIL and QC RIL behaves
> differently to report absent SIMs. On the MOZ RIL, if there is no SIM we
> don't list it within IccInfo, but with MOZ QC, it seems that we do list it
> with null or empty values.
> 
> As mentioned above, I'll write a patch for the Mobile ID API to make the API
> work with both RILs. I'm traveling today, but I'll try to upload the patch
> for review tonight.
Attached patch v1Splinter Review
Attachment #8510219 - Flags: review?(spenrose)
Attachment #8510219 - Flags: review?(spenrose)
Attachment #8510219 - Flags: review?(spenrose)
Comment on attachment 8510219 [details] [diff] [review]
v1

Thanks Fernando!
Attachment #8510219 - Flags: review?(spenrose) → review+
Whiteboard: ft:Loop [blocking][platform][tef-triage][NPOTB] → ft:Loop [blocking][platform][tef-triage][NPOTB][patch available]
https://hg.mozilla.org/mozilla-central/rev/d09af495bc92
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S7 (24Oct)
Comment on attachment 8510219 [details] [diff] [review]
v1

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Mobile ID
User impact if declined: This issue completely breaks the Mobile ID functionality with QC RIL.
Testing completed: Local tests and unit test added.
Risk to taking this patch (and alternatives if risky): Very low. We only added a more specific condition to skip SIMs.
String or UUID changes made by this patch: none
Attachment #8510219 - Flags: approval-mozilla-b2g34?
Attachment #8510219 - Flags: approval-mozilla-b2g32?
Approving, and Ni anshul so he's aware of this blocking change. Anshul please refer to comment #11 here for he need.
Flags: needinfo?(anshulj)
Attachment #8510219 - Flags: approval-mozilla-b2g34?
Attachment #8510219 - Flags: approval-mozilla-b2g34+
Attachment #8510219 - Flags: approval-mozilla-b2g32?
Attachment #8510219 - Flags: approval-mozilla-b2g32+
Flags: needinfo?(anshulj)
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/1f3980c9599a
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/b32862f6c1a0
Whiteboard: ft:Loop [blocking][platform][tef-triage][NPOTB][patch available] → ft:Loop [blocking][platform][tef-triage]
You need to log in before you can comment on or make changes to this bug.