[Asterisk-code-review] app sendtext: Enhance SendText to support Enhanced Messaging (asterisk[13])

[George Joseph]

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/8764 )

Change subject: app_sendtext:  Enhance SendText to support Enhanced Messaging
......................................................................


Patch Set 1:

(8 comments)

https://gerrit.asterisk.org/#/c/8764/1/apps/app_sendtext.c
File apps/app_sendtext.c:

https://gerrit.asterisk.org/#/c/8764/1/apps/app_sendtext.c@54
PS1, Line 54: 			<note><para><literal>current channel</literal> could be the caller or callee depending
            : 			on the context in which this application is called.</para></note>
> This note can be removed.  I think the callee designation before
 > was misleading anyway.  Normally it would be the callee but the
 > features.conf DYNAMIC_FEATURES can execute this app on either
 > channel.

I think you've got it backwards.  It's the caller channel that gets the message by default, at least for chan_pjsip.
If you use a pre dial handler you can make it the callee.


https://gerrit.asterisk.org/#/c/8764/1/apps/app_sendtext.c@68
PS1, Line 68: 				<variable name="SENDTEXT_CONTENT_TYPE">
> Should state the default content type.
 > 
 > I take it the default content type is text/plain and that is set
 > somewhere else if we feed an empty content type string to the
 > channel driver.

Correct.


https://gerrit.asterisk.org/#/c/8764/1/apps/app_sendtext.c@127
PS1, Line 127: SENDTEXT_FROM_CONTENT_TYPE
> SENDTEXT_CONTENT_TYPE
Done


https://gerrit.asterisk.org/#/c/8764/1/apps/app_sendtext.c@131
PS1, Line 131: SENDTEXT_FROM_CONTENT_TYPE
> SENDTEXT_CONTENT_TYPE
Done


https://gerrit.asterisk.org/#/c/8764/1/apps/app_sendtext.c@157
PS1, Line 157: 	if (!data) {
             : 		ast_log(LOG_WARNING, "SendText requires an argument (text)\n");
             : 		return -1;
             : 	}
> This isn't exactly true anymore.  We should always have an empty string bod
Done


https://gerrit.asterisk.org/#/c/8764/1/apps/app_sendtext.c@169
PS1, Line 169: 	from = pbx_builtin_getvar_helper(chan, "SENDTEXT_FROM_DISPLAYNAME");
             : 	to = pbx_builtin_getvar_helper(chan, "SENDTEXT_TO_DISPLAYNAME");
             : 	content_type = pbx_builtin_getvar_helper(chan, "SENDTEXT_CONTENT_TYPE");
             : 	body = pbx_builtin_getvar_helper(chan, "SENDTEXT_BODY");
> Should these be unset after usage? If someone doesn't clear them themselves
Done


https://gerrit.asterisk.org/#/c/8764/1/apps/app_sendtext.c@178
PS1, Line 178: 	msg_type = "NONE";
> This assignment is always overwritten.
Done


https://gerrit.asterisk.org/#/c/8764/1/apps/app_sendtext.c@209
PS1, Line 209: 	} else if(ast_channel_tech(chan)->send_text) {
> spacing
Done



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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: I648b4574478119f95de09d9f08e9595831b02830
Gerrit-Change-Number: 8764
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Wed, 11 Apr 2018 15:32:56 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180411/6313adb2/attachment-0001.html>