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

Mark Murawski asteriskteam at digium.com
Fri Jan 13 20:08:25 CST 2023


Attention is currently required from: N A.

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

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


Patch Set 6:

(11 comments)

File channels/chan_dahdi.h:

https://gerrit.asterisk.org/c/asterisk/+/19264/comment/ce7fa489_a97bdd67 
PS5, Line 206:        /*!
> You're still using spaces here, not tabs, and the alignment is off
Done


File channels/chan_dahdi.c:

https://gerrit.asterisk.org/c/asterisk/+/19264/comment/f38b499f_b33636da 
PS2, Line 9788:                res = pbx_exec(chan, theapp, NULL);
> The reason for the answer is to fully 'open the line' in terms of what dahdi is concerned with. […]
As far as I can tell: Answer() is the only way to allocate the line completely cutting it off from use both on incoming calls and on outgoing calls.

This makes sense from a high level perspective as well.  Going off-hook in the typical sense of being directly on the port in question involves opening the circuit... you're literally Answer()ing the line at that point, with nothing on the other end.


File channels/chan_dahdi.c:

https://gerrit.asterisk.org/c/asterisk/+/19264/comment/ec7d2459_b802e79a 
PS5, Line 211:        <manager name="DAHDIBusyOut" language="en_US">
> Spaces should be tabs
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/48967ff1_635c5984 
PS5, Line 225:                        <para>Enable or disable BusyOut on DAHDI channels.</para>
> This is not descriptive enough. […]
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/a801f105_de38df4e 
PS5, Line 227:                </description>
> I don't see the xmldocs for the application, just the manager action. […]
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/b09cc2ad_657146cd 
PS5, Line 5894: 
> And all these spaces need to be converted to tabs too
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/31de6c22_51e495aa 
PS5, Line 9786: 
> I still think this is suspect. […]
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/21c7543d_33af478f 
PS5, Line 9788: 			ast_log(LOG_WARNING, "PBX exited non-zero\n");
> This warning message is not accurate.
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/5d57a3bb_a8b14615 
PS5, Line 11973: 
> This is that blank line you added that should be removed in the change
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/4637b9ce_5c5760b8 
PS5, Line 20289: static void *dahdi_set_channel_busyout_on_thread(void *data);
> Why do you have a forward declaration when the definition is immediately below?
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/279ab644_ce3ae600 
PS5, Line 20443: 	while (*channel_arg_cur);
> The while should be on the same line as the end brace
Done



-- 
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: 6
Gerrit-Owner: Mark Murawski <markm at intellasoft.net>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: N A <asterisk at phreaknet.org>
Gerrit-Attention: N A <asterisk at phreaknet.org>
Gerrit-Comment-Date: Sat, 14 Jan 2023 02:08:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: N A <asterisk at phreaknet.org>
Comment-In-Reply-To: Mark Murawski <markm at intellasoft.net>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20230114/e4ad42fc/attachment-0001.html>


More information about the asterisk-code-review mailing list