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

[N A]

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

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


Patch Set 5:

(6 comments)

File apps/app_sendmf.c:

https://gerrit.asterisk.org/c/asterisk/+/16080/comment/2559fc73_bc27ef2f 
PS4, Line 91: 			<parameter name="Receive" required="false">
            : 				<para>Emulate receiving MF on this channel instead of sending it out.</para>
            : 			</parameter>
> You didn't implement this (which is fine) but don't you want to add the interval, KP and ST duration […]
Done


https://gerrit.asterisk.org/c/asterisk/+/16080/comment/bae1a13e_0387bc83 
PS4, Line 158: static int senddigit_mf(struct ast_channel *chan, char digit, unsigned int duration, unsigned int durationkp, unsigned int durationst)
             : {
             : 	if (duration < DEFAULT_EMULATE_MF_DURATION) {
             : 		duration = DEFAULT_EMULATE_MF_DURATION;
             : 	}
             : 	if (ast_channel_tech(chan)->send_digit_begin) {
             : 		if (digit == '*')
             : 			duration = durationkp;
             : 		else if (digit == '#' || digit == 'A' || digit == 'B' || digit == 'C')
             : 			duration = durationst;
             : 		senddigit_mf_begin(chan, digit);
             : 		ast_safe_sleep(chan, duration);
             : 	}
             : 	return senddigit_mf_end(chan);
             : }
             : 
             : static int senddigit_external_mf(struct ast_channel *chan, char digit, unsigned int duration,
             : 	unsigned int durationkp, unsigned int durationst)
             : {
             : 	if (duration < DEFAULT_EMULATE_MF_DURATION) {
             : 		duration = DEFAULT_EMULATE_MF_DURATION;
             : 	}
             : 	if (ast_channel_tech(chan)->send_digit_begin) {
             : 		senddigit_mf_begin(chan, digit);
             : 		if (digit == '*')
             : 			duration = durationkp;
             : 		else if (digit == '#' || digit == 'A' || digit == 'B' || digit == 'C')
             : 			duration = durationst;
             : 		usleep(duration * 1000);
             : 	}
             : 	return senddigit_mf_end(chan);
             : }
             : 
> Since these are private (unlike their DTMF counterparts) and the only difference between them is the […]
Done


https://gerrit.asterisk.org/c/asterisk/+/16080/comment/68cee8f3_c3dd4243 
PS4, Line 191: static int external_sleep(struct ast_channel *chan, int ms)
             : {
             : 	usleep(ms * 1000);
             : 	return 0;
             : }
> If you move this above senddigit and add the is_internal flag, you can collapse the two senddigit fu […]
I sort of get where this is going but I think I'm missing where the second part would go instead...


https://gerrit.asterisk.org/c/asterisk/+/16080/comment/cb6324de_005b3b8b 
PS4, Line 204: 	int (*my_senddigit)(struct ast_channel *chan, char digit, unsigned int duration, unsigned int durationkp, unsigned int durationst);
> You don't need the function pointer stuff if you do the above.
Done


https://gerrit.asterisk.org/c/asterisk/+/16080/comment/54d9098d_3031bc14 
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;
             : }
> Again, since this is private, just collapse with mf_stream.
Done


https://gerrit.asterisk.org/c/asterisk/+/16080/comment/360e316a_21988c5c 
PS4, Line 338: 	const char *duration = astman_get_header(m, "Duration");
> Don't you want to add the interval and other durations?  I know SendDTMF didn't add interval to the  […]
I added the other durations for defaults.
Timeout interval I don't think makes sense to add, since the manager action is for one digit at a time.



-- 
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: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Sarah Autumn <sarah at endlesstemple.org>
Gerrit-Attention: George Joseph <gjoseph at digium.com>
Gerrit-Comment-Date: Wed, 04 Aug 2021 15:50:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
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/20210804/906f06ca/attachment.html>