882095 - [MMS] Internal PDP is deactivation after changing MMS settings

Status: RESOLVED FIXED

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

Description

[Leo]

1. Title: Internal PDP deactivation after changing MMS settings 
2. Precondition: SMS app should be working
3. Tester's Action:  1)go to settings-> cellular&data -> message settings
     2) change the status of the option auto retrieve and press ok
     3)press the ok button near the name of menu (message settings)
4. Detailed Symptom (ENG.) : When the ok button of step 3 the DUT deactivate and reactivate the internal PDP without any reason
5. Expected: The internal PDP shouldn't be deactivated 
6. Reproducibility: Y
1) Frequency Rate : 100%
7. Gaia Master/v1-train: Reproduced on v1-train and master
8. Gaia Revision:  bd07ce233160a0e6325bf736769772a9d2273458
9. Personal email id: [e-mail]

Comment 1

[Leo]

Hi,
This is happening even when we are trying to change the SMS settings (for e.g.,Delivery Reports)
Can you please let me know whether this is the expected behaviour?
Thanks,

Comment 2

[Leo]

Hi Borja,

Please check comment #1 and let us know your comments.

Thanks

Comment 3

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Leo from comment #0)
> 1. Title: Internal PDP deactivation after changing MMS settings 
> 2. Precondition: SMS app should be working
> 3. Tester's Action:  1)go to settings-> cellular&data -> message settings
>      2) change the status of the option auto retrieve and press ok
>      3)press the ok button near the name of menu (message settings)
> 4. Detailed Symptom (ENG.) : When the ok button of step 3 the DUT deactivate
> and reactivate the internal PDP without any reason
> 5. Expected: The internal PDP shouldn't be deactivated 
> 6. Reproducibility: Y
> 1) Frequency Rate : 100%
> 7. Gaia Master/v1-train: Reproduced on v1-train and master
> 8. Gaia Revision:  bd07ce233160a0e6325bf736769772a9d2273458
> 9. Personal email id: [e-mail]

I'll comment here with Borja's permission. That's the behavior implemented and expected. We force data connection to restart when changes in the panel are validated. See code at [1] please. I guess that makes sense only when updating the APN settings for MMS and we should avoid it in case of modifying delivery report settings or auto retrieve settings. Requesting ni? at :kaze and Steve Chung.

Kaze, Steve, your feedback here is more than welcome.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/carrier.js#L226-L229

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/carrier.js#L226-L229

Comment 4

[Steve Chung [:steveck]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #3)

> I'll comment here with Borja's permission. That's the behavior implemented
> and expected. We force data connection to restart when changes in the panel
> are validated. See code at [1] please. I guess that makes sense only when
> updating the APN settings for MMS and we should avoid it in case of
> modifying delivery report settings or auto retrieve settings. Requesting ni?
> at :kaze and Steve Chung.
> 
> Kaze, Steve, your feedback here is more than welcome.
> 
It sounds a reasonable behavior, but we don't have an efficient way to monitor if only the delivery report and auto retrieving option value changed (Maybe listening the apnSettings-list and apnSettings-advanced ul panel click event?). Hi Kaze, if you think this fixing is necessary and have a better solution for this scenario, you can assign it to me or any other proper developers, thanks.

Comment 5

[Arthur Chen [:arthurcc][inactive after 6/30]]

The logic specified by Jose has a flaw which is pressing OK without changing anything also triggers reconnection. I think we need to implement "submit" and "reset" functions in carrier.js so that we have a chance to check the value changes.

Comment 6

[Arthur Chen [:arthurcc][inactive after 6/30]]

Attachment: Link to https://github.com/mozilla-b2g/gaia/pull/10481

Kaze, the patch added some logic checking if APN related settings are changed. Although the solution is not 100% accurate but it saves time of querying, saving, and comparing settings. It needs to be refined after bug 842252 landed. After that we will have more accurate information when merging the APN settings. Please help review the patch, thanks!

Comment 7

[Fabien Cazenave [:kaze]]

Comment on attachment 764643 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/10481

r=me given the timeframe, but let’s make sure we get rid of this patch ASAP — that is, along with bug 842252.

Comment 8

[Arthur Chen [:arthurcc][inactive after 6/30]]

Kaze, thanks for reviewing!

master: https://github.com/mozilla-b2g/gaia/commit/2af068e72bcda7fa37b2bcd00de4d78f999370f9
v1-train: https://github.com/mozilla-b2g/gaia/commit/f13928e9f6fa44357ccd4e330348844004b36a45

Comment 9

[John Ford [:jhford] CET/CEST Berlin Time]

1.1hd: f13928e9f6fa44357ccd4e330348844004b36a45