[Asterisk-code-review] chan_dahdi: Add DAHDIBusyOut (asterisk[master])

N A asteriskteam at digium.com
Thu Sep 15 18:03:55 CDT 2022


Attention is currently required from: Mark Murawski.

N A has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/19264 )

Change subject: chan_dahdi: Add DAHDIBusyOut
......................................................................


Patch Set 2: Code-Review-1

(9 comments)

Patchset:

PS2: 
As a general comment, your spacing is all off. Whitespaces should be converted to tabs, and alignment needs to be fixed.

Also, don't submit against master  directly, it should be another branch, and this needs a JIRA issue. Also needs a CHANGES entry.


File channels/chan_dahdi.h:

https://gerrit.asterisk.org/c/asterisk/+/19264/comment/35f72ab6_77e82ec5 
PS1, Line 211:        unsigned int _busydetect:1;
I think this variable should be renamed, and the description updated appropriately. Unclear right now.


File channels/chan_dahdi.c:

https://gerrit.asterisk.org/c/asterisk/+/19264/comment/03d29899_0c104261 
PS2, Line 5920: 
This isn't actually implemented yet


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/b74dd3f2_56dda567 
PS2, Line 8682:                return &p->subs[idx].f;;
Duplicate semicolon


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/e7c254e6_43aa74fc 
PS2, Line 9788:                res = pbx_exec(chan, theapp, NULL);
Two things:
1) use ast_answer_chan or ast_raw_answer or some C API directly, there is no reason to find the Answer app and execute it.
2) Why are we answering the channel at all??? Seems like a big no no to me.


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/b024e4cc_f3f37ed4 
PS2, Line 9791:                        ast_log(LOG_WARNING, "PBX exited non-zero\n");
Yeah, and we don't need this either, just call the right ast_ API directly.


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/31d2958f_449f334b 
PS2, Line 11976: 
remove


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/6764e864_874c7fa8 
PS2, Line 20378: //     ast_log(LOG_NOTICE, "channels: %s\n", channel_arg);
Remove these commented out logs


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/4cc837f2_b7a0f944 
PS2, Line 20540:                ast_log(LOG_WARNING, "Channel DAHDI/%d busyout queue cancelled\n", dahdi_chan->channel);
why is this a warning?



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Ib75c87c554bb18b8f65be73b4474474e7e1877b7
Gerrit-Change-Number: 19264
Gerrit-PatchSet: 2
Gerrit-Owner: Mark Murawski <markm at intellasoft.net>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: N A <mail at interlinked.x10host.com>
Gerrit-Attention: Mark Murawski <markm at intellasoft.net>
Gerrit-Comment-Date: Thu, 15 Sep 2022 23:03:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220915/07772b5c/attachment.html>


More information about the asterisk-code-review mailing list