Closed Bug 869768 Opened 7 years ago Closed 6 years ago

[B2G][CDMA] Set and Query the roaming preference in CDMA

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25
blocking-b2g koi+
Tracking Status
firefox26 --- fixed

People

(Reporter: kchang, Assigned: edgar)

References

Details

(Whiteboard: [fixed-in-birch])

Attachments

(3 files, 4 obsolete files)

We should have the interfaces, RIL_REQUEST_CDMA_SET_ROAMING_PREFERENCE and RIL_REQUEST_CDMA_QUERY_ROAMING_PREFERENCE, to set and query the roaming preference in CDMA.
Blocks: 869771
(In reply to Ken Chang from comment #0)
> We should have the interfaces, RIL_REQUEST_CDMA_SET_ROAMING_PREFERENCE and
> RIL_REQUEST_CDMA_QUERY_ROAMING_PREFERENCE, to set and query the roaming
> preference in CDMA.

We should also handle the interface, RIL_UNSOL_CDMA_PRL_CHANGED, for this bug.
No longer blocks: 869771
Blocks: b2g-ril-cdma
Assignee: nobody → echen
The network selection mode of cdma is quite different from gsm. Gsm allows manually select a specific network based on scan result.  Not like gsm, cdma dose not allow select a specific network. The network selecting of cdma should reference "Preferred Roaming List (PRL)" [1]. And the PRL can be updated using OTASP.

After some tracing, I did not find AOSP handle network selection for CDMA. It seems modem will cover this.
But we still need to provide a way to set roaming preference and also get PRL version.

Thanks

[1] Please see C.S0016-D_v2.0 ANNEX C
(In reply to Ken Chang from comment #1)
> (In reply to Ken Chang from comment #0)
> > We should have the interfaces, RIL_REQUEST_CDMA_SET_ROAMING_PREFERENCE and
> > RIL_REQUEST_CDMA_QUERY_ROAMING_PREFERENCE, to set and query the roaming
> > preference in CDMA.
> 
> We should also handle the interface, RIL_UNSOL_CDMA_PRL_CHANGED, for this
> bug.

I think RIL_UNSOL_CDMA_PRL_CHANGED should not be handled in this bug, bug 869778 is more suitable for that. Please see bug 869778 comment #1. Thanks
Ok, let's implement RIL_UNSOL_CDMA_PRL_CHANGED for bug 869778.
Whiteboard: RN5/29
Whiteboard: RN5/29 → RN6/7
As mention in comment #2. We need to add new APIs to set/quert roaming preference for cdma. The mobileConnection is a suitable place for these APIs, since it already has some network selection APIs for gsm, like |getNetworks()|, |selectNetwork()| ... etc.
Add new interfaces for setting/querying roaming preference. Request returns an error if the network doesn't support this function (In gsm network).
Adding Casey for UX changes needed here to support CDMA.
Flags: needinfo?(kyee)
Can you please provide more insight on the requirement?   

" RIL_REQUEST_CDMA_SET_ROAMING_PREFERENCE and RIL_REQUEST_CDMA_QUERY_ROAMING_PREFERENCE, to set and query the roaming preference in CDMA."

This unfortunately doesn't mean much to me.

What is this feature used for?  How is this implemented in other devices?
Flags: needinfo?(kyee)
Forgot ot Change the UUID, fix it.
Attachment #758425 - Attachment is obsolete: true
Comment on attachment 762469 [details] [diff] [review]
Part 1: Add interface for roaming preference (IDL), v2

Hi Hsinyi/Jonas,

I would like to request review for interface first. The network selection of CDMA is different with GSM. CDMA only allows user to select roaming mode [1]. And these api will return an error if UE is not support CDMA.

Thanks

[1] please see comment #2 and comment #9.
Attachment #762469 - Flags: superreview?(jonas)
Attachment #762469 - Flags: review?(htsai)
Comment on attachment 762469 [details] [diff] [review]
Part 1: Add interface for roaming preference (IDL), v2

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

Looks good to me. Thanks!
Attachment #762469 - Flags: review?(htsai) → review+
Blocks: 890809
Blocks: 890330
No longer blocks: 890330
Blocks: 891742
(In reply to Edgar Chen [:edgar][:echen] from comment #9)
> This allows user to select roaming mode (home only, roaming only or any).

Correct the roaming mode we support.
1. home only.
2. roaming on affiliated networks.
3. roaming on any network.
(In reply to Edgar Chen [:edgar][:echen] from comment #16)
> (In reply to Edgar Chen [:edgar][:echen] from comment #9)
> > This allows user to select roaming mode (home only, roaming only or any).
> 
> Correct the roaming mode we support.
> 1. home only.
> 2. roaming on affiliated networks.
> 3. roaming on any network.

Anshul, I have some questions about the behavior of ril request, RIL_REQUEST_CDMA_SET_ROAMING_PREFERENCE [1] as below. Could you help to provide some information? Thanks in advance.

What is meaning of "affiliated networks"?
From the spec, C.S0016-D_v2.0, I did not find any 

What is meaning of "roaming on any network"?
Dose it means the any network which is showed in PRL?
Or device will camp on a network even it is not showed in PRL?
 
[1] https://github.com/android/platform_hardware_ril/blob/master/include/telephony/ril.h#L2697
Flags: needinfo?(anshulj)
(In reply to Edgar Chen [:edgar][:echen] from comment #17)
> (In reply to Edgar Chen [:edgar][:echen] from comment #16)
> What is meaning of "affiliated networks"?
> From the spec, C.S0016-D_v2.0, I did not find any 
From the spec, C.S0016-D_v2.0, I did not find any clue about the meaning of "affiliated".
Edgar, I am not familiar with this API myself. I will get back to you when I can find some time to investigate the API further.
Flags: needinfo?(anshulj)
Test by cdma device + CT card in Asia Pacific Telecom network, these patches work as expected.
* Set to "home only", device can not connect to the roaming network.
* Set to "roaming on affiliated networks" or "roaming on any network", device can connect to the roaming network.
Attachment #774461 - Flags: review?(bugs)
Attachment #774480 - Flags: review?(htsai)
Hi Jonas:

Could you help to review this interface changes for roaming preference of CDMA?

Thanks in advance
Attachment #774461 - Flags: review?(bugs) → review+
Comment on attachment 774480 [details] [diff] [review]
Part 3: RIL implementation for roaming preference, v1

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

::: dom/system/gonk/RILContentHelper.js
@@ +614,5 @@
> +    }
> +
> +    let request = Services.DOMRequest.createRequest(window);
> +    let requestId = this.getRequestId(request);
> +

Let's bail out earlier if mode is null.

if (!mode) {
  this.dispatchFireRequestError(requestId, "InvalidParameter");
 return request;
}

::: dom/system/gonk/ril_worker.js
@@ +1529,5 @@
>    /**
> +   * Set the roaming preference mode
> +   */
> +  setRoamingPreference: function setRoamingPreference(options) {
> +    if (!options || !options.mode) {

It's fine to remove this as we already bail out for null mode in RILContentHelper.

@@ +5278,5 @@
> +    this.sendDOMMessage(options);
> +    return;
> +  }
> +
> +  options.mode = CDMA_ROAMING_PREFERENCE_TO_GECKO[Buf.readUint32List()[0]];

Let's use

let mode = Buf.readUint32List();
options.mode = CDMA_ROAMING_PREFERENCE_TO_GECKO[mode[0]];
Attachment #774480 - Flags: review?(htsai)
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #22)
> Comment on attachment 774480 [details] [diff] [review]
> Part 3: RIL implementation for roaming preference, v1
> 
> Review of attachment 774480 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/RILContentHelper.js
> @@ +614,5 @@
> > +    }
> > +
> > +    let request = Services.DOMRequest.createRequest(window);
> > +    let requestId = this.getRequestId(request);
> > +
> 
> Let's bail out earlier if mode is null.
> 
> if (!mode) {
>   this.dispatchFireRequestError(requestId, "InvalidParameter");
>  return request;
> }
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +1529,5 @@
> >    /**
> > +   * Set the roaming preference mode
> > +   */
> > +  setRoamingPreference: function setRoamingPreference(options) {
> > +    if (!options || !options.mode) {
> 
> It's fine to remove this as we already bail out for null mode in
> RILContentHelper.
> 
> @@ +5278,5 @@
> > +    this.sendDOMMessage(options);
> > +    return;
> > +  }
> > +
> > +  options.mode = CDMA_ROAMING_PREFERENCE_TO_GECKO[Buf.readUint32List()[0]];
> 
> Let's use
> 
> let mode = Buf.readUint32List();
> options.mode = CDMA_ROAMING_PREFERENCE_TO_GECKO[mode[0]];

Okay, I will provide a new version to address these. Thanks.
blocking-b2g: --- → koi+
1). Rebase.
2). Address review comment #22.
Attachment #774480 - Attachment is obsolete: true
Attachment #779099 - Flags: review?(htsai)
Comment on attachment 779099 [details] [diff] [review]
Part 3: RIL implementation for roaming preference, v2

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

Awesome! Thank you :)
Attachment #779099 - Flags: review?(htsai) → review+
Blocks: 896394
Comment on attachment 762469 [details] [diff] [review]
Part 1: Add interface for roaming preference (IDL), v2

Hi Mounir:

Could you please help to take a look at this interface changes? (For more details please see comment #12)
I think Jonas is quite busy now. :)

Thanks in advance
Attachment #762469 - Flags: superreview?(jonas) → superreview?(mounir)
Comment on attachment 762469 [details] [diff] [review]
Part 1: Add interface for roaming preference (IDL), v2

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

When you have time, could you try to make this interface a WebIDL?

::: dom/network/interfaces/nsIDOMMobileConnection.idl
@@ +136,5 @@
> +   * Otherwise, the request's onerror will be called, and the request's error
> +   * will be either 'RadioNotAvailable', 'RequestNotSupported',
> +   * 'IllegalSIMorME', or 'GenericFailure'
> +   */
> +  nsIDOMDOMRequest setRoamingPreference(in DOMString mode);

Could you add a TODO saying that "mode" should be a WebIDL enum when this interface is converted to WebIDL?
Attachment #762469 - Flags: superreview?(mounir) → superreview+
(In reply to Mounir Lamouri (:mounir) from comment #27)
> Comment on attachment 762469 [details] [diff] [review]
> Part 1: Add interface for roaming preference (IDL), v2
> 
> Review of attachment 762469 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> When you have time, could you try to make this interface a WebIDL?
Sure, I will file a follow-up for WebIDL.

> 
> ::: dom/network/interfaces/nsIDOMMobileConnection.idl
> @@ +136,5 @@
> > +   * Otherwise, the request's onerror will be called, and the request's error
> > +   * will be either 'RadioNotAvailable', 'RequestNotSupported',
> > +   * 'IllegalSIMorME', or 'GenericFailure'
> > +   */
> > +  nsIDOMDOMRequest setRoamingPreference(in DOMString mode);
> 
> Could you add a TODO saying that "mode" should be a WebIDL enum when this
> interface is converted to WebIDL?

Okay.

Thanks for your review :)
1). Rebase.
2). Address review comment #28.
3). Add r=hsinyi and sr=mounir after r+ and sr+.
Attachment #762469 - Attachment is obsolete: true
Attachment #781727 - Flags: superreview+
Attachment #781727 - Flags: review+
Attachment #781727 - Attachment filename: bug_869768_part1_add_interface_for_roaming_preference_idl_v3_r=hsinyi_sr=mounir.patch → Part 1: Add interface for roaming preference (IDL), v3, r=hsinyi, sr=mounir
Attachment #781727 - Attachment description: bug_869768_part1_add_interface_for_roaming_preference_idl_v3_r=hsinyi_sr=mounir.patch → Part 1: Add interface for roaming preference (IDL), v3, r=hsinyi, sr=mounir
Attachment #781727 - Attachment filename: Part 1: Add interface for roaming preference (IDL), v3, r=hsinyi, sr=mounir → bug_869768_part1_add_interface_for_roaming_preference_idl_v3_r=hsinyi_sr=mounir.patch
Add r=bugs after r+.
Attachment #774461 - Attachment is obsolete: true
Attachment #781730 - Flags: review+
Whiteboard: RN6/7, [fixed-in-birch] → [fixed-in-birch]
Comment on attachment 779099 [details] [diff] [review]
Part 3: RIL implementation for roaming preference, v2

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

::: dom/system/gonk/ril_worker.js
@@ +1620,5 @@
> +    let roamingMode = CDMA_ROAMING_PREFERENCE_TO_GECKO.indexOf(options.mode);
> +
> +    if (roamingMode === -1) {
> +      options.errorMsg = GECKO_ERROR_INVALID_PARAMETER;
> +      this.sendDOMMessage(options);

Hmm.. sendDOMMessage is already renamed to sendChromeMessage. I will file a follow-up to correct this.
Blocks: 899484
(In reply to Anshul from comment #19)
> Edgar, I am not familiar with this API myself. I will get back to you when I
> can find some time to investigate the API further.

Hi Anshul,

Do you update further update of this question? I would like to apply CDMA account which can be used to test non-affiliated network case. But could you provide any detail of how can device tell whether it's affiliated carrier or not? By PRL? Which value in the PRL table? This is crucial for establishing testing environment. Thank you.
Flags: needinfo?(anshulj)
Hi Anshul,

Do you have any update? Or should we remove non-affiliated item from current roaming preference options?
echu, I am sorry I am not an expert in this area and won't be able to provide any help here.
Flags: needinfo?(anshulj)
You need to log in before you can comment on or make changes to this bug.