1001352 - [B2G][RIL][Flame] Enable data subscription for flame dsds mode.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Ken Chang[:kenkai]]

There is a new interface added in flame. The interface is used for enabling data service in a specific SIM, otherwise, the data service cannot be enabled in flame. Therefore, we need to add this mechanism in reference ril for flame.

Comment 1

[Edgar Chen [:edgar]]

Attachment: Patch, v1

Comment 2

[Edgar Chen [:edgar]]

Use the system property which was introduced in bug 1003011 to control this feature. (ro.moz.ril.subscription_control)

Comment 3

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

Comment on attachment 8417324 [details] [diff] [review]
Patch, v1

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +793,5 @@
>          let radioInterface = connHandler.radioInterface;
>          if (RILQUIRKS_DATA_REGISTRATION_ON_DEMAND) {
>            radioInterface.setDataRegistration(true);
>          }
> +        if (RILQUIRKS_SUBSCRIPTION_CONTROL) {

RILQUIRKS_DATA_REGISTRATION_ON_DEMAND and RILQUIRKS_SUBSCRIPTION_CONTROL need to be exclusive. Please do 

if (..._ON_DEMAND) {
} else if (..._CONTROL) {
}

so once _ON_DEMAND is true, we don't need to check _CONTROL.

@@ +818,5 @@
>            oldIface.setDataRegistration(false);
>            newIface.setDataRegistration(true);
>          }
> +        if (RILQUIRKS_SUBSCRIPTION_CONTROL) {
> +          newIface.setDataSubscription();

ditto.

@@ +836,5 @@
>            if (RILQUIRKS_DATA_REGISTRATION_ON_DEMAND) {
>              newIface.setDataRegistration(true);
>            }
> +          if (RILQUIRKS_SUBSCRIPTION_CONTROL) {
> +            newIface.setDataSubscription();

ditto.

@@ +862,5 @@
>          oldIface.setDataRegistration(false);
>          newIface.setDataRegistration(true);
>        }
> +      if (RILQUIRKS_SUBSCRIPTION_CONTROL) {
> +        newIface.setDataSubscription();

ditto.

::: dom/system/gonk/ril_worker.js
@@ +1365,5 @@
> +   */
> +  setDataSubscription: function(options) {
> +    this.context.Buf.simpleRequest(REQUEST_SET_DATA_SUBSCRIPTION, options);
> +  },
> +

In fugu we did [1] to a) track the previous preference before radio off, and b) track the preference on bootup, because we will try to attach data registration on the default sim for data, but it would fail due to 'radio not available', so we need to set it again on radio on. We need to consider those for Flame as well, right?

[1] http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js?from=ril_worker.js&case=true#6486

Comment 4

[Edgar Chen [:edgar]]

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #3)
> Comment on attachment 8417324 [details] [diff] [review]
> Patch, v1
> 
> Review of attachment 8417324 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/RadioInterfaceLayer.js
> @@ +793,5 @@
> >          let radioInterface = connHandler.radioInterface;
> >          if (RILQUIRKS_DATA_REGISTRATION_ON_DEMAND) {
> >            radioInterface.setDataRegistration(true);
> >          }
> > +        if (RILQUIRKS_SUBSCRIPTION_CONTROL) {
> 
> RILQUIRKS_DATA_REGISTRATION_ON_DEMAND and RILQUIRKS_SUBSCRIPTION_CONTROL
> need to be exclusive. Please do 
Will do this in next version, thank you.

> 
> if (..._ON_DEMAND) {
> } else if (..._CONTROL) {
> }
> 
> so once _ON_DEMAND is true, we don't need to check _CONTROL.
> 
> @@ +818,5 @@
> >            oldIface.setDataRegistration(false);
> >            newIface.setDataRegistration(true);
> >          }
> > +        if (RILQUIRKS_SUBSCRIPTION_CONTROL) {
> > +          newIface.setDataSubscription();
> 
> ditto.
> 
> @@ +836,5 @@
> >            if (RILQUIRKS_DATA_REGISTRATION_ON_DEMAND) {
> >              newIface.setDataRegistration(true);
> >            }
> > +          if (RILQUIRKS_SUBSCRIPTION_CONTROL) {
> > +            newIface.setDataSubscription();
> 
> ditto.
> 
> @@ +862,5 @@
> >          oldIface.setDataRegistration(false);
> >          newIface.setDataRegistration(true);
> >        }
> > +      if (RILQUIRKS_SUBSCRIPTION_CONTROL) {
> > +        newIface.setDataSubscription();
> 
> ditto.
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +1365,5 @@
> > +   */
> > +  setDataSubscription: function(options) {
> > +    this.context.Buf.simpleRequest(REQUEST_SET_DATA_SUBSCRIPTION, options);
> > +  },
> > +
> 
> In fugu we did [1] to a) track the previous preference before radio off, and
> b) track the preference on bootup, because we will try to attach data
> registration on the default sim for data, but it would fail due to 'radio
> not available', so we need to set it again on radio on. We need to consider
> those for Flame as well, right?

Oh, I missed this case, will address this in next version. Thank you.

> 
> [1]
> http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.
> js?from=ril_worker.js&case=true#6486

Comment 5

[Edgar Chen [:edgar]]

Attachment: Patch, v2

Comment 6

[Edgar Chen [:edgar]]

Attachment: Patch, v2

Uploaded a wrong patch, attach correct one.

Comment 7

[Edgar Chen [:edgar]]

Comment on attachment 8418524 [details] [diff] [review]
Patch, v2

In this version, I adjust the logic a little bit.
Because the usage of data subscription is almost the same as |setDataRegistration()| (the fugu one), so I reuse the same call path but send different RIL REQUEST (this also cover the radio on case). How about this way? Thank you.

Comment 8

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

Comment on attachment 8418524 [details] [diff] [review]
Patch, v2

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

Looks good, thank you!

Comment 9

[Edgar Chen [:edgar]]

Try result: https://tbpl.mozilla.org/?tree=Try&rev=d66bd4d807f6

Comment 10

[Edgar Chen [:edgar]]

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

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/52c25214f476

Comment 12

[Edgar Chen [:edgar]]

Noming for 1.4 given that Flame needs this for dsds data connection.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/2cf0233532cb