[b2g-ril] Support calling line identification restriction (+CLIR)

RESOLVED FIXED in mozilla25

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jaoo, Assigned: jaoo)

Tracking

Trunk
mozilla25
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: u=commsapps-user c=dialer p=0 [fixed-in-birch])

Attachments

(4 attachments, 9 obsolete attachments)

5.78 KB, patch
Details | Diff | Splinter Review
1.46 KB, patch
Details | Diff | Splinter Review
12.71 KB, patch
Details | Diff | Splinter Review
4.36 KB, patch
Details | Diff | Splinter Review
It allows a calling subscriber to enable or disable the presentation of the calling line identity (CLI) to the called party when originating a call.

See TS 27.007 7.7.

Note: I've not been able to find that this bug is already reported. If so, feel free to resolve this one as duplicated. In the mean time I gonna work on it.

Comment 1

4 years ago
This bug is part of localization effort for MMI so nominating it for leo.
blocking-b2g: --- → leo?
Just FYI, There are
- CLIR  (ex: *31#)
- CLIP
- temporary CLIR  (ex: *31#PhoneNumber)
- temporary CLIP

You may consider designing a solution for all of them together.
(In reply to Anshul from comment #1)
> This bug is part of localization effort for MMI so nominating it for leo.

If I am not wrong, we already have the required bits in Gaia for localizing the resulting strings coming from CLIR or CLIP request via MMI. See https://bugzilla.mozilla.org/show_bug.cgi?id=883178#c30 and #c31.

I guess this bug is about supporting CLIR in the reference RIL. Which I don't think it should be a blocker.
(Assignee)

Comment 4

4 years ago
(In reply to Fernando Jiménez Moreno [:ferjm] (needinfo instead of CC, please) from comment #3)
> (In reply to Anshul from comment #1)
> I guess this bug is about supporting CLIR in the reference RIL. Which I
> don't think it should be a blocker.

Yep, It shouldn't be a blocker for v1.1 release AFAIK. I filed the bug just for staring implementing something new (missing) in the reference RIL. I don't see why this has been considered part of localization effort for MMI.
blocking-b2g: leo? → -
(Assignee)

Comment 5

4 years ago
Created attachment 770852 [details] [diff] [review]
Part 1: Support CLIR in mozMobileConnection API. WIP.

Comment 6

4 years ago
(In reply to Fernando Jiménez Moreno [:ferjm] (needinfo instead of CC, please) from comment #3)
> (In reply to Anshul from comment #1)
> > This bug is part of localization effort for MMI so nominating it for leo.
> 
> If I am not wrong, we already have the required bits in Gaia for localizing
> the resulting strings coming from CLIR or CLIP request via MMI. See
> https://bugzilla.mozilla.org/show_bug.cgi?id=883178#c30 and #c31.

Fernando, there's a bug with the existing code that does not localize statusMessage for the default case: https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/mmi.js#L192

Let me know if you need me to open a new but or if you want to track that change here.

> 
> I guess this bug is about supporting CLIR in the reference RIL. Which I
> don't think it should be a blocker.
Flags: needinfo?(ferjmoreno)
Thanks Carol! This is going to be fixed in Bug 890463.
Flags: needinfo?(ferjmoreno)

Updated

4 years ago
blocking-b2g: - → koi?
(Assignee)

Comment 8

4 years ago
Created attachment 772205 [details] [diff] [review]
Part 2: MobileConnection.cpp additions for supporting CLIR
(Assignee)

Comment 9

4 years ago
Created attachment 772206 [details] [diff] [review]
Part 3: RIL changes for CLIR support
Created attachment 772207 [details] [diff] [review]
Part 4: Test for CLIR
(Assignee)

Updated

4 years ago
Blocks: 891345

Updated

4 years ago
Whiteboard: u=commsapps-user c=dialer p=0

Updated

4 years ago
Flags: in-testsuite?
Created attachment 773118 [details] [diff] [review]
Part 1: Support CLIR in mozMobileConnection API. v1

Jonas, this bug add a way to handle calling line identification restriction feature. Could you take a look at it please? Thanks.
Attachment #770852 - Attachment is obsolete: true
Attachment #773118 - Flags: superreview?(jonas)
Created attachment 773119 [details] [diff] [review]
Part 2: MobileConnection.cpp additions for supporting CLIR. v1

He, this bug add a way to handle calling line identification restriction feature. Here are the changes in the c++ code. Could you take a look at it please? Thanks.
Attachment #772205 - Attachment is obsolete: true
Attachment #773119 - Flags: review?(bugs)
Created attachment 773122 [details] [diff] [review]
Part 3: RIL changes for CLIR support. v1

Vicamo, this bug add a way to handle calling line identification restriction feature. Here are the changes in the RIL plumbing. Could you take a look at it please? Thanks.
Attachment #772206 - Attachment is obsolete: true
Attachment #773122 - Flags: review?(vyang)
Created attachment 773123 [details] [diff] [review]
Part 4: Test for CLIR. v1

Vicamo, this bug add a way to handle calling line identification restriction feature. Here are the tests. Could you take a look at it please? Thanks.
Attachment #772207 - Attachment is obsolete: true
Attachment #773123 - Flags: review?(vyang)
https://tbpl.mozilla.org/?tree=Try&rev=e772e1667f08
Comment on attachment 773122 [details] [diff] [review]
Part 3: RIL changes for CLIR support. v1

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

queryCLIP is never referenced.  Anything missed?

::: dom/system/gonk/ril_worker.js
@@ +1426,5 @@
>    },
>  
>    /**
> +   * Queries current CLIR status.
> +   * 

trailing ws.

@@ +1430,5 @@
> +   * 
> +   */
> +  getCLIR: function getCLIR(options) {
> +    Buf.newParcel(REQUEST_GET_CLIR, options);
> +    Buf.sendParcel();

Buf.simpleRequest(REQUEST_GET_CLIR, options);

@@ +1452,5 @@
> +  /**
> +   * Queries current CLIP status.
> +   * 
> +   * (MMI request for code "*#30#")
> +   * 

ditto

@@ +1460,5 @@
> +   *         1 for CLIP provisioned and
> +   *         2 for CLIP not provisioned and
> +   *         3 for unknown.
> +   */
> +  queryCLIP: function queryCLIP(options) {

This function is never referenced.

@@ +1462,5 @@
> +   *         3 for unknown.
> +   */
> +  queryCLIP: function queryCLIP(options) {
> +    Buf.newParcel(REQUEST_QUERY_CLIP, options);
> +    Buf.sendParcel();

Buf.simpleRequest(REQUEST_QUERY_CLIP, options);

@@ +4888,5 @@
> +    options.errorMsg = RIL_ERROR_TO_GECKO_ERROR[options.rilRequestError];
> +    this.sendDOMMessage(options);
> +    return;
> +  }
> +  

ditto

@@ +5165,5 @@
> +    this.sendDOMMessage(options);
> +    return;
> +  }
> +
> +  options.provisioned = Buf.readUint32();

This doesn't match Android's parcel format.  RIL_REQUEST_QUERY_CLIP should return an array of integers.
Attachment #773122 - Flags: review?(vyang)
Comment on attachment 773123 [details] [diff] [review]
Part 4: Test for CLIR. v1

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

::: dom/system/gonk/tests/test_ril_worker_clir.js
@@ +89,5 @@
> +
> +  worker.RIL.getCLIR = function fakeGetCLIR(options) {
> +    worker.Buf.int32Array = [
> +      1,  // Presentation indicator is used according to the subscription
> +          // of the CLIR service. 

trailing ws

@@ +120,5 @@
> +
> +  worker.RIL.getCLIR = function fakeGetCLIR(options) {
> +    worker.Buf.int32Array = [
> +      1,  // Presentation indicator is used according to the subscription
> +          // of the CLIR service. 

ditto
Attachment #773123 - Flags: review?(vyang) → review+
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #16)
> Comment on attachment 773122 [details] [diff] [review]
> Part 3: RIL changes for CLIR support. v1
> 
> Review of attachment 773122 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> queryCLIP is never referenced.  Anything missed?

Nope, actually I've included the support for CLIP here since it's quite related to CLIR. CLIP (only query the status) is only handled by MMI codes. We could implement the getCLIP() function in ril_worker.js and use it once we develop the bits for CLIP support by MMI codes or delete the CLIP stuff and add it once we implement the support CLIP through MMI codes, what do you prefer Vicamo?
Flags: needinfo?(vyang)
Please just remove it for now.
Flags: needinfo?(vyang)
Created attachment 773249 [details] [diff] [review]
Part 3: RIL changes for CLIR support. v2

Review comments (comment #6) addressed.
Attachment #773122 - Attachment is obsolete: true
Attachment #773249 - Flags: review?(vyang)
Comment on attachment 773119 [details] [diff] [review]
Part 2: MobileConnection.cpp additions for supporting CLIR. v1

(I hope we'll move MobileConnection to use WebIDL and can move permission checks to WebIDL files, if possible.)
Attachment #773119 - Flags: review?(bugs) → review+
Comment on attachment 773249 [details] [diff] [review]
Part 3: RIL changes for CLIR support. v2

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

Thank you :)
Attachment #773249 - Flags: review?(vyang) → review+
(In reply to Olli Pettay [:smaug] from comment #21)
> I hope we'll move MobileConnection to use WebIDL and can move permission
> checks to WebIDL files, if possible.

Yes, that's also planned.  Actually because we think we might have to use WebIDL when creating new native classes, IPDLization (bug 815526) for all RIL related components are postponed.
(Assignee)

Updated

4 years ago
Blocks: 892498
(In reply to Szu-Yu Chen [:aknow] from comment #2)
> Just FYI, There are
> - CLIR  (ex: *31#)
> - CLIP
> - temporary CLIR  (ex: *31#PhoneNumber)
> - temporary CLIP
> 
> You may consider designing a solution for all of them together.

This will be implemented in bug 892498. Thanks for suggesting it.
Attachment #773118 - Flags: superreview?(jonas) → superreview+
Created attachment 777375 [details] [diff] [review]
Part 1: Support CLIR in mozMobileConnection API. sr=sicking
Attachment #773118 - Attachment is obsolete: true
Attachment #773119 - Attachment is obsolete: true
Attachment #773123 - Attachment is obsolete: true
Attachment #773249 - Attachment is obsolete: true
Created attachment 777376 [details] [diff] [review]
Part 2: MobileConnection.cpp additions for supporting CLIR. r=smaug
Created attachment 777377 [details] [diff] [review]
Part 3: RIL changes for CLIR support. r=vicamo
Created attachment 777378 [details] [diff] [review]
Part 4: Test for CLIR. r=vicamo
https://hg.mozilla.org/projects/birch/rev/3f0716221088
https://hg.mozilla.org/projects/birch/rev/868153040785
https://hg.mozilla.org/projects/birch/rev/14e71bab9c31
https://hg.mozilla.org/projects/birch/rev/6eb6a7a1210c
Whiteboard: u=commsapps-user c=dialer p=0 → u=commsapps-user c=dialer p=0 [fixed-in-birch]
Target Milestone: --- → mozilla25
https://hg.mozilla.org/mozilla-central/rev/3f0716221088
https://hg.mozilla.org/mozilla-central/rev/868153040785
https://hg.mozilla.org/mozilla-central/rev/14e71bab9c31
https://hg.mozilla.org/mozilla-central/rev/6eb6a7a1210c
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
See Also: → bug 897869

Updated

4 years ago
blocking-b2g: koi? → ---
Removing koi? since this is in gecko 25 which is < 26 which will be the basis for Firefox OS 1.2.
You need to log in before you can comment on or make changes to this bug.