Philipp von Weitershausen [:philikon] Reporter
	Description • 13 years ago

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.

	Philipp von Weitershausen [:philikon] Reporter
	Comment 1 • 13 years ago

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 :)

	Kyle Machulis [:qdot] [:kmachulis] (INACTIVE) Assignee
	Comment 2 • 13 years ago

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.

	Kyle Machulis [:qdot] [:kmachulis] (INACTIVE) Assignee
	Comment 3 • 13 years ago

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.

	Philipp von Weitershausen [:philikon] Reporter
	Comment 4 • 13 years ago

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.

	Kyle Machulis [:qdot] [:kmachulis] (INACTIVE) Assignee
	Comment 5 • 13 years ago

nits picked, functions renamed. Landable.

	Philipp von Weitershausen [:philikon] Reporter
	Comment 6 • 13 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/7121c07c3e7f

	Matt Brubeck (:mbrubeck)
	Comment 7 • 13 years ago

https://hg.mozilla.org/mozilla-central/rev/7121c07c3e7f