[Home]

Summary:ASTERISK-04119: [patch] Functions to replace SetLanguage, AbsoluteTimeout, DigitTimeout, ResponseTimeout, SetCIDNum, SetCIDName, SetRDNIS
Reporter:Jeffrey C. Ollie (jcollie)Labels:
Date Opened:2005-05-09 10:15:27Date Closed:2008-01-15 15:34:46.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Functions/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) asterisk-miscfunc-4.patch.txt
( 1) func_callerid.c
( 2) func_language.c
( 3) func_timeout.c
Description:Dialplan functions to replace SetLanguage, AbsoluteTimeout, DigitTimeout, ResponseTimeout, SetCIDNum, SetCIDName, SetRDNIS.  Includes doc updates.

****** ADDITIONAL INFORMATION ******

Disclaimer on file.
Comments:By: Olle Johansson (oej) 2005-05-09 10:30:33

Setlanguage is involved in the channel -changes a channel setting. Shouldn't that still be an app?
By: Jeffrey C. Ollie (jcollie) 2005-05-09 10:49:44

I quote the coding guidelines:

"Functions (which can be provided by any type of module) are used when the provided functionality is simple... getting/retrieving a value, for example. Functions should also be used when the operation is in no way related to a channel (a computation or string operation, for example)."
By: Tilghman Lesher (tilghman) 2005-05-09 11:11:44

The timeouts directly affect the operation of the channel and probably should not be functions.
By: Jeffrey C. Ollie (jcollie) 2005-05-09 11:27:06

I don't see how setting the timeouts in an application is better than setting them in a function.  The code for either the application or the function is very simple.  The function version just looks more complex because it combines 3 applications into 1 function.  It would seem to me that this is exactly what functions are supposed to be for.
By: Tilghman Lesher (tilghman) 2005-05-09 11:58:16

It's not a matter of 'better' or 'worse'.  It's a matter that these values directly affect the operation of a channel, which is what we've decided applications are for.  The timeouts are probably in a gray area, in terms of their functionality.  Perhaps it would be helpful to have them as both (i.e. in addition to, not instead of)
By: Brian West (bkw918) 2005-05-09 12:39:12

No these should be functions really.. they do very simple operations to set or get things from a channel.  In reality a function are no diffrent than apps.. they are just evaluated in the context of a variable.

/b
By: Jeffrey C. Ollie (jcollie) 2005-05-09 15:47:31

Added patch/files to replace SetCIDNum, SetCIDName, SetRDNIS.
By: Kevin P. Fleming (kpfleming) 2005-05-14 23:36:13

I agree, these should be functions. If the coding guidelines on the function vs. application distinction are not adequate to cover this situation, then let's get them fixed up.
By: Kevin P. Fleming (kpfleming) 2005-05-14 23:45:57

There are few buglets here:

- The syntax for LANGUAGE is wrong; it shouldn't take the value as an argument (in the 'data' parameter), but as a value (in the 'value' parameter).

- All the write() functions need to check for value being non-NULL and exit early if it is NULL.

- Don't use '%i' for formatting numbers, we're trying to stop that showing up in Asterisk code :-)

- This is borked:
} else if (strncasecmp("ani", data, 3) == 0) {
if (chan->cid.cid_num) {
ast_copy_string(buf, chan->cid.cid_ani, len);
}

- Please use uppercase for ANI, DNID and RDNIS in the docs, since they are abbreviations and that is common usage (although technically DNID is not really Caller ID information at all <G>)
By: Jeffrey C. Ollie (jcollie) 2005-05-15 13:17:20

Kevin, I've uploaded new patch/files that I believe take care of the issues that you mention.
By: Kevin P. Fleming (kpfleming) 2005-05-15 13:40:15

Committed to CVS HEAD with minor mods as discussed on IRC. Thanks!
By: Digium Subversion (svnbot) 2008-01-15 15:34:46.000-0600

Repository: asterisk
Revision: 5679

U   trunk/UPGRADE.txt
U   trunk/apps/app_setcidname.c
U   trunk/apps/app_setcidnum.c
U   trunk/apps/app_setrdnis.c
U   trunk/funcs/Makefile
A   trunk/funcs/func_callerid.c
U   trunk/funcs/func_groupcount.c
A   trunk/funcs/func_language.c
A   trunk/funcs/func_timeout.c
U   trunk/pbx.c

------------------------------------------------------------------------
r5679 | kpfleming | 2008-01-15 15:34:45 -0600 (Tue, 15 Jan 2008) | 2 lines

add dialplan functions for Caller ID, language and timeouts (bug ASTERISK-4119, with mods)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=5679