[b2g-ril] SIM PIN requested after booting/rebooting with airplane mode on (mozilla-b2g18 only)

RESOLVED FIXED in Firefox 22

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jaoo, Assigned: m4)

Tracking

Trunk
mozilla22
All
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:tef+, firefox20 wontfix, firefox21 wontfix, firefox22 fixed, b2g18+ fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

Attachments

(4 attachments, 2 obsolete attachments)

Created attachment 723376 [details]
|adb logcat| output RIL debug enabled.

While trying to uplift bugs 822522 and 837755 to mozilla-b2g18 release repo I've noticed that the phone prompts the user to enter the PIN code even if the phone boots/reboots while airplane mode is on. I'm seeing issue in mozilla-b2g18 release repo.

It seems that something's fishy in the responses for `REQUEST_GET_SIM_STATUS` parcels. Log attached.
(Reporter)

Updated

4 years ago
Blocks: 710489
(Reporter)

Comment 1

4 years ago
Marking as tef? since the issue is also happening with shipping RIL.
blocking-b2g: --- → tef?
Assignee: nobody → anshulj
blocking-b2g: tef? → tef+
blocking-b2g: tef+ → ---
blocking-b2g: --- → tef+
(Reporter)

Comment 2

4 years ago
Created attachment 724004 [details] [diff] [review]
v1

After spending lot of time trying to figure out why the behavior between m-c and m-b2g18 is totally different on this issue I've ended up making this patch. I've been trying to find out the patch (or patches) that has been landed on m-b2g18 that makes this issue happens and the patch (or patches) that has been landed on m-c and we must uplift for achieving the same behavior. I had no luck and it seems so difficult to know where the differences between repos are. I request feedback here to you guys to know what to do about this issue.

This patch could be landed on m-c as well as doesn't brake anything and that will make having the same code in the two different repos. This patch depends on both 822522 and 837755 so before landing it on m-b2g18 we must uplift those bugs to m-b2g18 release repo.
Attachment #724004 - Flags: feedback?(vyang)
Attachment #724004 - Flags: feedback?(kyle)
(Reporter)

Updated

4 years ago
Depends on: 822522, 837755

Updated

4 years ago
Assignee: anshulj → mschwart
Comment on attachment 724004 [details] [diff] [review]
v1

Review of attachment 724004 [details] [diff] [review]:
-----------------------------------------------------------------

Seems ok to me, but I'm going to defer to vicamo's feedback/review since I'm not up on current RIL dev.
Attachment #724004 - Flags: feedback?(kyle) → feedback+
Comment on attachment 724004 [details] [diff] [review]
v1

Yoshi, could you help this?
Attachment #724004 - Flags: feedback?(vyang) → feedback?(allstars.chh)
Antonio, you should already fixed this in Bug 837755, right?
But the patch in Bug 837755 cannot be applied into b2g18.

I manually fix the conflict, and It works now,
no PIN dialog pop up.
Comment on attachment 724004 [details] [diff] [review]
v1

Review of attachment 724004 [details] [diff] [review]:
-----------------------------------------------------------------

I think this bug is already fixed by jaoo himself in Bug 837755.
Attachment #724004 - Flags: feedback?(allstars.chh) → feedback-
Oh, seems patch from Bug 837755 doesn't fix this.
Attachment #724004 - Flags: feedback-
(Reporter)

Comment 8

4 years ago
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #7)
> Oh, seems patch from Bug 837755 doesn't fix this.

Yes, that's true. This is because there is an additional patch in this bug. What we should do IMHO is to uplift bugs both 822522 and 837755 and them apply this bug to mozilla-b2g18 release repo. On the other hand this patch could be landed on m-c as well as doesn't brake anything and that will make having the same code in the two different repos.
Hi, Antonio
I wonder why m-c won't have this problem? 
Do you know why?
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #9)
> Hi, Antonio
> I wonder why m-c won't have this problem? 
> Do you know why?

I've spent lot of time trying to figure out why the behavior between m-c and m-b2g18 is totally different on this issue. I've been trying to find out the patch (or patches) that has been landed on m-b2g18 that makes this issue happens and the patch (or patches) that has been landed on m-c and we must uplift for achieving the same behavior. I had no luck and it seems so difficult to know where the differences between repos are.
I'll also check this and discuss this with Vicamo.
(Assignee)

Comment 12

4 years ago
Simply overriding the cardstate at the lower levels will indicate to the UI that the card in not ready when it is.  That will cut the user's ability to interact with card services such as STK, contacts on the card, changing PIN enabled, etc. when it should be OK.  I don't think that would be GCF compliant.

The UI should determine when showing PIN-entry is appropriate and do it then.
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #11)
> I'll also check this and discuss this with Vicamo.

Yoshi, any update here? Thanks.
Sorry for my late reply,
I also spent a lot of time to figure this out.

I think this is related to some init timing issue.

In b2g18 branch, when ril_worker connects to rild, modem isn't completely initialized yet, so the first 4~5 requests all fail with error 1, RADIO_NOT_AVAILABLE.

Whereas in m-c, there are some changes about SystemWorkerManager and RadioInterfaceLayer, and I compare the timing of socket is connected between m-c and b2g18, m-c is some milliseconds later then b2g18. So on m-c, those first requests returned successfully.

And one of the requests, is to turn off RADIO_POWER.
So RADIO_POWER off request is successed in m-c, but failed in b2g18.

I also check the radio log, on b2g18, ril_worker sends the 1st request to rild immediately.


E/RILC    (  112): isMultiSimEnabled: prop_val = 0 enabled = 0
D/RILC    (  112): RID 0 currentState() -> Radio Unavailable(1)
D/RILC    (  112): RID 0 currentState() -> Radio Unavailable(1)
D/RILC    (  112): getVersion() -> Qualcomm RIL 1.0
D/RILC    (  112): UI --- RIL_REQUEST_RADIO_POWER (23) ---> RIL [RID 0, token id 1, data len 4]
D/RILC    (  112): Ignore RIL_REQUEST_RADIO_POWER, unsupported state.
D/RILC    (  112): UI <--- RIL_REQUEST_RADIO_POWER (23) Complete --- RIL [RID 0, Token 1, Radio Not Available, Len 0 ]


(Actually from your log, the first request is SET_PREFERRED_NETWORK_TYPE, but I disable it for debug purpose, in your log, RADIO_POWER is from token 3).
Created attachment 725300 [details] [diff] [review]
Patch

I think the problem is on b2g18, REQUEST_RADIO_OFF is always failed due to RADIO_UNAVAIABLE, so I move the flag _isInitialRadioState to handler, and only turn it to false only when the request succeed.
Attachment #725300 - Flags: review?(vyang)
Comment on attachment 725300 [details] [diff] [review]
Patch

Review of attachment 725300 [details] [diff] [review]:
-----------------------------------------------------------------

After discuss with Yoshi in private, his patch matches Android's behaviour and solves the original problem, too. There is a timing difference between b2g18 and m-c. Althought the latter is immune to this issue right now, I'm not 100% sure it won't happen in the future. I think Yoshi's patch solves the root cause. Thank you both :).
Attachment #725300 - Flags: review?(vyang) → review+
Comment on attachment 724004 [details] [diff] [review]
v1

This patch is obsoleted by attachment 725300 [details] [diff] [review]. Thank you for the great job done here :)
Attachment #724004 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/f445e0cbba3b
Have you tested if everything works when landing 822522 and 837755 and this bugs in mozilla-b2g18? Great job here btw. Thanks.
Backout
https://hg.mozilla.org/integration/mozilla-inbound/rev/da8c5998b7e4

There's some mistake in my patch.
I should haven't returned too early. :(
Created attachment 725318 [details] [diff] [review]
Patch v2.
Attachment #725300 - Attachment is obsolete: true
Attachment #725318 - Flags: review?(vyang)
Attachment #725318 - Flags: review?(vyang) → review+
From jaoo's log,

in line 693, radio has reported RADIO_STATE_OFF,
however in line 721, ril_worker has called REQUEST_GET_SIM_STATUS (token 5), in line 743 the response of GET_SIM_STATUS is reporting RIL_CARDSTATE_PRESENT.

That seems a bug from modem side.

Hi, Michael Schwarts, is this a known problem to you ?
https://hg.mozilla.org/integration/mozilla-inbound/rev/04544e876ce3
Created attachment 725343 [details] [diff] [review]
[b2g18] Patch

The same patch for b2g18 branch
The b2g18 patch should depend on patches from Bug 822522, and Bug 837755.
ni(m4) for comment 22.
Flags: needinfo?(mschwart)
(Assignee)

Comment 27

4 years ago
The card is present and it's reporting it as present - I don't see the bug.

jaoo, getting back to the main issue reported, yes Gaia is being notified of the card state transitions.  Gaia needs to be a little smarter and conditionally show PIN entry as needed.

PIN entry shouldn't be required for basic smartphone use - it only locks the subscription information and related files.  Whenever the user tries to access PIN locked functionality, we check the state and conditionally show PIN entry.  During airplane mode, the user doesn't care so we shouldn't show it.
Flags: needinfo?(mschwart)
(In reply to Michael Schwartz [:m4] from comment #27)
> The card is present and it's reporting it as present - I don't see the bug.
> 

So when we turn off the radio, will card also report 'PRESENT' ?
https://hg.mozilla.org/mozilla-central/rev/04544e876ce3
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Does this needs to be uplifted? If so we need to set-up the tracking-flags accordingly (I assume so as it is a tef+ bug).
This bug is already tef+.
And my patch is for Mozilla RIL, not for partner's RIL.
I am not quite sure about the status of them.

Also see my comment 25, there are some other patches should be land first.
Clearing tef+ as we don't need this patch to commercialize v1.0.1
blocking-b2g: tef+ → -
Hi Michael,
Can you see Comment 1 from jaoo as this also happened on partner's RIL?
m4, do we need any gecko/gaia changes to resolve this issue with the latest commercial RIL?
Flags: needinfo?(mschwart)
This issue was detected while trying to uplift bugs 822522 and 837755 to mozilla-b2g18 release. Bug 837755 is marked as `approval-mozilla-b2g18+` so this work -and bug 822522- (its dependencies) has to be uplifted as well. This is also happening even with shipping RIL.
Target Milestone: mozilla22 → ---
I'd suggest move dicussion of partner's RIL to other bug.
Move Taget Milestone back to Mozilla 22, as I found jaoo seemed to remov it by accendent.
Target Milestone: --- → mozilla22
approval-mozilla-b2g18+ is for v1.1 right?
(In reply to Michael Vines [:m1] [:evilmachines] from comment #38)
> approval-mozilla-b2g18+ is for v1.1 right?

Yes
(In reply to Michael Vines [:m1] [:evilmachines] from comment #38)
> approval-mozilla-b2g18+ is for v1.1 right?

That's correct for now until we branch v1.1 to its own repo, approval-mozilla-b2g18+ will always mean approval for whatever version is the v1-train.
Comment on attachment 725343 [details] [diff] [review]
[b2g18] Patch

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 #): None.
User impact if declined: The phone prompts the user to enter the PIN code even if the phone boots/reboots while airplane mode is on.
Testing completed: Yes.
Risk to taking this patch (and alternatives if risky): Low.
String or UUID changes made by this patch: None.
Attachment #725343 - Flags: approval-mozilla-b2g18?
status-b2g18: --- → affected
tracking-b2g18: --- → +
Comment on attachment 725343 [details] [diff] [review]
[b2g18] Patch

Approving for v1.1 uplift
Attachment #725343 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
https://hg.mozilla.org/releases/mozilla-b2g18/rev/e5ea983c38a0
status-b2g18: affected → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-firefox20: --- → wontfix
status-firefox21: --- → wontfix
status-firefox22: --- → fixed
(Assignee)

Comment 44

4 years ago
(In reply to Michael Vines [:m1] [:evilmachines] from comment #34)
> m4, do we need any gecko/gaia changes to resolve this issue with the latest
> commercial RIL?

No, a local hack like the one here will work.
Flags: needinfo?(mschwart)
We need to uplift this bug due the landing of bug 837755 in mozilla-b2g18_v1_0_1 release repo. Requesting tef+ flag here.

Anshul, could you review if this uplifting could bring any problem on shipping RIL?
blocking-b2g: - → tef?
Flags: needinfo?(dcoloma)
Flags: needinfo?(anshulj)
Whiteboard: NO_UPLIFT
Created attachment 741714 [details] [diff] [review]
[b2g18_v1_0_1] Patch

With Yoshi's permission I'm attaching the patch I've rebased for testing on mozillab2g-18_v1_0_1 release repo. Yoshi, a double check from your side would be great. I've rebase the patch and attached here. Feel free to mark as obsolete if you want to attach your. Thanks Yoshi!
Attachment #741714 - Flags: feedback?(allstars.chh)
Attachment #741714 - Flags: feedback?(allstars.chh) → feedback+

Updated

4 years ago
blocking-b2g: tef? → tef+
Ah crap, I confused this with bug 849757 and pushed it to b2g18_v1_0_1.
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/e7671882de42

Let me know if I need to backout.
status-b2g18-v1.0.1: wontfix → fixed
Doesn't really matter as this patch is unused in commercial devices for both v1.0.1 or v1.1, so it's a NOP.
Whiteboard: NO_UPLIFT

Comment 49

4 years ago
Also, we already have support for this in commercial RIL on v1.
Flags: needinfo?(anshulj)

Updated

4 years ago
Flags: needinfo?(dcoloma)
You need to log in before you can comment on or make changes to this bug.