[Asterisk-code-review] app_sendmf: New SendMF application (asterisk[master])

[George Joseph]

Attention is currently required from: N A, Joshua Colp, Sarah Autumn.
George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/16080 )

Change subject: app_sendmf: New SendMF application
......................................................................


Patch Set 5: Code-Review-1

(8 comments)

File apps/app_sendmf.c:

https://gerrit.asterisk.org/c/asterisk/+/16080/comment/e4184fb6_3be07bfe 
PS4, Line 191: static int external_sleep(struct ast_channel *chan, int ms)
             : {
             : 	usleep(ms * 1000);
             : 	return 0;
             : }
> I sort of get where this is going but I think I'm missing where the second part would go instead...

Replace the external_sleep function with this updated implementation...
static int mysleep(struct ast_channel *chan, int ms, int is_external)
{
	return is_external ? usleep(ms * 1000) : ast_safe_sleep(chan, ms);
}

Then see below...


https://gerrit.asterisk.org/c/asterisk/+/16080/comment/675e55f3_b129b985 
PS4, Line 257: static int mf_stream_digits(struct ast_channel *chan, struct ast_channel *peer, const char *digits, int between, unsigned int duration, unsigned int durationkp, unsigned int durationst)
             : {
             : 	int res;
             : 
             : 	if (peer && ast_autoservice_start(peer)) {
             : 		return -1;
             : 	}
             : 	res = mf_stream(chan, digits, between, duration, durationkp, durationst, 0);
             : 	if (peer && ast_autoservice_stop(peer)) {
             : 		res = -1;
             : 	}
             : 
             : 	return res;
             : }
> Additionally, before we go too far along here, it's on my list to add a Dial option that would send  […]
I'd cross that bridge when you come to it. 😊


File apps/app_sendmf.c:

https://gerrit.asterisk.org/c/asterisk/+/16080/comment/9bcee25f_79cca154 
PS5, Line 91: 			<parameter name="Receive" required="false">
            : 				<para>Emulate receiving MF on this channel instead of sending it out.</para>
            : 			</parameter>
            : 
Need to remove.


https://gerrit.asterisk.org/c/asterisk/+/16080/comment/904b6a32_6551bfb2 
PS5, Line 129: 	if (digit >= '0' && digit <='9')
             : 		ast_playtones_start(chan, 0, mf_tones[digit-'0'], 0);
             : 	else if (digit == '*')
             : 		ast_playtones_start(chan, 0, mf_tones[10], 0);
             : 	else if (digit == '#')
             : 		ast_playtones_start(chan, 0, mf_tones[11], 0);
             : 	else if (digit == 'A')
             : 		ast_playtones_start(chan, 0, mf_tones[12], 0);
             : 	else if (digit == 'B')
             : 		ast_playtones_start(chan, 0, mf_tones[13], 0);
             : 	else if (digit == 'C')
             : 		ast_playtones_start(chan, 0, mf_tones[14], 0);
Older code doesn't do this but we now require if statements to always use {}.


https://gerrit.asterisk.org/c/asterisk/+/16080/comment/ceae4465_14783826 
PS5, Line 176: 		if (is_external)
             : 			usleep(duration * 1000);
             : 		else
             : 			ast_safe_sleep(chan, duration);
Replace with mysleep(chan, duration, is_external);


https://gerrit.asterisk.org/c/asterisk/+/16080/comment/5d4b3d7b_78a2fa2c 
PS5, Line 190: 	int (*my_sleep)(struct ast_channel *chan, int ms);
             : 
             : 	if (is_external) {
             : 		my_sleep = external_sleep;
             : 	} else {
             : 		my_sleep = ast_safe_sleep;
             : 	}
             : 
remove


https://gerrit.asterisk.org/c/asterisk/+/16080/comment/cd8bb57b_93667a44 
PS5, Line 224: res = my_sleep(chan, between);
replace with res = mysleep(chan, between, is_external);


https://gerrit.asterisk.org/c/asterisk/+/16080/comment/5d8f199a_425e9f0a 
PS5, Line 335: !ast_strlen_zero(duration) && (sscanf(duration, "%30u", &duration_ms) != 1
Isn't this going to override the overrides for '*' and '#'?



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I5d89afdbccee3f86cc702ed96d882f3d351327a4
Gerrit-Change-Number: 16080
Gerrit-PatchSet: 5
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-CC: Sarah Autumn <sarah at endlesstemple.org>
Gerrit-Attention: N A <mail at interlinked.x10host.com>
Gerrit-Attention: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Sarah Autumn <sarah at endlesstemple.org>
Gerrit-Comment-Date: Fri, 06 Aug 2021 14:17:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: N A <mail at interlinked.x10host.com>
Comment-In-Reply-To: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210806/160f0506/attachment-0001.html>