The default bug view has changed. See this FAQ.

B2G telephony: implement DTMF

RESOLVED FIXED in mozilla11

Status

()

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

People

(Reporter: philikon, Assigned: qdot)

Tracking

Trunk
mozilla11
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][lang=js][mentor=philikon])

Attachments

(1 attachment, 2 obsolete attachments)

This should be pretty easy:

* DOM message & parcel handling in ril_worker.js
* extending nsITelephone and its implementation, nsTelephonyWorker
* implement relevant WebTelephony API in our current throw-away DOM implementation, Telephony.js

If anybody wants to tackle this, poke me and I'll happily provide some guidance.
Oh, in case anybody is wondering what DTMF is: these are the tones the keypad makes when you're in a call. Very useful for operating, say, the Mozilla conference bridge :)

Updated

5 years ago
Whiteboard: [good first bug][lang=js] → [good first bug][lang=js][mentor=philikon]
(Assignee)

Updated

5 years ago
Assignee: nobody → kyle
(Reporter)

Updated

5 years ago
Component: DOM → DOM: Device Interfaces
QA Contact: general → device-interfaces
Version: unspecified → Trunk
(Assignee)

Comment 2

5 years ago
Created attachment 582403 [details] [diff] [review]
First pass for B2G DTMF

First pass patch for DTMF. Tested via b2g-js-ril, RIL level functionality works. Still needs to line up with proposed telephony DOM api, doesn't actually work with DOM test yet.
Attachment #582403 - Flags: review?(philipp)
(Assignee)

Comment 3

5 years ago
Created attachment 582418 [details] [diff] [review]
DTMF addition to RIL and DOM for B2G

Implements startTone and stopTone functions for the Telephony API. sendTones not yet implemented because I'm not quite sure how the timing would work, but that can probably be punted.
Attachment #582403 - Attachment is obsolete: true
Attachment #582403 - Flags: review?(philipp)
Attachment #582418 - Flags: review?(philipp)
Comment on attachment 582418 [details] [diff] [review]
DTMF addition to RIL and DOM for B2G

>@@ -356,6 +356,14 @@ Telephony.prototype = {
>     return call;
>   },
> 
>+  startTone: function startTone(dtmfChar) {
>+	this.telephone.startTone(dtmfChar);
>+  },
>+
>+  stopTone: function stopTone() {
>+	this.telephone.stopTone();
>+  },

Nit: two space indentation, not four.

(you might want to adjust your default values for js2-mode.)

>@@ -685,6 +685,30 @@ let RIL = {
>   },
> 
>   /**
>+   * Start a DTMF Tone.
>+   *
>+   * @param dtmfChar
>+   *        DTMF signal to send, 0-9, *, +
>+   */
>+
>+  startDTMF: function startDTMF(dtmfChar) {
>+	Buf.newParcel(REQUEST_DTMF_START);
>+	Buf.writeString(dtmfChar);
>+	Buf.sendParcel();
>+  },
>+
>+  stopDTMF: function startDTMF() {
>+	Buf.newParcel(REQUEST_DTMF_STOP);
>+	Buf.sendParcel();
>+  },

Buf.simpleRequest(REQUEST_DTMF_STOP);

>+  sendDTMF: function sendDTMF(dtmfChar) {
>+	Buf.newParcel(REQUEST_DTMF);
>+	Buf.writeString(dtmfChar);
>+	Buf.sendParcel();
>+  },

Nit: indentation.

>@@ -832,7 +856,9 @@ RIL[REQUEST_OPERATOR] = function REQUEST_OPERATOR(length) {
>   Phone.onOperator(operator);
> };
> RIL[REQUEST_RADIO_POWER] = null;
>-RIL[REQUEST_DTMF] = null;
>+RIL[REQUEST_DTMF] = function REQUEST_DTMF() {
>+  Phone.onDTMFSend();
>+};
...
>+RIL[REQUEST_DTMF_START] = function REQUEST_DTMF_START() {
>+  Phone.onDTMFStart();
>+};
>+RIL[REQUEST_DTMF_STOP] = function REQUEST_DTMF_STOP() {
>+  Phone.onDTMFStop();
>+};
...
>@@ -1270,11 +1300,19 @@ let Phone = {
>   onSetMute: function onSetMute() {
>   },
> 
>+  onDTMFSend: function onDTMFSend() {
>+  },
>+
>+  onDTMFStart: function onDTMFStart() {
>+  },
>+
>+  onDTMFStop: function onDTMFStop() {
>+  },

For now the convention has been that if RIL.fooBar() method, the response will call Phone.onFooBar(). Your RIL methods above are called {start|stop|send}DTMF(), so I would prefer if these were called on{Start|Stop|Send}DTMF() as well.

>@@ -1310,6 +1348,33 @@ let Phone = {
>   },
> 
>   /**
>+   * Send DTMF Tone
>+   *
>+   * @param dtmfChar
>+   *        String containing the DTMF signal to send.
>+   */
>+  sendTone: function sendTone(options) {
>+    RIL.sendDTMF(options.dtmfChar);
>+  },
>+
>+  /**
>+   * Start DTMF Tone
>+   *
>+   * @param dtmfChar
>+   *        String containing the DTMF signal to send.
>+   */
>+  startTone: function startTone(options) {
>+    RIL.startDTMF(options.dtmfChar);
>+  },
>+
>+  /**
>+   * Stop DTMF Tone
>+   */
>+  stopTone: function stopTone() {
>+    RIL.stopDTMF();
>+  },
>+
>+  /**

Up until now, the methods on Phone mirror the names of the corresponding methods on RIL. I see no reason to break this convention. Makes it easier to remember how stuff is wired up.

Rest looks good! r=me with nits + conventions addressed.
Attachment #582418 - Flags: review?(philipp) → review+
(Assignee)

Comment 5

5 years ago
Created attachment 582440 [details] [diff] [review]
Final version of DTMF addition for B2G RIL

nits picked, functions renamed. Landable.
Attachment #582418 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/7121c07c3e7f
https://hg.mozilla.org/mozilla-central/rev/7121c07c3e7f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.