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

Mark Murawski asteriskteam at digium.com
Sat Jan 14 19:58:19 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 9:

(7 comments)

File channels/chan_dahdi.c:

https://gerrit.asterisk.org/c/asterisk/+/19264/comment/e84fee64_9bacbbf6 
PS7, Line 55: #include "asterisk.h"
> Empty line after asterisk.h […]
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/7c826606_54e07636 
PS7, Line 16612:        AST_CLI_DEFINE(dahdi_set_busy, "Sets/resets Busyout mode on a channel"),
> whitespace
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/b15ac3dd_55b6d162 
PS7, Line 17934:        ast_unregister_application(dahdi_busyout_app);
> whitespace
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/9f14d38e_0c3e530f 
PS7, Line 17965:        ast_manager_unregister("DAHDIBusyOut");
> whitespace
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/57d8b2d8_6b4bc1c4 
PS7, Line 20159:        ast_manager_register_xml("DAHDIBusyOut", 0, action_dahdibusyout);
> whitespace
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/d83c3fd8_014af7e2 
PS7, Line 20304: 
> Again, I would ask the same question as before. Say you enable this on 20 channels at once? Is it really efficient to have 20 threads up for channels that don't really exist?
> 
> I'm not positive, but I think this could be done a different way. At least, if not, I'd like it explained why not.
> 
> Suppose for simplicity we are setting busy out on idle channels. We set a busyout flag to 1.
> 
> There is a function in chan_dahdi.c that determines if a channel is available (sig_analog too I think). It returns NO for example if the DND flag is set. I think using DND as a model is useful here. I know that only applies to FXS lines, and this should also apply to FXO lines / trunks, etc.
> 
> This covers the subscriber scenario, since requesting a DAHDI channel with busy out will fail.
> 
> I'm positive this is not needed for FXS lines (FXO signalled).
> For an FXO line that we want to appear busy to somebody trying to call us, the physical loop will need to be closed, so I can understand that much. However, I'm not sure if we really need threads tied up for each busy out. You could manually set the hook state in DAHDI and keep track of that. Since busy outs are all cancelled manually I believe, we don't even need to check for that elsewhere.
> 
> I realize this only covers analog, is this supposed to apply to non analog as well?
> 
> Overall, I think spawning threads like this should be avoided if possible.

I'm onboard with making this a not-new-thread operation.  I'll check into making this more inline.  I'm not sure if this works for doing a busyout on say, individual PRI channels, so my starting point was analog-only.


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/4be9449f_75775eea 
PS7, Line 20423: 		return 0;
> Should this return -1? Same above?
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: 9
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: Sun, 15 Jan 2023 01:58:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: N A <asterisk at phreaknet.org>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20230115/e7018aaf/attachment.html>


More information about the asterisk-code-review mailing list