1083650 - [B2G][Emulator] Support facility lock

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Emulator, defect)

Description

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

Enhance emulator to support setting/querying facility lock.

Comment 1

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

See
- RIL_REQUEST_QUERY_FACILITY_LOCK [1]
- RIL_REQUEST_SET_FACILITY_LOCK [2]
- AT+CLCK [3]

[1] https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/telephony/ril.h#L1881-L1908
[2] https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/telephony/ril.h#L1910-L1937
[3] TS 127.007, section 7.4 Facility lock +CLCK

Comment 2

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

working branch:
https://github.com/EdgarChen/platform_hardware_ril/commits/bug_1083650/facility_lock
https://github.com/EdgarChen/platform_external_qemu/commits/bug_1083650/facility_lock

Comment 3

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

Summary what I would like to do in this bug:
1. Support getting/setting Facility lock, but only `PIN` for now. Then we can write a marionette test for querying/enabling/disabling pin lock.
2. Support reporting card state as "PIN LOCKED" if pin lock is enabled. And change state to "PUK LOCKED" if passing wrong pin to unlock over 3 times. Then we can write a marionette tests for unlocking pin/puk lock. Then test follow will be something like:
- enable pin lock
- switch radio off --> this will reset the card state
- switch radio on 
- card state is `PIN LOCKED`
- unlock with correct pin
- card state changes to 'READY'
- disable pin lock

Comment 4

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

Attachment: [platform_hardware_ril] PR #40

Comment 5

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

Attachment: [platform_external_qemu] PR #117

Comment 6

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

I triggered some try runs to make sure everything is good before requesting review.

Try run with qemu + ril changes: https://tbpl.mozilla.org/?tree=Try&rev=554612fec35c
Try run with qemu + ril changes and bug 1084233: https://tbpl.mozilla.org/?tree=Try&rev=b3b3f5aa6ad7

Comment 7

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

(In reply to Edgar Chen [:edgar][:echen] from comment #6)
> I triggered some try runs to make sure everything is good before requesting
> review.
> 
> Try run with qemu + ril changes:
> https://tbpl.mozilla.org/?tree=Try&rev=554612fec35c
> Try run with qemu + ril changes and bug 1084233:
> https://tbpl.mozilla.org/?tree=Try&rev=b3b3f5aa6ad7

Ah, there is a typo in the patch of updating qemu+ril. Will fix it and provide try link again later.

Comment 8

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

(In reply to Edgar Chen [:edgar][:echen] from comment #7)
> Ah, there is a typo in the patch of updating qemu+ril. Will fix it and
> provide try link again later.

Try run with qemu + ril changes: https://tbpl.mozilla.org/?tree=Try&rev=6a8a6e652e8f
Try run with qemu + ril changes and bug 1084233: https://tbpl.mozilla.org/?tree=Try&rev=6305e7992f44

Comment 9

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

Comment on attachment 8511910 [details] [review]
[platform_hardware_ril] PR #40

May I have your review, Hsinyi? Thank you.

Comment 10

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

Comment on attachment 8511912 [details] [review]
[platform_external_qemu] PR #117

May I have your review, Hsinyi? Thank you.

Comment 11

[Hsin-Yi Tsai (she/her) | away [:hsinyi]]

(In reply to Edgar Chen [:edgar][:echen] from comment #9)
> Comment on attachment 8511910 [details] [review]
> [platform_hardware_ril] PR #40
> 
> May I have your review, Hsinyi? Thank you.

Please see my comment for part 1 in github.

Comment 12

[Hsin-Yi Tsai (she/her) | away [:hsinyi]]

(In reply to Edgar Chen [:edgar][:echen] from comment #10)
> Comment on attachment 8511912 [details] [review]
> [platform_external_qemu] PR #117
> 
> May I have your review, Hsinyi? Thank you.

Please find my comments for part 1 and part 2 in github, thank you. Happy weekend :P

Comment 13

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

Thank you, Hsinyi. :)

Comment 14

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

Try result: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=a7b993cc3667

Comment 15

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/platform_hardware_ril/commit/d5d3f93914558b6f168447b805cd799c8233e300
https://github.com/mozilla-b2g/platform_external_qemu/commit/a47dd04f8f66e42fd331711140f2c3e2fed0767d

Comment 16

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

https://hg.mozilla.org/integration/b2g-inbound/rev/adff0e83b260

Comment 17

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

https://hg.mozilla.org/mozilla-central/rev/adff0e83b260

Comment 18

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

Attachment: (Kitkat) [platform_hardware_ril] PR #41

Comment 19

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

Attachment: (Kitkat) [platform_external_qemu] PR #126

Comment 20

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

Checkin-needed for Kitkat branch.

Comment 21

[Vicamo Yang [:vicamo][:vyang]]

Hi, in the two repositories, hardware/ril and external/qemu, their b2g-kitkat branch merges b2g-jellybean, which merges master again. You simply should not cherry-pick commits from master branch, just merge them and that keeps you from memorizing which commits are already included or not. Besides, you have only cherry-picks for kitkat branch. What will you going to do with b2g-jellybean branch? This move is going to bring in *a lot* effort for further development. Please just don't do that!

I had merged branches in my repository already. See
https://github.com/vicamo/b2g_platform_external_qemu/tree/next/b2g/b2g-jellybean
https://github.com/vicamo/b2g_platform_external_qemu/tree/next/b2g/b2g-kitkat
https://github.com/vicamo/b2g_platform_hardware_ril/tree/next/b2g/b2g-jellybean
https://github.com/vicamo/b2g_platform_hardware_ril/tree/next/b2g/b2g-kitkat

Comment 22

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #21)
> Hi, in the two repositories, hardware/ril and external/qemu, their
> b2g-kitkat branch merges b2g-jellybean, which merges master again. You
> simply should not cherry-pick commits from master branch, just merge them
> and that keeps you from memorizing which commits are already included or
> not. Besides, you have only cherry-picks for kitkat branch. What will you
> going to do with b2g-jellybean branch? This move is going to bring in *a
> lot* effort for further development. Please just don't do that!

AFAIK, we are going to move all the emulator things to b2g-kitkat and JB will not be maintained anymore. So after discussing, we just cherry-pick them into kitkat branch.

> 
> I had merged branches in my repository already. See
> https://github.com/vicamo/b2g_platform_external_qemu/tree/next/b2g/b2g-
> jellybean
> https://github.com/vicamo/b2g_platform_external_qemu/tree/next/b2g/b2g-kitkat
> https://github.com/vicamo/b2g_platform_hardware_ril/tree/next/b2g/b2g-
> jellybean
> https://github.com/vicamo/b2g_platform_hardware_ril/tree/next/b2g/b2g-kitkat

Since you already have merged branch, I will let you handle this. Thank you. :)

BTW, do you know how to acquire emulator merge privilege? I can help to maintain these repositories, too.

Comment 23

[Hsin-Yi Tsai (she/her) | away [:hsinyi]]

(In reply to Edgar Chen [:edgar][:echen] from comment #22)
> (In reply to Vicamo Yang [:vicamo][:vyang] from comment #21)
> > Hi, in the two repositories, hardware/ril and external/qemu, their
> > b2g-kitkat branch merges b2g-jellybean, which merges master again. You
> > simply should not cherry-pick commits from master branch, just merge them
> > and that keeps you from memorizing which commits are already included or
> > not. Besides, you have only cherry-picks for kitkat branch. What will you
> > going to do with b2g-jellybean branch? This move is going to bring in *a
> > lot* effort for further development. Please just don't do that!
> 
> AFAIK, we are going to move all the emulator things to b2g-kitkat and JB
> will not be maintained anymore. So after discussing, we just cherry-pick
> them into kitkat branch.
> 

Edgar is correct. Per the discussion with Thomas, we don't need to care about support for JB, besides building for it. 

> > 
> > I had merged branches in my repository already. See
> > https://github.com/vicamo/b2g_platform_external_qemu/tree/next/b2g/b2g-
> > jellybean
> > https://github.com/vicamo/b2g_platform_external_qemu/tree/next/b2g/b2g-kitkat
> > https://github.com/vicamo/b2g_platform_hardware_ril/tree/next/b2g/b2g-
> > jellybean
> > https://github.com/vicamo/b2g_platform_hardware_ril/tree/next/b2g/b2g-kitkat
> 
> Since you already have merged branch, I will let you handle this. Thank you.
> :)
> 
> BTW, do you know how to acquire emulator merge privilege? I can help to
> maintain these repositories, too.

Comment 24

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #23)
> (In reply to Edgar Chen [:edgar][:echen] from comment #22)
> > (In reply to Vicamo Yang [:vicamo][:vyang] from comment #21)
> > > Hi, in the two repositories, hardware/ril and external/qemu, their
> > > b2g-kitkat branch merges b2g-jellybean, which merges master again. You
> > > simply should not cherry-pick commits from master branch, just merge them
> > > and that keeps you from memorizing which commits are already included or
> > > not. Besides, you have only cherry-picks for kitkat branch. What will you
> > > going to do with b2g-jellybean branch? This move is going to bring in *a
> > > lot* effort for further development. Please just don't do that!
> > 
> > AFAIK, we are going to move all the emulator things to b2g-kitkat and JB
> > will not be maintained anymore. So after discussing, we just cherry-pick
> > them into kitkat branch.
> > 
> 
> Edgar is correct. Per the discussion with Thomas, we don't need to care
> about support for JB, besides building for it. 

Deprecating JB is just not a good reason to switch merge policy to cherry-picks. You can merge b2g/master directly into b2g/b2g-kitkat if you want. The main problem lies in the cherry-picks itself. Today we may cherry-pick this bug 1083650 to b2g-kitkat, but guess what, we have to do this because cherry-picking bug 1098193, bug 1100211, bug 1093506 and bug 1105988 was chosen, and this bug was __MISSED__. Please remember we're all humans, not saints, and humans are meant to forget. So do yourself a favor not to keep all these minor issues in you mind. Let mechanism solve your problems automatically whenever possible.

Comment 25

[Vicamo Yang [:vicamo][:vyang]]

NI :mwu per comment 22. If possible, need write permission for:

https://github.com/mozilla-b2g/platform_external_qemu
https://github.com/mozilla-b2g/platform_hardware_ril
https://github.com/mozilla-b2g/device_generic_goldfish

Comment 26

[Hsin-Yi Tsai (she/her) | away [:hsinyi]]

Your concern makes sense. Thanks, Vicamo. I had written an email to Thomas (emulator owner) and Michael to discuss the improvement and to see how to acquire merge privilege.

Comment 27

[Vicamo Yang [:vicamo][:vyang]]

Ping?

Comment 28

[Michael Wu [:mwu]]

Hsin-Yi Tsai and/or Edgar, you will need level 3 commit access. If you have access, please link the bug where it was granted. Thanks!

Comment 29

[Edgar Chen [:edgar] (PTO 2/2 ~ 2/6)]

(In reply to Michael Wu [:mwu] from comment #28)
> Hsin-Yi Tsai and/or Edgar, you will need level 3 commit access. If you have
> access, please link the bug where it was granted. Thanks!

Hi Michael, here is the bug for my level 3 commit access, https://bugzilla.mozilla.org/show_bug.cgi?id=896402. Thank you.

Comment 30

[Hsin-Yi Tsai (she/her) | away [:hsinyi]]

(In reply to Michael Wu [:mwu] from comment #28)
> Hsin-Yi Tsai and/or Edgar, you will need level 3 commit access. If you have
> access, please link the bug where it was granted. Thanks!

My level 3 commit access was granted in Bug 763811. Thank you for the help, Michael.