[Asterisk-code-review] app_mf: Add channel agnostic MF sender (asterisk[master])
N A
asteriskteam at digium.com
Sun Aug 8 15:17:49 CDT 2021
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_mf: Add channel agnostic MF sender
......................................................................
Patch Set 8:
(11 comments)
Patchset:
PS6:
> Oh man. We'd really prefer you NOT add functionality to reviews this late. […]
Sorry about that, it seemed like it might make sense to combine some things into one module.
Here is the old code with the changes indicated, though I do plan to do as you said and submit another review to refactor and add other components. The following will be added in a future review, as a heads up:
- refactor the actual sending functions into app.c and channel.c so they are publicly callable
- add MF digit sending to Dial D option
- add ReceiveMF application
ReceiveSF and SendSF may also be added, but that depends on another review which I have yet to push as a dependency so that part may be delayed some while.
File apps/app_sendmf.c:
https://gerrit.asterisk.org/c/asterisk/+/16080/comment/0f97d836_167e29f2
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.. […]
Done
https://gerrit.asterisk.org/c/asterisk/+/16080/comment/7287f70d_63b80bb5
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;
: }
> By "on my list", I am working on that right now, actively writing code for it. […]
Ack
https://gerrit.asterisk.org/c/asterisk/+/16080/comment/28b24240_e9465c80
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;
: }
> By "on my list", I am working on that right now, actively writing code for it. […]
Ack
File apps/app_sendmf.c:
https://gerrit.asterisk.org/c/asterisk/+/16080/comment/c22c5d14_b98d2490
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.
Done
https://gerrit.asterisk.org/c/asterisk/+/16080/comment/bbadb668_3c554c92
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 {}.
Done
https://gerrit.asterisk.org/c/asterisk/+/16080/comment/cfa40272_fdb10ffb
PS5, Line 176: if (is_external)
: usleep(duration * 1000);
: else
: ast_safe_sleep(chan, duration);
> Replace with mysleep(chan, duration, is_external);
Done
https://gerrit.asterisk.org/c/asterisk/+/16080/comment/7cd44619_0c745234
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
Done
https://gerrit.asterisk.org/c/asterisk/+/16080/comment/223113f0_08bf899a
PS5, Line 224: res = my_sleep(chan, between);
> replace with res = mysleep(chan, between, is_external);
Done
https://gerrit.asterisk.org/c/asterisk/+/16080/comment/d31b274e_eec3660b
PS5, Line 335: !ast_strlen_zero(duration) && (sscanf(duration, "%30u", &duration_ms) != 1
> Yes, wouldn't that be the right way, since we are expecting the user to pass in the right duration f […]
Ack
https://gerrit.asterisk.org/c/asterisk/+/16080/comment/dfe6952e_01791bdd
PS5, Line 335: !ast_strlen_zero(duration) && (sscanf(duration, "%30u", &duration_ms) != 1
> Yes, wouldn't that be the right way, since we are expecting the user to pass in the right duration f […]
Ack
--
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: 8
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: Sun, 08 Aug 2021 20:17:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
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/20210808/a112e915/attachment-0001.html>
More information about the asterisk-code-review
mailing list