1126677 - [FFOS2.0][Woodduck]No IMSI detach prodecure when power off

Status: RESOLVED FIXED

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

Description

[sync-1]

DEFECT DESCRIPTION:
 No IMSI detach prodecure when power off 
 
  REPRODUCING PROCEDURES:
 Power on the device,after device register successfully, power off the device.
 From the log, we can see no IMSI detach signalling.
 
 Note: When enable flight mode,we can see IMSI detach procedure, but if we power off, no IMSI detach procedure.
   
  EXPECTED BEHAVIOUR:
 When power off, devide should perform detach procedure normally.
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 
  For FT PR, Please list reference mobile's behavior:
 
  Reporter phone number:0752-2639862(61862)

Comment 1

[sync-1]

Attachment: mtklog

Comment 2

[sync-1]

Attachment: airplane on and off mtklog

Comment 3

[sync-1]

Attachment: normal detach procedure

Comment 4

[sync-1]

Attachment: mtklog

Comment 5

[Josh Cheng [:josh]]

Hi Shawn,
Could you please help to check the problem? Thanks!

Comment 6

[shawn ku [:sku]]

It looks like Gaia only request shutdown/reboot command when user request power off/shutdown (see [1]). 
IMO, part in [2] needs to be included for shutdown/reboot procedure to make IMSI detach work properly.


[1] - https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/sleep_menu.js#L445
[2] - https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/radio.js#L149

Comment 7

[Josh Cheng [:josh]]

Hi Reporter,

Is this a critical test case blocks your certification?
This is by our design now and I suggest we fix this in future version than 2.0M. Thanks!

Comment 8

[sync-1]

(In reply to comment #3)
 > Comment from Mozilla:Hi Reporter,
 > 
 > Is this a critical test case blocks your certification?
 > This is by our design now and I suggest we fix this in future version than
 > 2.0M. Thanks!
 > 
 Dear mozilla,
   这个不是certification，但是这个功能是客户需求功能，运营商要求实现这个功能，请知悉，谢谢。

Comment 9

[sync-1]

Dear mozilla,
    这个PR是PTCRB认证，需要在今明两天完成，tcl周一要给到TMC进行复测，能尽快解决吗？谢谢

Comment 10

[Josh Cheng [:josh]]

Hi Gary,
Could you please help to check the problem? Thanks!

Comment 11

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/27808

Hi all,
   Here is my WIP, could you help to apply this patch on partner's branch for testing?
   Thanks.

Comment 12

[kai.du]

I will test it.

Comment 13

[kai.du]

test ok,thank you for your help.

Comment 14

[sync-1]

Dear dengwei,
    this pr have fixed,please close it,tcl pr:912720

Comment 15

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Comment on attachment 8556991 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/27808

Hi Tim,
   According to partner's request, do you think we need to land this patch in maser or just land in v2.0m only?
   Seems we don't handle this case and I have no idea who can be consultant about this issue. Could you help to take a look?
  Thanks.

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8556991 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/27808

Glad you asked. This is a workaround for 2.0m only, unless Hsinyi says it's not :)

I would imagine the RIL should handle device reboot/power off on it's own without System app -- it's entirely possible in the future the app with the permission to poweroff() does not have the permission to turn off the mobile radio.

FYR, |Radio.enabled = false| points to this setter:

https://github.com/mozilla-b2g/gaia/blob/1217bf61e26c/apps/system/js/radio.js#L211-L221

Comment 17

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #16)
> Glad you asked. This is a workaround for 2.0m only, unless Hsinyi says it's
> not :)

That said, if we don't have a timely solution for v3.0 we could land this .... with XXX and bug #.

Comment 18

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

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #16)
> Comment on attachment 8556991 [details] [review]
> https://github.com/mozilla-b2g/gaia/pull/27808
> 
> Glad you asked. This is a workaround for 2.0m only, unless Hsinyi says it's
> not :)
> 
> I would imagine the RIL should handle device reboot/power off on it's own
> without System app -- it's entirely possible in the future the app with the
> permission to poweroff() does not have the permission to turn off the mobile
> radio.
> 

It would be great that this is handled by gecko, but in the power module not exactly in RIL, which however has no idea if user is going to poweroff or not. So, the flow I imagine is that when the power module receives poweroff request, the power module asks RIL module to send out additional radio command.


> FYR, |Radio.enabled = false| points to this setter:
> 
> https://github.com/mozilla-b2g/gaia/blob/1217bf61e26c/apps/system/js/radio.
> js#L211-L221

Comment 19

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

Comment on attachment 8556991 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/27808

Please see comment 18.In the long-term, it makes sense to me to do this in gecko.

Comment 20

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi all,
   Per comment 18, I would like to nominate "v2.0m?" first for resolving issue in v2.0m branch.

Hi Josh and Tim,
   Do you think we need this patch on other branches? eg, 2.1 or higher.
   Or file another bug?

Thanks.

Comment 21

[Josh Cheng [:josh]]

Hi Gary, Hi Tim, Hi Hsin-Yi,
I think we can land this for 2.0M for now and maybe you will open a bug for long term solution later?
Thanks!

Comment 22

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8556991 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/27808

As I stated before, I have no problem landing this on v2.0m or in master.

But I ask the platform bug to be filed and a XXX comment left in Gaia code should it lands on master. Thanks!

Comment 23

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

gecko bug 1129272 filed.

Comment 24

[Josh Cheng [:josh]]

Hi Kai-Zhen,
Could you help to land this on 2.0M? Thanks!

Comment 25

[Kai-Zhen Li [:seinlin][:kli]]

Gary, could you help to add comment in gaia code per comment 22? Once you finished, please ni me. Thanks!

Comment 26

[Josh Cheng [:josh]]

Hi Kai-Zhen,
This will only land on 2.0M as workaround. HsinYi will fix this with bug 1129272 on master. Thanks!

Comment 27

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

(In reply to Josh Cheng [:josh] from comment #26)
> Hi Kai-Zhen,
> This will only land on 2.0M as workaround.
> HsinYi will fix this with bug 1129272 on master. Thanks!
Ha, there seems miscommunication here. I said the formal fix should be on bug 1129272, however not sure where the statement (I will fix this) came from. ;) If workload permits, I'll like to help of course.

Comment 28

[Kai-Zhen Li [:seinlin][:kli]]

v2.0m: https://github.com/mozilla-b2g/gaia/commit/68bc0bf86e9addf9f4291737d858c491258a74f8