Note: There are a few cases of duplicates in user autocompletion which are being worked on.

B2G RIL: Support USSD codes

RESOLVED FIXED in mozilla16

Status

()

Core
DOM: Device Interfaces
P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mwu, Assigned: ferjm)

Tracking

Trunk
mozilla16
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(blocking-kilimanjaro:+, blocking-basecamp:+)

Details

(Whiteboard: [qa+])

Attachments

(4 attachments, 12 obsolete attachments)

6.62 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
6.58 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
7.21 KB, patch
Details | Diff | Splinter Review
12.33 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
On T-Mobile you can find out your number by dialing #686# and it displays a message with your phone number. This requires support for USSD codes. Other carriers have similar things.
Indeed, other carriers send you updates on your balance after each call or text, 3G settings, push notifications, etc.

This bug will cover the RIL side of things which we can implement right away. We will also have to figure out how to expose this to content. I will start this discussion on dev-webapi shortly.
Summary: Support USSD codes → B2G RIL: Support USSD codes
(Assignee)

Updated

5 years ago
Assignee: nobody → ferjmoreno
(Assignee)

Comment 2

5 years ago
Created attachment 622555 [details] [diff] [review]
Part 1: IDL v1

I guess I should give you some background on this, as USSD are not something usually known.
From Wikipedia: "USSD is a protocol used by GSM cellular telephones to communicate with the service provider's computers. USSD can be used for WAP browsing, prepaid callback service, mobile-money services, location-based content services, menu-based information services, and as part of configuring the phone on the network.
USSD messages are up to 182 alphanumeric characters in length. Unlike Short Message Service (SMS) messages, USSD messages create a real-time connection during a USSD session. The connection remains open, allowing a two-way exchange of a sequence of data".

The RIL currently exposes two functions to work with USSD codes:
- REQUEST_SEND_USSD: which creates a new session if none previously exists and sends the USSD in the context of that session. Only one session may exists at a time and it exists until the network sends an USSD with code 0 (no further action) or 2 (session terminated) or until a REQUEST_CANCEL_USSD parcel is sent to the network.
- REQUEST_CANCEL_USSD: Cancels the current USSD session if one exists.

The UNSOL_ON_USSD parcel is received when a new USSD is received by the RIL. It is followed by a type code and a chunk of data (the USSD message in UTF-8) if applicable.

As an example of use case, the following is the flow for a fictional menu-driven USSD application (operator side) enabling a mobile user to view his/her current mobile account balance and top up as needed:
1. A mobile user initiates the “Balance Enquiry and Top Up” service by dialing the USSD string defined by the service provider; for example, *#123#. (REQUEST_SEND_USSD)
1.1 The operator server responds with an "Accepted request" USSD message. (UNSOL_ON_USSD).
2. The USSD application receives the service request from the user and responds by sending the user a menu of options. (UNSOL_ON_USSD)
3. The user responds by selecting a “current balance” option. (REQUEST_SEND_USSD)
4. The USSD application sends back details of the mobile user’s current account balance and also gives the option to top up the balance. (UNSOL_ON_USSD)
5. The user selects to top up his/her account. (REQUEST_SEND_USSD)
6. The application responds by asking how much credit to
add? (UNSOL_ON_USSD)
7. The mobile user responds with the amount to add. (REQUEST_SEND_USSD)
8. The USSD application responds by sending an updated balance. (UNSOL_ON_USSD)
8.1 The USSD application responds ending the session. (UNSOL_ON_USSD)

So, given the previous explanation, after talking to Philipp, we agreed to expose this functionality via WebMobileConnection API. Here it goes the proposed modifications for its idl.
Attachment #622555 - Flags: feedback?(jonas)
Comment on attachment 622555 [details] [diff] [review]
Part 1: IDL v1

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

::: dom/network/interfaces/nsIDOMMobileConnection.idl
@@ +38,5 @@
>  
>    /**
> +   * Send a USSD message.
> +   *
> +   * The network reply should be reported via onussd event.

s/should/will/

Also worth noting that the 'success' event for the request means the USSD message has been sent successfully.

@@ +68,5 @@
>  
> +  /**
> +   * The 'ussd' event is notified whenever a new USSD message is received. 
> +   */
> +  attribute nsIDOMEventListener onussd;

bikeshed: I would suggest 'onussdreceived'
Attachment #622555 - Flags: feedback+
(Assignee)

Comment 4

5 years ago
Created attachment 623021 [details] [diff] [review]
Part 2: RIL v1

I´ve tested the RIL part in Gaia. Please, ignore the DEBUG flags set to true as it will not be that way on the final patch.
Attachment #622555 - Attachment is obsolete: true
Attachment #623021 - Flags: feedback?(philipp)
Attachment #622555 - Flags: feedback?(jonas)
(Assignee)

Updated

5 years ago
Attachment #622555 - Attachment is obsolete: false
(Assignee)

Updated

5 years ago
Attachment #622555 - Flags: feedback+ → feedback?(jonas)
Attachment #622555 - Flags: feedback?(jonas) → feedback+
Comment on attachment 623021 [details] [diff] [review]
Part 2: RIL v1

Nice! Just make sure not to include parts from the IDL patch (part 1) in the final version.
Attachment #623021 - Flags: feedback?(philipp) → feedback+

Updated

5 years ago
Priority: -- → P1
(Assignee)

Comment 6

5 years ago
Created attachment 625240 [details] [diff] [review]
Part 1: IDLs v2

Added nsIDOMUSSDReceivedEvent.idl and some additions to nsIMobileConnectionProvider.idl.
Attachment #622555 - Attachment is obsolete: true
Attachment #625240 - Flags: feedback?(philipp)
(Assignee)

Comment 7

5 years ago
Created attachment 625242 [details] [diff] [review]
Part 3: DOMEvent v1

I am specially interested on your feedback about the addition of this event.
Attachment #625242 - Flags: feedback?(philipp)
(Assignee)

Comment 8

5 years ago
Created attachment 625244 [details] [diff] [review]
Part 4: MobileConnection v1

I am missing the DOMRequest part, some error handling and the addition of some tests for this. Could I fill follow-up bugs for any of this?
Attachment #625244 - Flags: feedback?(philipp)
(Assignee)

Comment 9

5 years ago
Created attachment 625245 [details] [diff] [review]
Part 2: RIL v2
Attachment #623021 - Attachment is obsolete: true
Attachment #625245 - Flags: feedback?(philipp)
(Assignee)

Comment 10

5 years ago
All this patches have been successfully tested with Gaia. Both USSD types, terminal initiated and network initiated works.
(Assignee)

Comment 11

5 years ago
Comment on attachment 625240 [details] [diff] [review]
Part 1: IDLs v2

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

::: dom/network/interfaces/nsIDOMMobileConnection.idl
@@ +38,5 @@
>  
>    /**
> +   * Send a USSD message.
> +   *
> +   * The network reply will be reported via onussd event.

Sorry, I´ve just noticed this error on the comment. s/onussd/onussdreceived/. I´ll change it on the next version.
Just FYI the DOM patches should have a DOM peer look over them. mounir or sicking or me, maybe?
No longer blocks: 710489
(Assignee)

Comment 13

5 years ago
(In reply to ben turner [:bent] from comment #12)
> Just FYI the DOM patches should have a DOM peer look over them. mounir or
> sicking or me, maybe?
Didn´t know that. Thanks :). Jonas was on CC though.
(Assignee)

Comment 14

5 years ago
Created attachment 625494 [details] [diff] [review]
Part 1: IDLs v2

Minor change on comment
Attachment #625494 - Flags: feedback?(philipp)
(Assignee)

Updated

5 years ago
Attachment #625240 - Attachment is obsolete: true
Attachment #625240 - Flags: feedback?(philipp)
(Assignee)

Comment 15

5 years ago
Created attachment 625495 [details] [diff] [review]
Part 2: RIL v3

DOMRequest support added
Attachment #625245 - Attachment is obsolete: true
Attachment #625495 - Flags: feedback?(philipp)
Attachment #625245 - Flags: feedback?(philipp)
(Assignee)

Comment 16

5 years ago
Created attachment 625496 [details] [diff] [review]
Part 4: MobileConnection v2

DOMRequest support added
Attachment #625244 - Attachment is obsolete: true
Attachment #625496 - Flags: feedback?(philipp)
Attachment #625244 - Flags: feedback?(philipp)
(Assignee)

Comment 17

5 years ago
Created attachment 625497 [details] [diff] [review]
Part 5: DOMRequestHelper

This patch is probably not going to land with the others as it was taken from Bug 731786. I´ve only added here to let you know that I have the intention of using that part of code. When bug 731786 lands, I´ll need to rebase my patches.
(Assignee)

Updated

5 years ago
Attachment #625242 - Flags: feedback?(bent.mozilla)
(Assignee)

Updated

5 years ago
Attachment #625494 - Flags: feedback?(bent.mozilla)
(Assignee)

Updated

5 years ago
Attachment #625496 - Flags: feedback?(bent.mozilla)
(Assignee)

Comment 18

5 years ago
I´ve successfully tested the DOMRequest part with a modification of Gaia dialer app. 
I am also asking Ben for feedback for the DOM related parts.
(Assignee)

Updated

5 years ago
Attachment #625494 - Attachment is obsolete: true
Attachment #625494 - Flags: feedback?(philipp)
Attachment #625494 - Flags: feedback?(bent.mozilla)
(Assignee)

Updated

5 years ago
Attachment #625495 - Attachment is obsolete: true
Attachment #625495 - Flags: feedback?(philipp)
(Assignee)

Updated

5 years ago
Attachment #625497 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #625496 - Attachment is obsolete: true
Attachment #625496 - Flags: feedback?(philipp)
Attachment #625496 - Flags: feedback?(bent.mozilla)
(Assignee)

Comment 19

5 years ago
Bug 731786 has already landed in m-c, so I am marking my previous patches as obsolete and rebasing it. I´ll be uploading new patches soon.
Comment on attachment 625494 [details] [diff] [review]
Part 1: IDLs v2

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

Yeah it needs to be rebased, but that's trivial. r+
Attachment #625494 - Flags: review+
Comment on attachment 625495 [details] [diff] [review]
Part 2: RIL v3

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

Needs rebasing like you said and a few nits below.

::: dom/system/gonk/RILContentHelper.js
@@ +117,5 @@
>  
> +  sendUSSD: function sendUSSD(window, ussd) {
> +    debug("Sending USSD " + ussd);
> +    if (!window) {
> +      throw Components.Exception("Can´t get window object",

Use apostrophe ('), not inverse backtick (´)

@@ +129,5 @@
> +
> +  cancelUSSD: function cancelUSSD(window) {
> +    debug("Cancel USSD");
> +    if (!window) {
> +      throw Components.Exception("Can´t get window object",

Ditto

@@ +279,5 @@
> +                                     msg.json.message);
> +        break;
> +      case "RIL:SendUssdOK":
> +      case "RIL:CancelUssdOK":
> +        request = this.getRequest(msg.json.requestId);

s/getRequest/takeRequest/! Otherwise you leak the request.

@@ +286,5 @@
> +        }
> +        break;
> +      case "RIL:SendUssdKO":
> +      case "RIL:CancelUssdKO":
> +        request = this.getRequest(msg.json.requestId);

Ditto.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +694,5 @@
> +  },
> +
> +  handleSendUSSD: function handleSendUSSD(message) {
> +    debug("handleSendUSSD " + JSON.stringify(message));
> +    let messageType = message.success ? "RIL:SendUssdOK" : "RIL:SendUssdKO";

Please add a colon before OK and KO. Or, just do the error checking in the content process?

@@ +700,5 @@
> +  },
> +
> +  handleCancelUSSD: function handleCancelUSSD(message) {
> +    debug("handleCancelUSSD " + JSON.stringify(message));
> +    let messageType = message.success ? "RIL:CancelUssdOK" : "RIL:CancelUssdKO";

Ditto.

::: dom/system/gonk/ril_worker.js
@@ +2560,5 @@
> +  }
> +  this.sendDOMMessage({type: "sendussd",
> +                       success: options.rilRequestError == 0 ? true : false,
> +                       errorMsg: options.errorMsg,
> +                       requestId: options.requestId});

Just reuse the `options` object:

  options.success = options.rilRequestError == 0 ? true : false;
  this.sendDOMMessage(options);

(`options.type` will be "sendUSSD".)

@@ +2569,5 @@
> +  }
> +  this.sendDOMMessage({type: "cancelussd",
> +                       success: options.rilRequestError == 0 ? true : false,
> +                       errorMsg: options.errorMsg,
> +                       requestId: options.requestId});

Ditto.
Attachment #625495 - Flags: feedback+
Comment on attachment 625242 [details] [diff] [review]
Part 3: DOMEvent v1

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

I'd r+, but I'm not a DOM peer.
Attachment #625242 - Flags: review?(bent.mozilla)
Attachment #625242 - Flags: feedback?(philipp)
Attachment #625242 - Flags: feedback?(bent.mozilla)
Attachment #625242 - Flags: feedback+
Comment on attachment 625496 [details] [diff] [review]
Part 4: MobileConnection v2

I don't think this particular patch is obsolete and it looks good to me, although I'm not a DOM peer, so I'm flinging the review over to bent.

>--- a/dom/network/src/MobileConnection.h
>+++ b/dom/network/src/MobileConnection.h
>@@ -32,20 +32,28 @@ public:
>   void Shutdown();
> 
>   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(MobileConnection,
>                                            nsDOMEventTargetHelper)
> 
> private:
>   nsCOMPtr<nsIMobileConnectionProvider> mProvider;
> 
>+  nsIDOMEventTarget*
>+  ToIDOMEventTarget() const
>+  {
>+    return static_cast<nsDOMEventTargetHelper*>(
>+           const_cast<MobileConnection*>(this));
>+  }

I'm assuming there's a reason why you had to add this? I'm curious to know what that is. :)
Attachment #625496 - Attachment is obsolete: false
Attachment #625496 - Flags: review?(bent.mozilla)
Attachment #625496 - Flags: feedback+
Comment on attachment 625242 [details] [diff] [review]
Part 3: DOMEvent v1

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

::: dom/network/src/USSDReceivedEvent.cpp
@@ +32,5 @@
> +  NS_INTERFACE_MAP_ENTRY(nsIDOMUSSDReceivedEvent)
> +  NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(USSDReceivedEvent)
> +NS_INTERFACE_MAP_END_INHERITING(nsDOMEvent)
> +
> +

Nit: extra newline here

::: dom/network/src/USSDReceivedEvent.h
@@ +6,5 @@
> +#define mozilla_dom_network_ussdreceivedevent_h
> +
> +#include "nsIDOMUSSDReceivedEvent.h"
> +#include "nsDOMEvent.h"
> +#include "nsIDOMEventTarget.h"

Nit: Not sure, but it doesn't look like this is needed.

@@ +48,5 @@
> +    return NS_OK;
> +  }
> +
> +private:
> +  USSDReceivedEvent()

You should do the destructor too.
Attachment #625242 - Flags: review?(bent.mozilla) → review+
Comment on attachment 625496 [details] [diff] [review]
Part 4: MobileConnection v2

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

::: dom/network/src/MobileConnection.cpp
@@ +123,5 @@
>  
> +  if (!strcmp(aTopic, kUssdReceivedTopic)) {
> +    nsString ussd;
> +    ussd.Assign(aData);
> +    nsRefPtr<USSDReceivedEvent> event = USSDReceivedEvent::Create(ussd);

Nit: You can do this:

  USSDReceivedEvent::Create(nsDependentString(aData));

@@ +185,5 @@
> +NS_IMETHODIMP
> +MobileConnection::SendUSSD(const nsAString& aUSSDString,
> +                           nsIDOMDOMRequest** request)
> +{
> +  *request = nsnull;

This is unnecessary. XPConnect will not inspect any out-params until you return a success code.

@@ +193,5 @@
> +  }
> +
> +  if (aUSSDString.IsEmpty()) {
> +    NS_WARNING("Empty USSD string will be ignored");
> +    return NS_OK;

Hm... I don't think this is right. It means that a JS caller that passes a bad value won't get a request object returned. I feel like you should either throw an exception (if this is really something that developers should never do) or return a DOMRequest that later gets a success callback if this is supposed to be ok.

@@ +202,5 @@
> +
> +NS_IMETHODIMP
> +MobileConnection::CancelUSSD(nsIDOMDOMRequest** request)
> +{
> +  *request = nsnull;

Remove.
Over the weekend an issue occurred with the BMO database which resulted in duplication of dependencies. The dependency issue may have resulted in "Depends On" and "Blocks" values being removed while updating a bug. This issue should now be resolved, however dependencies may need to be manually restored to some bugs.

This bug had dependencies removed during the failure period and will need verification that the dependency removal(s) were intentional. Please help out by taking a look at this bug and adding anything back that was mistakenly removed.
(In reply to David Lawrence [:dkl] from comment #26)
> Over the weekend an issue occurred with the BMO database which resulted in
> duplication of dependencies. The dependency issue may have resulted in
> "Depends On" and "Blocks" values being removed while updating a bug. This
> issue should now be resolved, however dependencies may need to be manually
> restored to some bugs.
> 
> This bug had dependencies removed during the failure period and will need
> verification that the dependency removal(s) were intentional. Please help
> out by taking a look at this bug and adding anything back that was
> mistakenly removed.

Yup, comment 12 was the culprit. Fixed.
Blocks: 710489
(In reply to ben turner [:bent] from comment #12)
> Just FYI the DOM patches should have a DOM peer look over them. mounir or
> sicking or me, maybe?

Wait, you're *not* a DOM peer?
(Assignee)

Comment 29

5 years ago
Created attachment 627200 [details] [diff] [review]
Part 1: IDLs v3

I´ve rebased the previous IDL part patch and make a few changes only to group functions and events.
(Assignee)

Comment 30

5 years ago
Created attachment 627204 [details] [diff] [review]
Part 2: RIL v4

This patch addresses Philipp's corrections and adds a simple check to avoid progressing empty USSD messages (replies only with TypeCode, like the ones that closes the USSD session) to the DOM.
(Assignee)

Comment 31

5 years ago
Created attachment 627207 [details] [diff] [review]
Part 3: DOMEvent v2

Ben's comments addressed
Attachment #625242 - Attachment is obsolete: true
(Assignee)

Comment 32

5 years ago
Created attachment 627211 [details] [diff] [review]
Part 4: MobileConnection v3

This patch addresses most of Ben's feedback, except the comment regarding nsDependentString, which I couldn´t make it build :\
Attachment #625496 - Attachment is obsolete: true
Attachment #625496 - Flags: review?(bent.mozilla)
There's no reviewer specified for any of these patches. Are they ready to be reviewed?
(Assignee)

Updated

5 years ago
Attachment #627200 - Flags: review?(philipp)
(Assignee)

Updated

5 years ago
Attachment #627204 - Flags: review?(philipp)
(Assignee)

Updated

5 years ago
Attachment #627207 - Flags: review?(bent.mozilla)
(Assignee)

Updated

5 years ago
Attachment #627211 - Flags: review?(bent.mozilla)
Comment on attachment 627200 [details] [diff] [review]
Part 1: IDLs v3

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

::: dom/network/interfaces/nsIMobileConnectionProvider.idl
@@ +19,5 @@
>    readonly attribute nsIDOMMozMobileConnectionInfo voiceConnectionInfo;
>    readonly attribute nsIDOMMozMobileConnectionInfo dataConnectionInfo;
>  
>    nsIDOMDOMRequest getNetworks(in nsIDOMWindow window);
> +  

tsk tsk, trailing whitespace
Attachment #627200 - Flags: review?(philipp) → review+
Comment on attachment 627204 [details] [diff] [review]
Part 2: RIL v4

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

::: dom/system/gonk/ril_worker.js
@@ +2872,5 @@
> +RIL[REQUEST_SEND_USSD] = function REQUEST_SEND_USSD(length, options) {
> +  if (DEBUG) {
> +    debug("REQUEST_SEND_USSD " + JSON.stringify(options)); 
> +  }
> +  options.type = "sendussd";

Seems pointless to me to rewrite this from "sendUSSD" to "sendussd", but whatever.
Attachment #627204 - Flags: review?(philipp) → review+
blocking-basecamp: --- → +
Attachment #627207 - Flags: review?(bent.mozilla) → review+
Attachment #627211 - Flags: review?(bent.mozilla) → review+
Keywords: checkin-needed
ferjm, can you rebase the patches so they apply on the latest m-c? Tjanks!
Keywords: checkin-needed
(Assignee)

Comment 37

5 years ago
Sure! Sorry for the late reply. Rebasing now.
(Assignee)

Comment 38

5 years ago
Created attachment 631539 [details] [diff] [review]
Part 1: IDLs v3

Patch rebased and some extra blank spaces removed
Attachment #627200 - Attachment is obsolete: true
(Assignee)

Comment 39

5 years ago
Created attachment 631540 [details] [diff] [review]
Part 2: RIL v4

Patch rebased
Attachment #627204 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9491fa6ac54
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a82222e6d5b
https://hg.mozilla.org/integration/mozilla-inbound/rev/fed1cc976391
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e8cda8abd8d
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/c9491fa6ac54
https://hg.mozilla.org/mozilla-central/rev/6a82222e6d5b
https://hg.mozilla.org/mozilla-central/rev/fed1cc976391
https://hg.mozilla.org/mozilla-central/rev/8e8cda8abd8d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
/me mumbles yet another way to create events :(
(Assignee)

Updated

5 years ago
Depends on: 763326
blocking-kilimanjaro: --- → +
John, can you please find an owner who could possibly verify this?  Thanks!
Whiteboard: [qa+]
Duplicate of this bug: 773577
You need to log in before you can comment on or make changes to this bug.