[Asterisk-code-review] chan_unistim: Fix code, causing all incoming DTMF sent back to asterisk (...asterisk[16])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/c/asterisk/+/12804 )

Change subject: chan_unistim: Fix code, causing all incoming DTMF sent back to asterisk
......................................................................

chan_unistim: Fix code, causing all incoming DTMF sent back to asterisk

Current implementation of ast_channel_tech send_digit_begin hook uses
same function for tone playback as key press handler. This cause every
incoming dtmf send back to asterisk. In case of two unistim phones
connected to each other, it'll cause indefinite DTMF loop. Fix add
separate function for dtmf tone phone play.

Change-Id: I5795db468df552f0c89c7576b6b3858b26c4eab4
---
M channels/chan_unistim.c
1 file changed, 28 insertions(+), 29 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/channels/chan_unistim.c b/channels/chan_unistim.c
index c1328c1..964d9e1 100644
--- a/channels/chan_unistim.c
+++ b/channels/chan_unistim.c
@@ -3331,22 +3331,12 @@
 	return;
 }
 
-static int unistim_do_senddigit(struct unistimsession *pte, char digit)
+static int send_dtmf_tone(struct unistimsession *pte, char digit)
 {
-	struct ast_frame f = { .frametype = AST_FRAME_DTMF, .subclass.integer = digit, .src = "unistim" };
-	struct unistim_subchannel *sub;
-        int row, col;
+	int row, col;
 
-	sub = get_sub(pte->device, SUB_REAL);
-	if (!sub || !sub->owner || sub->alreadygone) {
-		ast_log(LOG_WARNING, "Unable to find subchannel in dtmf senddigit\n");
-		return -1;
-	}
-
-	/* Send DTMF indication _before_ playing sounds */
-	ast_queue_frame(sub->owner, &f);
 	if (unistimdebug) {
-		ast_verb(0, "Send Digit %c (%i ms)\n", digit, pte->device->dtmfduration);
+		ast_verb(0, "Phone Play Digit %c\n", digit);
 	}
 	if (pte->device->dtmfduration > 0) {
 		row = (digit - '1') % 3;
@@ -3364,6 +3354,28 @@
 		} else {
 			send_tone(pte, 500, 2000);
 		}
+	}
+	return 0;
+}
+
+static int unistim_do_senddigit(struct unistimsession *pte, char digit)
+{
+	struct ast_frame f = { .frametype = AST_FRAME_DTMF, .subclass.integer = digit, .src = "unistim" };
+	struct unistim_subchannel *sub;
+
+	sub = get_sub(pte->device, SUB_REAL);
+	if (!sub || !sub->owner || sub->alreadygone) {
+		ast_log(LOG_WARNING, "Unable to find subchannel in dtmf senddigit\n");
+		return -1;
+	}
+
+	/* Send DTMF indication _before_ playing sounds */
+	ast_queue_frame(sub->owner, &f);
+	if (pte->device->dtmfduration > 0) {
+		if (unistimdebug) {
+			ast_verb(0, "Send Digit %c (%i ms)\n", digit, pte->device->dtmfduration);
+		}
+		send_dtmf_tone(pte, digit);
 		usleep(pte->device->dtmfduration * 1000);	 /* XXX Less than perfect, blocking an important thread is not a good idea */
 		send_tone(pte, 0, 0);
 	}
@@ -5506,34 +5518,21 @@
 	if (!pte) {
 		return -1;
 	}
-	return unistim_do_senddigit(pte, digit);
+	return send_dtmf_tone(pte, digit);
 }
 
 static int unistim_senddigit_end(struct ast_channel *ast, char digit, unsigned int duration)
 {
 	struct unistimsession *pte = channel_to_session(ast);
-	struct ast_frame f = { 0, };
-	struct unistim_subchannel *sub;
 
-	sub = get_sub(pte->device, SUB_REAL);
-
-	if (!sub || !sub->owner || sub->alreadygone) {
-		ast_log(LOG_WARNING, "Unable to find subchannel in dtmf senddigit_end\n");
+	if (!pte) {
 		return -1;
 	}
 
 	if (unistimdebug) {
-		ast_verb(0, "Send Digit off %c\n", digit);
-	}
-	if (!pte) {
-		return -1;
+		ast_verb(0, "Send Digit off %c (duration %d)\n", digit, duration);
 	}
 	send_tone(pte, 0, 0);
-	f.frametype = AST_FRAME_DTMF;
-	f.subclass.integer = digit;
-	f.src = "unistim";
-	ast_queue_frame(sub->owner, &f);
-
 	return 0;
 }
 

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/12804
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I5795db468df552f0c89c7576b6b3858b26c4eab4
Gerrit-Change-Number: 12804
Gerrit-PatchSet: 1
Gerrit-Owner: Igor Goncharovsky <igor.goncharovsky at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190903/bb70b185/attachment.html>