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

[Richard Mudgett]

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

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


Patch Set 1: Code-Review-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.


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.


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


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


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 body as the default body no matter which place we get the body from.


https://gerrit.asterisk.org/#/c/8764/1/apps/app_sendtext.c@172
PS1, Line 172: 	body = pbx_builtin_getvar_helper(chan, "SENDTEXT_BODY");
We probably want to run ast_str_get_encoded_str() on the SENDTEXT_BODY string.  As a consequence, we would want to defer allocating str until we have decided which string we are going to decode.


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


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



-- 
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: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 22:35:32 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180410/91db19d5/attachment.html>