Closed
Bug 734145
Opened 12 years ago
Closed 12 years ago
B2G RIL: Support USSD codes
Categories
(Core :: DOM: Device Interfaces, defect, P1)
Core
DOM: Device Interfaces
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: mwu, Assigned: ferjm)
References
Details
(Whiteboard: [qa+])
Attachments
(4 files, 12 obsolete files)
6.62 KB,
patch
|
bent.mozilla
:
review+
|
Details | Diff | Splinter Review |
6.58 KB,
patch
|
bent.mozilla
:
review+
|
Details | Diff | Splinter Review |
7.21 KB,
patch
|
Details | Diff | Splinter Review | |
12.33 KB,
patch
|
Details | Diff | Splinter Review |
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.
Comment 1•12 years ago
|
||
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•12 years ago
|
Assignee: nobody → ferjmoreno
Assignee | ||
Comment 2•12 years ago
|
||
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 3•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
Attachment #622555 -
Attachment is obsolete: false
Assignee | ||
Updated•12 years ago
|
Attachment #622555 -
Flags: feedback+ → feedback?(jonas)
Attachment #622555 -
Flags: feedback?(jonas) → feedback+
Comment 5•12 years ago
|
||
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•12 years ago
|
Priority: -- → P1
Assignee | ||
Comment 6•12 years ago
|
||
Added nsIDOMUSSDReceivedEvent.idl and some additions to nsIMobileConnectionProvider.idl.
Attachment #622555 -
Attachment is obsolete: true
Attachment #625240 -
Flags: feedback?(philipp)
Assignee | ||
Comment 7•12 years ago
|
||
I am specially interested on your feedback about the addition of this event.
Attachment #625242 -
Flags: feedback?(philipp)
Assignee | ||
Comment 8•12 years ago
|
||
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•12 years ago
|
||
Attachment #623021 -
Attachment is obsolete: true
Attachment #625245 -
Flags: feedback?(philipp)
Assignee | ||
Comment 10•12 years ago
|
||
All this patches have been successfully tested with Gaia. Both USSD types, terminal initiated and network initiated works.
Assignee | ||
Comment 11•12 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: b2g-ril
Assignee | ||
Comment 13•12 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•12 years ago
|
||
Minor change on comment
Attachment #625494 -
Flags: feedback?(philipp)
Assignee | ||
Updated•12 years ago
|
Attachment #625240 -
Attachment is obsolete: true
Attachment #625240 -
Flags: feedback?(philipp)
Assignee | ||
Comment 15•12 years ago
|
||
DOMRequest support added
Attachment #625245 -
Attachment is obsolete: true
Attachment #625495 -
Flags: feedback?(philipp)
Attachment #625245 -
Flags: feedback?(philipp)
Assignee | ||
Comment 16•12 years ago
|
||
DOMRequest support added
Attachment #625244 -
Attachment is obsolete: true
Attachment #625496 -
Flags: feedback?(philipp)
Attachment #625244 -
Flags: feedback?(philipp)
Assignee | ||
Comment 17•12 years ago
|
||
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•12 years ago
|
Attachment #625242 -
Flags: feedback?(bent.mozilla)
Assignee | ||
Updated•12 years ago
|
Attachment #625494 -
Flags: feedback?(bent.mozilla)
Assignee | ||
Updated•12 years ago
|
Attachment #625496 -
Flags: feedback?(bent.mozilla)
Assignee | ||
Comment 18•12 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•12 years ago
|
Attachment #625494 -
Attachment is obsolete: true
Attachment #625494 -
Flags: feedback?(philipp)
Attachment #625494 -
Flags: feedback?(bent.mozilla)
Assignee | ||
Updated•12 years ago
|
Attachment #625495 -
Attachment is obsolete: true
Attachment #625495 -
Flags: feedback?(philipp)
Assignee | ||
Updated•12 years ago
|
Attachment #625497 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #625496 -
Attachment is obsolete: true
Attachment #625496 -
Flags: feedback?(philipp)
Attachment #625496 -
Flags: feedback?(bent.mozilla)
Assignee | ||
Comment 19•12 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 20•12 years ago
|
||
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 21•12 years ago
|
||
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 22•12 years ago
|
||
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 23•12 years ago
|
||
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.
Comment 26•12 years ago
|
||
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.
Comment 27•12 years ago
|
||
(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: b2g-ril
Comment 28•12 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? Wait, you're *not* a DOM peer?
Assignee | ||
Comment 29•12 years ago
|
||
I´ve rebased the previous IDL part patch and make a few changes only to group functions and events.
Assignee | ||
Comment 30•12 years ago
|
||
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•12 years ago
|
||
Ben's comments addressed
Attachment #625242 -
Attachment is obsolete: true
Assignee | ||
Comment 32•12 years ago
|
||
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)
Comment 33•12 years ago
|
||
There's no reviewer specified for any of these patches. Are they ready to be reviewed?
Assignee | ||
Updated•12 years ago
|
Attachment #627200 -
Flags: review?(philipp)
Assignee | ||
Updated•12 years ago
|
Attachment #627204 -
Flags: review?(philipp)
Assignee | ||
Updated•12 years ago
|
Attachment #627207 -
Flags: review?(bent.mozilla)
Assignee | ||
Updated•12 years ago
|
Attachment #627211 -
Flags: review?(bent.mozilla)
Comment 34•12 years ago
|
||
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 35•12 years ago
|
||
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+
Updated•12 years ago
|
blocking-basecamp: --- → +
Updated•12 years ago
|
Attachment #627207 -
Flags: review?(bent.mozilla) → review+
Updated•12 years ago
|
Attachment #627211 -
Flags: review?(bent.mozilla) → review+
Updated•12 years ago
|
Keywords: checkin-needed
Comment 36•12 years ago
|
||
ferjm, can you rebase the patches so they apply on the latest m-c? Tjanks!
Keywords: checkin-needed
Assignee | ||
Comment 37•12 years ago
|
||
Sure! Sorry for the late reply. Rebasing now.
Assignee | ||
Comment 38•12 years ago
|
||
Patch rebased and some extra blank spaces removed
Attachment #627200 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 40•12 years ago
|
||
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
Comment 41•12 years ago
|
||
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
Closed: 12 years ago
Resolution: --- → FIXED
Comment 42•12 years ago
|
||
/me mumbles yet another way to create events :(
Updated•12 years ago
|
blocking-kilimanjaro: --- → +
John, can you please find an owner who could possibly verify this? Thanks!
Whiteboard: [qa+]
You need to log in
before you can comment on or make changes to this bug.
Description
•