[Home]

Summary:ASTERISK-00922: [post-1.0][patch] Add option to app_dial for variable communication
Reporter:mman (mman)Labels:
Date Opened:2004-01-27 10:28:01.000-0600Date Closed:2008-01-15 15:12:20.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) dial_transfer_vars20040719_new.patch
Description:This patch enables app_dial to pass the variables of the channel spawning the "Dial" application, to the newly created channel(s). The feature is activated through the usage of a new option "v". When this option is not used, the behaviour of Dial is the one currently in use (doesn't break current dialplans).

Michael.
Comments:By: Brian West (bkw918) 2004-01-27 11:57:56.000-0600

Can you give an example of how this could be used?  Have you sent in your disclaimer?
By: mman (mman) 2004-01-27 12:24:20.000-0600

Here is an example.
Having the following section in dialplan:

exten => 111,1,SetVar(lala=10)
exten => 111,2,Dial(TYPE/NUMBER)

the (non-global) variable "lala" can't be seen by the new channel
created by "Dial".
With the attached patch and the following extensions:

exten => 111,1,SetVar(lala=10)
exten => 111,2,Dial(TYPE/NUMBER,,v)

the variable "lala" is available to the new channel.
This can be used as a simple way to pass non-standard parameters
(like distinctive ring) to the channels created by Dial (in XXX_call() function
of a channel driver).

I haven't sent a disclaimer, but I'll do it shortly.

Michael.
By: Olle Johansson (oej) 2004-03-21 10:19:35.000-0600

Great stuff, I'll test this. Was looking for a similar feature.
By: Olle Johansson (oej) 2004-04-15 15:52:03

Dislaimer sent? If not, please do.
By: mman (mman) 2004-04-16 06:17:46

On a second thought I'm not sure if a Dial option is needed. I think that it would be better to enable the functionality uncoditionally. No need to extend the already long options list of Dial. If you guys agree, I'll update the patch. Oh, and I'll send a disclaimer.
By: Olle Johansson (oej) 2004-04-16 06:19:33

Hmmm. I wonder if there's a reason for NOT forwarding all variables...
By: mman (mman) 2004-04-16 07:28:51

I can't think of any such reason too, so I have attached a modified patch.
The new patch (dial_communicate_vars_noopt.patch) doesn't use an option. This is a trivial patch, so I guess no disclaimer is needed.
By: Brian West (bkw918) 2004-04-17 23:09:50

this could be handy to find forwarding loops! :)
By: Olle Johansson (oej) 2004-04-20 12:22:40

I discussed this with Mark and we agreed upon a solution that we want feedback
on:

* Not transfer all variables by default (as in the current patch)
* Transfer all variables that begin with "_" (underscore) once and
 remove the underscore in the new channel leg
* Transfer all variables that begin with "__" (two underscore)
 without removing anything, making it stay even if the new leg
 is transferred to a third leg
By: Mark Spencer (markster) 2004-05-02 12:37:27

Note that the attached patch most assuredly does not do what we had discussed.
By: mman (mman) 2004-05-04 06:01:07

OK, the proposed method seems to cover most of the cases.
I'll come back when I have the modified patch that does what has been described.
By: mman (mman) 2004-05-04 09:43:40

The new patch (dial_transfer_vars.patch) does exactly what has been described.
An important note is that the channel variable "ALERT_INFO" is not transferred
by default anymore (it was wrong anyway) and might cause some trouble.
By: Olle Johansson (oej) 2004-05-24 15:22:23

The patch doesn't work against current cvs head. Please update!
By: mman (mman) 2004-05-25 05:37:03

Patch updated (dial_transfer_vars20040525.patch).
By: Mark Spencer (markster) 2004-05-26 01:23:08

I gotta ask, why not use pbx_builtin_setvar to set them in the new channel?
By: mman (mman) 2004-05-26 07:23:37

Equally right.
By: Olle Johansson (oej) 2004-06-11 08:29:06

I've tested this and it works for me.
By: mman (mman) 2004-06-11 09:16:51

I would suggest to hide the special meaning of '_' and '__' characters from the
name of the variables. The user should not know about these. Instead,
the functions that operate on the variables should know the special meaning of
them. This can be done by changing the functionality of pbx_builtin_setvar(...)
function into something like this:

void pbx_builtin_setvar_helper(struct ast_channel *chan, char *name, char *value, int type)

chan, name, value: as the current implementation
type: Determines the type of the variable. Possible values:
     1 - Non-transferable (the variable's name stays as is)
     2 - Soft-transferable (the variable's name is prefixed with a '_', causing the transfer of the variable, in case of a Dial() once)
     3 - Hard-transferable (the variable's name is prefixed with a '__', causing the transfer of the variable, during any subsequent Dial())

Ofcourse, a variable's name is not allowed to start with '_' and, additionally,
the function that returns a variable's value (pbx_builtin_getvar_helper) must be updated to support the above.

An example dialplan could be:

exten => 111,1,SetNTVar(LALA=123)  ; Set a non-transferable var
exten => 111,2,SetSTVar(LOLO=456)  ; Set a soft-transferable var
exten => 111,3,SetHTVar(LULU=789)  ; Set a hard-transferable var
exten => 111,4,Dial(CHAN/number)

Thoughts?
By: hwstar (hwstar) 2004-06-11 10:56:03

In addition to transferring variables to the other leg, It would be nice to be able to transfer them across logical channels when a user flashes the switchhook.

For example, when Asterisk receives a call from in incoming trunk with caller ID, it could copy it to a Hard transferrable variable, dispatch the incoming call with Dial to several extensions, and when one of the extension answers at say Zap/X-1, then flashes the switchhook, the hard variable would also be available in the namespace of Zap/X-2.

I'm in the process of implementing such a solution by allowing arbitrary copies from one namespace to another, but it would be more elegant to do it by extending your method.

Steve.
By: Olle Johansson (oej) 2004-06-11 11:08:51

Transferring over channels is another issue.
Adding more apps is a clever idea, makes it easier. I am not sure that
setNTvar/STvar or /HT are the easiest-to-understand names.
By: mman (mman) 2004-06-11 11:22:29

Please suggest some names for the applications. In the example I used some dummy names.
By: hwstar (hwstar) 2004-06-11 12:32:42

oej-

Are you suggesting I continue my efforts creating an arbitrary channel transfer app, are you concerned that such a change will complicate releasing this enhancement, or is there something else you had in mind to transfer variables across channels?

Steve.
By: Olle Johansson (oej) 2004-06-11 13:57:17

hwstar: Yes, transfer of variables across channels, between servers, is another issue, not to be included in this bug #. If you meant something else, I've misunderstood.

Mic:
We have setvar and setglobalvar, they will stay for backwards compatibility
maybe
setchanvar: Set variable that is copied into the next channel after dial
setpermchanvar: Set permanent variable that is copied as long as we have a channel chain
I'm not found of "setpermchanvar" but that's all I can come up with friday evening :-)
By: hwstar (hwstar) 2004-06-11 15:05:46

oej-

Thanks for the clarification. Sorry if I caused any confusion. I'm going to continue my work on a getchanvar app which will allow one to copy a variable from another channel into the current channel. I'll submit this as a separate bug.


Mic-
What about SetVarChanType(var, type)
Where 'type' can be one of the folowing:

normal,
inherit-once,
inherit-always

Steve.

edited on: 06-11-04 14:50

edited on: 06-11-04 14:51

edited on: 06-11-04 14:55
By: Olle Johansson (oej) 2004-06-13 13:55:40

See the new app in chan_sip2, sipaddheader(). It hides the _ construct from the user (thank you for that idea) and uses this patch to make the variables stay across the dial(). Other app developers may still use the _ construct.

SetInheritVar(type, var=value) is better. That way, a variable may contain a comma. And normal is SetVar() that stays due to backwards compatibility, so
SetInhetritVar(once|forever,varname=value) is my suggestion.
By: mman (mman) 2004-06-15 04:23:09

hwstar, oej:
Thanks for the suggestions. I like oej's SetInheritVar(type, var=value) for the
transferable variables, so I'll include it in the next updated patch.

Michael.
By: Olle Johansson (oej) 2004-06-15 04:31:16

Well, with the comma patch the other day, variables may contain commas regardless of position now. I still opt for having the value in the final position.
By: mman (mman) 2004-06-17 04:51:19

I have implemented a SetInheritVar() app and it works ok. There is another
issue though. Variables 'VAR', '_VAR' and '__VAR' are all the same variable
'VAR' (each one is of different type of course). So, ast_var_name() will be
changed to return 'VAR' in any of the three cases. Additionally, a function
ast_var_full_name() will be added (in chanvars.c) which will return the full
name of a channel variable (as the current implementation of ast_var_name()).
Something like that will be used by Dial().

That way, modules that use the ast_var_name() function will continue working
right (if they search for a variable 'MYVAR' and the current channel has a
variable named '__MYVAR', there will be a match).

Finally, ast_var_assign() will remain intact. The module author that uses this
function must know the special meaning of the underscores in the beginning of
the variable's name.

Does that approach seem reasonable?
By: mman (mman) 2004-06-22 11:23:19

The updated patch (dial_transfer_vars20040622) includes all the changes to
make things work as described in my latest bugnote.
The description of the new app:

 -= Info about application 'SetInheritVar' =-

[Synopsis]:
Set the value and the inheritance type of a variable

[Description]:
 SetInheritVar(once|forever|never, #n=value): Sets variable n to value. The
variable is marked as transferable, i.e. it will be transferred to the new
channel on a Dial(). The inheritance type can be 'once' (the variable
will be tranferred on the next Dial() only), 'forever' (the variable will be
transferred by all subsequent Dial() calls) or 'never' (the variable will not
be transferred on a subsequent Dial(), as in 'SetVar()'.
By: Mark Spencer (markster) 2004-06-22 14:00:10

I'm starting to have second thoughts about the feel of _ and __ as global/not global.  If we're setting inheritance, lets just make the inheritance a field in the variable structure, what do you say?  That also will prevent current code from having to change.
By: mman (mman) 2004-06-23 06:38:53

Using a variable in the ast_var_t struct to indicate the inheritance of a
chanvar won't do things easier. On the contrary, it will cause more changes
in current code because we will have to set/get somehow this variable in
all places where a chanvar is used.
Of course, I do believe that this is the best way to implement it.
By: jrollyson (jrollyson) 2004-06-23 14:21:38

Which patches do we still need? Just the last one?
By: Mark Spencer (markster) 2004-06-23 14:34:13

We still need something to set that inheritance -- both an Asterisk function and an application.
By: flavour (flavour) 2004-06-25 04:50:14

Today's CVS:

patching file apps/app_dial.c
Hunk #4 FAILED at 716.
By: mman (mman) 2004-06-25 07:20:25

The latest patch "dial_transfer_vars20040625.patch" can be used with current
CVS code. Please remove the previous. They are not needed anymore.

Mark: The patch provides both an Asterisk function (pbx_builtin_setinheritvar_helper)
and an application (SetInheritVar).
By: Olle Johansson (oej) 2004-07-16 13:50:52

Are we ready for CVS or does Mark have any more comments on this? We need this functionality.
By: Brian West (bkw918) 2004-07-16 22:20:16

can you update this patch ASAP
By: mman (mman) 2004-07-19 05:26:31

Patch updated. Disclaimer sent.
By: mman (mman) 2004-07-19 08:41:26

Please use the "dial_transfer_vars20040719_new.patch" file.

Michael.
By: schurig (schurig) 2004-07-23 14:25:58

I like that the inheritance is a field in the variable structure.

I'd also like if there would only be one app to set variables, but with parameters.


SetGlobalVar()                stays, but marked deprecated
SetVar(VAR=VALUE)             stays
SetVar(VAR,VALUE)             as before
SetVar(VAR,VALUE,g)           global
SetVar(VAR,VALUE,i)           inherit once
SetVar(VAR,VALUE,I)           inherit always

(maybe the last one can be an "a")


I'd also say that we then add the usual quoting rules that decent apps have. If "VALUE" is in quotes, then it can have commas, but the quotes get removed.
Maybe we can do it the python way, where "test's" is ok, as well as "Mother said 'No'".
By: Russell Bryant (russell) 2004-10-07 23:17:41

Housekeeping!

Is this still applicable with the new realtime stuff?
By: Olle Johansson (oej) 2004-10-08 02:09:39

As I see it, this has nothing to do with realtime configs. This is still very much needed. I want it in CVS :-)
By: Olle Johansson (oej) 2004-10-20 13:41:27

Still want this to be part of cvs...
By: Mark Spencer (markster) 2004-10-31 21:20:57.000-0600

Added to CVS with slight mods.
By: Russell Bryant (russell) 2004-11-02 20:56:56.000-0600

not in 1.0
By: Digium Subversion (svnbot) 2008-01-15 15:12:20.000-0600

Repository: asterisk
Revision: 4141

U   trunk/apps/app_dial.c
U   trunk/chanvars.c
U   trunk/include/asterisk/chanvars.h
U   trunk/pbx.c

------------------------------------------------------------------------
r4141 | markster | 2008-01-15 15:12:20 -0600 (Tue, 15 Jan 2008) | 2 lines

Make channel variables inheritable by _ (bug ASTERISK-922)

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

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