893727 - MMI codes: support calling line identification presentation (+CLIP)

Status: VERIFIED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[José Antonio Olivera Ortega [:jaoo]]

Handle support calling line identification presentation feature through MMI codes.

Comment 1

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 1: RIL changes for CLIP support. WIP v1

Comment 2

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 1: RIL changes for CLIP support. v1

Comment 3

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 2: Test for CLIP. v1

Comment 4

[José Antonio Olivera Ortega [:jaoo]]

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

Comment 5

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 776273 [details] [diff] [review]
Part 1: RIL changes for CLIP support. v1

Fernando, this patch adds CLIP supplementary service logic. Could you take a look at it please? Thanks.

Comment 6

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 776274 [details] [diff] [review]
Part 2: Test for CLIP. v1

Fernando, here are some tests for CLIP supplementary service logic. Could you take a look at it please? Thanks.

Comment 7

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 776273 [details] [diff] [review]
Part 1: RIL changes for CLIP support. v1

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

Thanks jaoo!

r=me with the comments addressed, please :)

::: dom/system/gonk/ril_worker.js
@@ +1459,5 @@
> +   * Queries current CLIP status.
> +   *
> +   * (MMI request for code "*#30#")
> +   *
> +   * @return options.provisioned

This function does not really return anything. You can either move this comment to REQUEST_QUERY_CLIP parcel handler or reference that handler from here.

@@ +2419,5 @@
> +        options.procedure = mmi.procedure;
> +        if (options.procedure === MMI_PROCEDURE_INTERROGATION) {
> +          this.queryCLIP(options);
> +        } else {
> +          _sendMMIError(MMI_ERROR_KS_NOT_SUPPORTED);

You know the service code at this point and we support some CLIP procedures, so add it to the error, please:

_sendMMIError(MMI_ERROR_KS_NOT_SUPPORTED, MMI_KS_SC_CLIP);

@@ +5170,5 @@
>  };
>  RIL[REQUEST_SEPARATE_CONNECTION] = null;
>  RIL[REQUEST_SET_MUTE] = null;
>  RIL[REQUEST_GET_MUTE] = null;
> +RIL[REQUEST_QUERY_CLIP] = function REQUEST_QUERY_CLIP(length, options) {

You are declaring another 'length' variable later, so I assume that you can remove this one.

@@ +5184,5 @@
> +    options.success = false;
> +    options.errorMsg = GECKO_ERROR_GENERIC_FAILURE;
> +    this.sendDOMMessage(options);
> +    return;
> +  }

You can probably say this as:

if (!options.success || !length || length <= 0) {
  options.success = false;
  options.errorMsg = options.rilRequestError ?
                     RIL_ERROR_TO_GECKO_ERROR[options.rilRequestError] :
                     GECKO_ERROR_GENERIC_FAILURE;
  this.sendDOMMessage(options);
  return;
}

@@ +5188,5 @@
> +  }
> +
> +  options.provisioned = Buf.readUint32();
> +  if (options.rilMessageType === "sendMMI") {
> +    var statusMessage;

s/var/let

But in any case, you don't need this variable.

@@ +5191,5 @@
> +  if (options.rilMessageType === "sendMMI") {
> +    var statusMessage;
> +    switch (options.provisioned) {
> +      case 0:
> +        statusMessage = MMI_SM_KS_SERVICE_DISABLED;

s/statusMessage/options.statusMessage

@@ +5194,5 @@
> +      case 0:
> +        statusMessage = MMI_SM_KS_SERVICE_DISABLED;
> +        break;
> +      case 1:
> +        statusMessage = MMI_SM_KS_SERVICE_ENABLED;

ditto

@@ +5201,5 @@
> +        options.success = false;
> +        options.errorMsg = MMI_ERROR_KS_ERROR;
> +        break;
> +    }
> +    options.statusMessage = statusMessage;

Remove this line.

Comment 8

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 1: RIL changes for CLIP support. r=ferjm

Comment 9

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 2: Part 2: Test for CLIP. r=ferjm

Comment 10

[José Antonio Olivera Ortega [:jaoo]]

https://hg.mozilla.org/projects/birch/rev/7acaefad07a2
https://hg.mozilla.org/projects/birch/rev/02d794e1f3b9

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/02d794e1f3b9
https://hg.mozilla.org/mozilla-central/rev/7acaefad07a2

Comment 12

[Noemí Freire (:noemi)]

Attachment: MMI code for Call Line Identification Presentation US description: recap

MMI code for Call Line Identification Presentation US description: recap

Comment 13

[rafael.marquez]

The user story has been tested with satisfactory results. We could not run the test when the state is "0 for CLIP not provisioned" or "2 for unknown".

The tests have been executed with a SIM Tuenti. We tested several movistar SIMS, but we could not activating you supplementary services to this type of SIM.

Comment 14

[Alex Keybl [:akeybl]]

koi+ per https://wiki.mozilla.org/B2G/Roadmap