If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

B2G RIL: send data registration request one by one

RESOLVED FIXED in Firefox 34, Firefox OS v2.0M

Status

Firefox OS
RIL
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jessica, Assigned: jessica)

Tracking

(Blocks: 1 bug)

unspecified
2.1 S2 (15aug)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

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

Details

Attachments

(8 attachments, 2 obsolete attachments)

(Assignee)

Description

3 years ago
In cases where we need to attach data registration on demand, e.g. on multi-sim devices, we should wait for the previous data registration request response to send the next data registration request.
(Assignee)

Comment 1

3 years ago
Created attachment 8468251 [details] [diff] [review]
patch, v1.

Hi Hsinyi, I have made setDataRegistration() return a promise and reorganized a little bit part of the code. May I have your review? Thanks.
Attachment #8468251 - Flags: review?(htsai)
Comment on attachment 8468251 [details] [diff] [review]
patch, v1.

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

Thank you Jessica! Please see my comments below.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +822,4 @@
>          if (RILQUIRKS_DATA_REGISTRATION_ON_DEMAND) {
>            let radioInterface = connHandler.radioInterface;
> +          radioInterface.setDataRegistration(false)
> +            .then(() => execPendingDataCallRequest());

Ideally, as the name "_pendingDataCallRequest" presents itself, it should include all the following operations, and we don't need an extra call to |radioInterface.setDataRegistration(false)|. Can we try to move |radioInterface.setDataRegistration(false)| into that pending request?

::: dom/system/gonk/ril_worker.js
@@ +6709,5 @@
> +  options.success = (options.rilRequestError === 0);
> +  if (!options.success) {
> +    options.errorMsg = RIL_ERROR_TO_GECKO_ERROR[options.rilRequestError];
> +  }
> +  this.sendChromeMessage(options);

RIL_Worker may internally trigger data attach, such as line 6688. In the case, we don't need to sendChromeMessage to the main thread. Please address that well, thanks.
Attachment #8468251 - Flags: review?(htsai)
(Assignee)

Comment 3

3 years ago
Created attachment 8468996 [details] [diff] [review]
patch, v2.

Hsinyi, I have made some modifications according to review comment 2, not sure if it's better this way, could you help review again? Thanks.
Attachment #8468251 - Attachment is obsolete: true
Attachment #8468996 - Flags: review?(htsai)
Comment on attachment 8468996 [details] [diff] [review]
patch, v2.

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

I like the structure! I think it's clearer :) Thank you thank you~

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +2814,5 @@
> +    this.workerMessenger.send("setDataRegistration",
> +                              {attach: attach},
> +                              (function(response) {
> +      // Always resolve to proceed with the following steps.
> +      deferred.resolve(response.errorMsg ? response.errorMsg : null);

Due to the existing structure design, we can't really do something for error handling, so this looks fine for now. :)

::: dom/system/gonk/ril_worker.js
@@ +6688,5 @@
>      this.setDataRegistration({attach: true});
>    }
>  };
> +RilObject.prototype[REQUEST_SET_DATA_SUBSCRIPTION] = function REQUEST_SET_DATA_SUBSCRIPTION(length, options) {
> +  if (options.rilMessageType == null) {

nit: if (!options.rilMessageType)

@@ +6709,5 @@
>    options.retryCount = length ? this.context.Buf.readInt32List()[0] : -1;
>    this.sendChromeMessage(options);
>  };
> +RilObject.prototype[RIL_REQUEST_GPRS_ATTACH] = function RIL_REQUEST_GPRS_ATTACH(length, options) {
> +  if (options.rilMessageType == null) {

ditto.
Attachment #8468996 - Flags: review?(htsai) → review+
(Assignee)

Comment 5

3 years ago
Created attachment 8469163 [details] [diff] [review]
patch (final).

Thank you, Hsinyi!

Address nits in review comment 4.
Attachment #8468996 - Attachment is obsolete: true
Attachment #8469163 - Flags: review+
(Assignee)

Comment 6

3 years ago
https://tbpl.mozilla.org/?tree=Try&rev=0f544f8f0b69

try green!
(Assignee)

Comment 7

3 years ago
https://hg.mozilla.org/integration/b2g-inbound/rev/0c0a98b643b5
Target Milestone: --- → 2.1 S2 (15aug)
https://hg.mozilla.org/mozilla-central/rev/0c0a98b643b5
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED

Comment 9

3 years ago
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.0?

Comment 10

3 years ago
(In reply to shawn ku [:sku] from comment #9)
> [Blocking Requested - why for this release]:

woodduck needs this solution to make data connection works on both SIM.
(Assignee)

Comment 11

3 years ago
(In reply to shawn ku [:sku] from comment #10)
> (In reply to shawn ku [:sku] from comment #9)
> > [Blocking Requested - why for this release]:
> 
> woodduck needs this solution to make data connection works on both SIM.

Hi Shawn, is it possible for them to try the patch first to see if it works? Thanks.
What is the user impact here?

What is the risk of not taking fix in 2.0?
needinfo on comment 10
Flags: needinfo?(sku)

Comment 14

3 years ago
(In reply to Jessica Jong (OOO Aug 19 - 31) [:jjong] [:jessica] from comment #11)
> (In reply to shawn ku [:sku] from comment #10)
> > (In reply to shawn ku [:sku] from comment #9)
> > > [Blocking Requested - why for this release]:
> > 
> > woodduck needs this solution to make data connection works on both SIM.
> 
> Hi Shawn, is it possible for them to try the patch first to see if it works?
> Thanks.

Hi Jessica:
 I will ask partner first, but I am afraid it might have some difficulty for partner to apply the patch since they use p4, not git.

Keep ni, before any response back from partner.

Comment 15

3 years ago
After apply the patch to 2.0, woodduck can get data service after change default data sim.
Still wait for partner to double confirm the result.
(In reply to shawn ku [:sku] from comment #15)
> After apply the patch to 2.0, woodduck can get data service after change
> default data sim.
> Still wait for partner to double confirm the result.

This cannot land on 2.0 as there is no impact to flame and the branch is strictly open for 2.0+. We are still discussing on how to proceed for patches needed for woodduck, so you should be hearing soon on how and where to land this. For now I'll keep the nom until we have the woodduck flags in place but please do not 2.0+ it.

Comment 17

3 years ago
(In reply to bhavana bajaj [:bajaj] from comment #16)
> (In reply to shawn ku [:sku] from comment #15)
> > After apply the patch to 2.0, woodduck can get data service after change
> > default data sim.
> > Still wait for partner to double confirm the result.
> 
> This cannot land on 2.0 as there is no impact to flame and the branch is
> strictly open for 2.0+. We are still discussing on how to proceed for
> patches needed for woodduck, so you should be hearing soon on how and where
> to land this. For now I'll keep the nom until we have the woodduck flags in
> place but please do not 2.0+ it.

understood, and thanks!!
Flags: needinfo?(sku)
Triage discussed and concluded we should not risk 2.0 branch stability for woodduck. Or at least not until other 2.0 branch consumers have cut their release. For now this should go where woodduck-only fixes go, once that's decided.
blocking-b2g: 2.0? → -

Comment 19

3 years ago
nominate 2.0M? per comment 10/15.
blocking-b2g: - → 2.0M?
Blocks: 1058585
2.0M+ as Woodduck needs this solution.
blocking-b2g: 2.0M? → 2.0M+

Updated

3 years ago
Blocks: 1054172
Jessica, could you make a patch for v2.0m branch? Thanks!
Flags: needinfo?(jjong)
(In reply to Kai-Zhen Li from comment #21)
> Jessica, could you make a patch for v2.0m branch? Thanks!

I will help on this. ni? me for tracking.
Flags: needinfo?(jjong)
Flags: needinfo?(echen)

Updated

3 years ago
Blocks: 1060923

Updated

3 years ago
Blocks: 1060921
Try result for 2.0m: https://tbpl.mozilla.org/?tree=Try&rev=6ff3c7493cc1
Created attachment 8482032 [details] [diff] [review]
[b2g_32_2_0m] patch, r=hsinyi

Patch for b2g32_2_0m
Flags: needinfo?(echen)
v2.0m: http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/b760e37130cd
status-b2g-v2.0M: --- → fixed
(Assignee)

Comment 26

3 years ago
(In reply to Edgar Chen [:edgar][:echen] from comment #22)
> (In reply to Kai-Zhen Li from comment #21)
> > Jessica, could you make a patch for v2.0m branch? Thanks!
> 
> I will help on this. ni? me for tracking.

Thanks Edgar for the help! :)
Depends on: 1060217

Updated

3 years ago
Blocks: 1069800

Updated

3 years ago
Blocks: 1076703
status-b2g-v2.0: --- → wontfix
status-b2g-v2.1: --- → fixed
status-firefox32: --- → wontfix
status-firefox33: --- → wontfix
status-firefox34: --- → fixed

Updated

3 years ago
Blocks: 1080481

Updated

3 years ago
status-b2g-v2.2: --- → fixed

Comment 27

3 years ago
Hi Norry,
Please verify on Woodduck 2.0M.
Flags: needinfo?(fan.luo)
Keywords: verifyme

Comment 28

3 years ago
Verification STR:

[Initial condition]
2 SIM cards, data connection is connected at SIM1.

[Step]
1 Switch data connection to SIM2
2 Browse some website with SIM2 connection
3 Switch connection back to SIM1
4 Browse some website with SIM1 connection

[Expected result]
Data connection should be switched smoothly without suffer from disconnection.

Comment 29

3 years ago
(In reply to Josh Cheng [:josh] from comment #28)
> Verification STR:
> 
> [Initial condition]
> 2 SIM cards, data connection is connected at SIM1.
> 
> [Step]
> 1 Switch data connection to SIM2
> 2 Browse some website with SIM2 connection
> 3 Switch connection back to SIM1
> 4 Browse some website with SIM1 connection
> 
> [Expected result]
> Data connection should be switched smoothly without suffer from
> disconnection.

Hi Josh
My test steps

1. 2 SIM cards, data connection is connected at SIM1
>You can browse the website successfully, SIM 1 data connection icon(PDH icon) is displayed,see figure:SIM1.png

2.Switch data connection to SIM2,Browse some website with SIM2 connection
>You can browse the website successfully,SIM 2 data connection icon(PDH icon) is displayed,see figure:SIM2.png

3.Switch data connection to SIM1 Again
>No Data connect icon(PDH icon) displayed and can not browse any website,see figure:No PDH icon.png

Test build information:

Gaia-Rev        fdb8236d7d99061ef6bedc021fd6b482e1af3f5a
Gecko-Rev       3d95c1683ef5eb017bd15bc38ba111ddb1ad792e
Build-ID        20141024050313
Version         32.0
Device-Name     soul35
FW-Release      4.4.2
FW-Incremental  1414098327
FW-Date         Fri Oct 24 05:05:49 CST 2014
Flags: needinfo?(fan.luo) → needinfo?(jocheng)

Comment 30

3 years ago
Created attachment 8510864 [details]
SIM1.png

Comment 31

3 years ago
Created attachment 8510865 [details]
SIM2.png

Comment 32

3 years ago
Created attachment 8510866 [details]
No PDH  icon.png

Comment 33

3 years ago
Created attachment 8510867 [details]
logcat
(Assignee)

Comment 34

3 years ago
(In reply to Eric from comment #33)
> Created attachment 8510867 [details]
> logcat

Eric, your log file does not match the screenshots, the log ends at 14:29, and I only see data connection switch to SIM 1 once:

> 10-24 05:33:36.534 I/Gecko   (  160): -*- DataConnectionManager: 'ril.data.defaultServiceId' is now 1

Could you help check if the attached log is correct? Thanks.
Flags: needinfo?(yongdong.xia)

Comment 35

3 years ago
(In reply to Jessica Jong [:jjong] [:jessica] from comment #34)
> (In reply to Eric from comment #33)
> > Created attachment 8510867 [details]
> > logcat
> 
> Eric, your log file does not match the screenshots, the log ends at 14:29,
> and I only see data connection switch to SIM 1 once:
> 
> > 10-24 05:33:36.534 I/Gecko   (  160): -*- DataConnectionManager: 'ril.data.defaultServiceId' is now 1
> 
> Could you help check if the attached log is correct? Thanks.

Default data connection is SIM1.

The attached log is recorded when we reproduce it for the first time, while the screenshot is captured when we reproduce for the second time. 

I reproduced this issue again  just now and attached the newest log for your reference.  Log time: 4:23PM-4:25pm.New log:New logcat.txt
Flags: needinfo?(yongdong.xia)

Comment 36

3 years ago
Created attachment 8510903 [details]
New logcat
(Assignee)

Comment 37

3 years ago
Thanks Eric for reproducing again. From the new logcat, data registration is set on SIM 0, but it did not get registered:

Set data registration to SIM 0:

> 10-24 16:24:44.077 I/Gecko   (  163): RIL Worker: [0] Received chrome message {"attach":true,"rilMessageClientId":0,"rilMessageToken":43,"rilMessageType":"setDataRegistration"}

Request success:

> 10-24 16:24:44.169 I/Gecko   (  163): -*- RadioInterface[0]: Received message from worker: {"attach":true,"rilMessageClientId":0,"rilMessageToken":43,"rilMessageType":"setDataRegistration","rilRequestType":5018,"rilRequestError":0,"success":true}

Data registration is still in UNKNOWN (4) state:

10-24 16:24:44.265 I/Gecko   (  163): -*- RadioInterface[1]: Received message from worker: {"rilMessageType":"networkinfochanged","rilMessageClientId":1,"dataRegistrationState":{"regState":4,"state":null,"connected":false,"roaming":false,"emergencyCallsOnly":true,"cell":{"gsmLocationAreaCode":-1,"gsmCellId":-1},"radioTech":0,"type":null,"rilMessageType":"dataregistrationstatechange"},"signal":{"voice":{"signalStrength":-75,"relSignalStrength":100},"data":{"signalStrength":-75,"relSignalStrength":100},"rilMessageType":"signalstrengthchange"}}

Can someone from modem check this as well? Thanks.

Comment 38

3 years ago
(In reply to Jessica Jong [:jjong] [:jessica] from comment #37)
> Thanks Eric for reproducing again. From the new logcat, data registration is
> set on SIM 0, but it did not get registered:
> 
> Set data registration to SIM 0:
> 
> > 10-24 16:24:44.077 I/Gecko   (  163): RIL Worker: [0] Received chrome message {"attach":true,"rilMessageClientId":0,"rilMessageToken":43,"rilMessageType":"setDataRegistration"}
> 
> Request success:
> 
> > 10-24 16:24:44.169 I/Gecko   (  163): -*- RadioInterface[0]: Received message from worker: {"attach":true,"rilMessageClientId":0,"rilMessageToken":43,"rilMessageType":"setDataRegistration","rilRequestType":5018,"rilRequestError":0,"success":true}
> 
> Data registration is still in UNKNOWN (4) state:
> 
> 10-24 16:24:44.265 I/Gecko   (  163): -*- RadioInterface[1]: Received
> message from worker:
> {"rilMessageType":"networkinfochanged","rilMessageClientId":1,
> "dataRegistrationState":{"regState":4,"state":null,"connected":false,
> "roaming":false,"emergencyCallsOnly":true,"cell":{"gsmLocationAreaCode":-1,
> "gsmCellId":-1},"radioTech":0,"type":null,"rilMessageType":
> "dataregistrationstatechange"},"signal":{"voice":{"signalStrength":-75,
> "relSignalStrength":100},"data":{"signalStrength":-75,"relSignalStrength":
> 100},"rilMessageType":"signalstrengthchange"}}
> 
> Can someone from modem check this as well? Thanks.

Hi Reporter,

Could you check whether the problem is fixed with your latest ROM? Thanks!
Flags: needinfo?(jocheng) → needinfo?(wudduc)

Updated

3 years ago
Keywords: verifyme
(Assignee)

Comment 39

3 years ago
BTW, I can switch to SIM 1 and back to SIM 0 with my local build, woodduck v2.0m:

[ro.build.fingerprint]: [alps/full_soul35/soul35:4.4.2/KOT49H/1412238667:eng/dev-keys]
[ro.mediatek.chip_ver]: [S01]
[ro.mediatek.gemini_support]: [true]
[ro.mediatek.platform]: [MT6572]
[ro.mediatek.version.branch]: [KK.AOSP]
[ro.mediatek.version.release]: [ALPS.W10.24.p0]
Created attachment 8511798 [details]
adb log radio.log

(In reply to Josh Cheng [:josh] from comment #28)
> Verification STR:
> 
> [Initial condition]
> 2 SIM cards, data connection is connected at SIM1.
> 
> [Step]
> 1 Switch data connection to SIM2
> 2 Browse some website with SIM2 connection
> 3 Switch connection back to SIM1
> 4 Browse some website with SIM1 connection
> 
> [Expected result]
> Data connection should be switched smoothly without suffer from
> disconnection.

I followed above steps, but I still can see the network disconnection problem on latest mozilla build.

Attached is the adb log. Test starts: 10-27 11:54:22.349
Steps:
0. Flash a new build, the default data SIM is SIM1
1. Browse a website, website load successfully
2. Change data SIM to SIM2
   --> It took about 20s for data connection icon appears on SIM2. If user load website during this time. They will see the network disconnection problem.
3. Change data SIM to SIM1
   --> It took over 10s for data connection icon appears on SIM1. If user load website during this time. They will see the network disconnection problem.
4. Change data SIM to SIM2
   --> Same as step 2. It took over 20s for data connection change.

Mozilla build
-------------------------------------------------------------------------
Gaia-Rev        fdb8236d7d99061ef6bedc021fd6b482e1af3f5a
Gecko-Rev       717b111a9f7102a3fe76cece8f21a5c794e3bddf
Build-ID        20141027050313
Version         32.0
Device-Name     soul35
FW-Release      4.4.2
FW-Incremental  1414357512
FW-Date         Mon Oct 27 05:05:35 CST 2014
(Assignee)

Comment 41

3 years ago
(In reply to Peipei Cheng from comment #40)
> Created attachment 8511798 [details]
> adb log radio.log
> 
> (In reply to Josh Cheng [:josh] from comment #28)
> > Verification STR:
> > 
> > [Initial condition]
> > 2 SIM cards, data connection is connected at SIM1.
> > 
> > [Step]
> > 1 Switch data connection to SIM2
> > 2 Browse some website with SIM2 connection
> > 3 Switch connection back to SIM1
> > 4 Browse some website with SIM1 connection
> > 
> > [Expected result]
> > Data connection should be switched smoothly without suffer from
> > disconnection.
> 
> I followed above steps, but I still can see the network disconnection
> problem on latest mozilla build.
> 
> Attached is the adb log. Test starts: 10-27 11:54:22.349
> Steps:
> 0. Flash a new build, the default data SIM is SIM1
> 1. Browse a website, website load successfully
> 2. Change data SIM to SIM2
>    --> It took about 20s for data connection icon appears on SIM2. If user
> load website during this time. They will see the network disconnection
> problem.
> 3. Change data SIM to SIM1
>    --> It took over 10s for data connection icon appears on SIM1. If user
> load website during this time. They will see the network disconnection
> problem.
> 4. Change data SIM to SIM2
>    --> Same as step 2. It took over 20s for data connection change.
> 
> Mozilla build
> -------------------------------------------------------------------------
> Gaia-Rev        fdb8236d7d99061ef6bedc021fd6b482e1af3f5a
> Gecko-Rev       717b111a9f7102a3fe76cece8f21a5c794e3bddf
> Build-ID        20141027050313
> Version         32.0
> Device-Name     soul35
> FW-Release      4.4.2
> FW-Incremental  1414357512
> FW-Date         Mon Oct 27 05:05:35 CST 2014

It is normal that you experience a short disconnection while you switch default SIM for data call, this is because data registration needs to get unregistered and registered and it takes time. The latency of this disconnection depends on the modem/network.

Comment 42

3 years ago
Hi Jessica,
If the test result per comment 40 is expected. I think it's resolved until partner report same problem with longer disconnect time.
Thanks!

Comment 43

3 years ago
Hi Jessica,
Right after I saved my last comment. I see ODM partner submitted exact same issue as bug 1090133. I am requesting the reporter for snapshot and console log. Do you still need anything else? 
Thanks!
Flags: needinfo?(jjong)
See Also: → bug 1090133
(Assignee)

Comment 44

3 years ago
(In reply to Josh Cheng [:josh] from comment #43)
> Hi Jessica,
> Right after I saved my last comment. I see ODM partner submitted exact same
> issue as bug 1090133. I am requesting the reporter for snapshot and console
> log. Do you still need anything else? 
> Thanks!

Thank you, Josh, I think we can discuss on that bug.
Flags: needinfo?(jjong)

Updated

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