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

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[José Antonio Olivera Ortega [:jaoo]]

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

[Anshul]

This bug is part of localization effort for MMI so nominating it for leo.

Comment 2

[Szu-Yu Chen [:aknow] (inactive)]

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.

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

(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.

Comment 4

[José Antonio Olivera Ortega [:jaoo]]

(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.

Comment 5

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 1: Support CLIR in mozMobileConnection API. WIP.

Comment 6

[Carol Yang [:cyang]]

(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.

Comment 7

[Fernando Jiménez Moreno [:ferjm]]

Thanks Carol! This is going to be fixed in Bug 890463.

Comment 8

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 2: MobileConnection.cpp additions for supporting CLIR

Comment 9

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 3: RIL changes for CLIR support

Comment 10

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 4: Test for CLIR

Comment 11

[José Antonio Olivera Ortega [:jaoo]]

Attachment: 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.

Comment 12

[José Antonio Olivera Ortega [:jaoo]]

Attachment: 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.

Comment 13

[José Antonio Olivera Ortega [:jaoo]]

Attachment: 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.

Comment 14

[José Antonio Olivera Ortega [:jaoo]]

Attachment: 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.

Comment 15

[José Antonio Olivera Ortega [:jaoo]]

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

Comment 16

[Vicamo Yang [:vicamo][:vyang]]

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.

Comment 17

[Vicamo Yang [:vicamo][: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

Comment 18

[José Antonio Olivera Ortega [:jaoo]]

(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?

Comment 19

[Vicamo Yang [:vicamo][:vyang]]

Please just remove it for now.

Comment 20

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 3: RIL changes for CLIR support. v2

Review comments (comment #6) addressed.

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

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.)

Comment 22

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 773249 [details] [diff] [review]
Part 3: RIL changes for CLIR support. v2

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

Thank you :)

Comment 23

[Vicamo Yang [:vicamo][:vyang]]

(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.

Comment 24

[José Antonio Olivera Ortega [:jaoo]]

(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.

Comment 25

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 1: Support CLIR in mozMobileConnection API. sr=sicking

Comment 26

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 2: MobileConnection.cpp additions for supporting CLIR. r=smaug

Comment 27

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 3: RIL changes for CLIR support. r=vicamo

Comment 28

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 4: Test for CLIR. r=vicamo

Comment 29

[José Antonio Olivera Ortega [:jaoo]]

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

Comment 30

[Ryan VanderMeulen [:RyanVM]]

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

Comment 31

[Andrew Overholt [:overholt]]

Removing koi? since this is in gecko 25 which is < 26 which will be the basis for Firefox OS 1.2.