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

[Mark Murawski]

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.c:

https://gerrit.asterisk.org/c/asterisk/+/19264/comment/a10b1645_d16b2a6f 
PS6, Line 132: #include "regex.h"
> Is this supposed to be regex.h or <regex.h>? […]
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/0e2b514c_8c91997e 
PS6, Line 5934: 	  AST_APP_ARG(channel);
> whitespacing is messed up here, and throughout this entire function
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/51f380cb_c2ba8486 
PS6, Line 8715:         if (p->busyout) {
> whitespacing
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/f86a4a01_c8aa43e2 
PS6, Line 9820: 		ast_answer(chan);
> If you're really going to answer, you should use ast_raw_answer instead.
> Have you confirmed this does not work without calling ast_answer? And can you explain why we are answering, as a code comment?

This does not work without answer()'ing.  You can set DAHDI_OFFHOOK, but the rest of the dahdi ecosystem still considers it available to use.  It makes sense from a high level as well, you need to 'open the circuit' to completely go off-hook.


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/5c632a57_295a732d 
PS6, Line 11855:                        }
> whitespacing
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/6a240bf4_82cc1223 
PS6, Line 15858: 		return 1;
> Again, I think returning 1 closes the manager session, this function should always return 0
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/40541f60_348e8ed7 
PS6, Line 15880: 	       }
> whitespacing in this function
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/edb25729_39713b4e 
PS6, Line 16561: 
> whitespacing
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/d4dbd06e_2f5762d0 
PS6, Line 20335: 
> whitespacing
Done


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/70e1f9dd_895a9eaf 
PS6, Line 20362:                 usleep(1000);
> I wonder if a thread to do this is the most efficient way.
> Can you not hook into one of the existing DAHDI event loops?

The thread was the most reasonable thing I could come up with at the time this was written.

What kind of docs are available on event handlers?  I see there's this thing dahdi_r2_event_iface defined, but it doesn't look like it's used anywhere in the asterisk mainline source.


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/f8d700bf_705b32f6 
PS6, Line 20451: 		}
> else should be on same line
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 20:36:14 +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/20230114/2951a945/attachment-0001.html>