[Asterisk-code-review] app dial: Add the "Q" option to set the cause on unanswered... (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: app_dial:  Add the "Q" option to set the cause on unanswered channels
......................................................................


Patch Set 2: Code-Review-1

(5 comments)

https://gerrit.asterisk.org/#/c/4033/2/apps/app_dial.c
File apps/app_dial.c:

PS2, Line 389: 						to send no cause.  See the <filename>causes.h</filename> file for the
             : 						full list of valid causes and names.
Asking someone to look in the code (causes.h) isn't a good thing.  However, there isn't a very good place to get the string equivalents for the numeric values other than from the causes array in channel.c.


Line 538: 			<example title="Dial alice and bob and send NO_ANSWER to bob instead of ANSWERED_ELSEWHRE when alice answers">
ANSWERED_ELSEWHERE typo


PS2, Line 2802: 				if (!strcasecmp(opt_args[OPT_ARG_HANGUPCAUSE], "NONE")) {
              : 					cause = 0;
Have you tested what "NONE"/0 is going to do?  It is likely going to result in a normal-clearing/16 cause.


Line 2808: 					cause = 0;
Probably should set cause to -1 since they didn't supply a valid substitute cause.


Line 3221: 		? AST_CAUSE_ANSWERED_ELSEWHERE : -1 );
Still have the extra space before )


-- 
To view, visit https://gerrit.asterisk.org/4033
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I71742e0919aaa16784c30a2b2e73fbeed7672e47
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes