Closed Bug 893727 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

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

People

(Reporter: jaoo, Assigned: jaoo)

References

Details

(Whiteboard: [TEF][UCID:Comms32, FT:comms, KOI:P1][Sprint 1][Status: pending qa approval]u=commsapps-user c=dialer p=0 [fixed-in-birch])

Attachments

(3 files, 3 obsolete files)

Handle support calling line identification presentation feature through MMI codes.
Whiteboard: u=commsapps-user c=dialer p=0
Attachment #775640 - Attachment is obsolete: true
Attached patch Part 2: Test for CLIP. v1 (obsolete) — Splinter Review
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.
Attachment #776273 - Flags: review?(ferjmoreno)
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.
Attachment #776274 - Flags: review?(ferjmoreno)
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.
Attachment #776273 - Flags: review?(ferjmoreno) → review+
Attachment #776274 - Flags: review?(ferjmoreno) → review+
Attachment #776273 - Attachment is obsolete: true
Attachment #776274 - Attachment is obsolete: true
https://hg.mozilla.org/projects/birch/rev/7acaefad07a2
https://hg.mozilla.org/projects/birch/rev/02d794e1f3b9
Whiteboard: u=commsapps-user c=dialer p=0 → u=commsapps-user c=dialer p=0 [fixed-in-birch]
https://hg.mozilla.org/mozilla-central/rev/02d794e1f3b9
https://hg.mozilla.org/mozilla-central/rev/7acaefad07a2
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Whiteboard: u=commsapps-user c=dialer p=0 [fixed-in-birch] → [TEF][UCID:Comms32, FT:comms, KOI:P1][Sprint 1][Status: pending qa approval]u=commsapps-user c=dialer p=0 [fixed-in-birch]
MMI code for Call Line Identification Presentation US description: recap
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.
Status: RESOLVED → VERIFIED
koi+ per https://wiki.mozilla.org/B2G/Roadmap
blocking-b2g: --- → koi+
QA Contact: rafael.marquez
You need to log in before you can comment on or make changes to this bug.