[Home]

Summary:ASTERISK-11392: [patch] Feature to write variables to existing channels other than your own (func chanvar)
Reporter:Ramon Peek-Fares (ramonpeek)Labels:
Date Opened:2008-02-06 15:08:44.000-0600Date Closed:2008-02-25 17:19:21.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Functions/func_logic
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20080208__bug11943__2.diff.txt
( 1) 20080208__bug11943.diff.txt
( 2) chanvar.diff.txt
( 3) func_chanvar_1.4.17.diff
( 4) func_chanvar.diff
( 5) func_shared.c
Description:Many, many times I ran into a dialplan issue where I wanted to set a variable in the called channel whilst the dialplan was running in the calling channel.
To my frustration an application like ImportVar() existed but something like ExportVar() did not.

After bumping into the issue a bit too often I deciced to write it myself :-)
It probably took me less time writing the patch than it took finding out ways to realize this feature with the current set of applications & functions :-(

Anyway...
I started out writing the patch for 1.4.17 and called it 'ExportVar'.
But after finding out it was deprecated in the svn-trunk and talking to Digium (Qwell) about it,
I decided to create a new logical function in func_logic called 'CHANVAR'.
This feature includes read & write making function 'ImportVar' & 'IMPORT' both obsolete.
Since function 'IMPORT' is only in teh svn-trunk perhaps it should be removed or deprecated if this patch gets submitted.

This is how the new feature works;

To read a variable from another channel;
- Set(CHANVAR(channel,variable)=value)

To write a variable to another channel;
- Set(varname=${CHANVAR(channel,variable)})


PS:
For everyone that wants to add this feature (themselves) to Asterisk 1.4.17 a path to backport to 1.4.17 is also uploaded.
Comments:By: Tilghman Lesher (tilghman) 2008-02-06 23:36:36.000-0600

Haven't we already created a consensus that this is a really bad idea?

See http://lists.digium.com/pipermail/asterisk-dev/2006-May/020710.html


By: Ramon Peek-Fares (ramonpeek) 2008-02-07 04:09:55.000-0600

The page you are referring to is explaining ways to use channel inheritence to set variables in child channels and the use of globalvars or a databaseentry to pass information between channels.

In the many situations where I bumped into this issue, inheritence was not an option so I too tried to go along the path of setting globalvars or databaseentries.
This turned out to be a bad solution since you do not always have the chance to remove the globalvar or databaseentry ending up with a lot of waste in the variabele enviroment or database.
Besides this, realizing this is often a very long-winded process, especially in larger and complex dialplans.

Generally what I wanted to do was to set a variable in the originating channel containing information about the hangup of the called channel gathered from the 'h' extension in that channel.
And to achieve this there is currently no 'good' solution.

I found a good and working solution for myself by creating this patch.
Which makes everything a lot easier and in my honest opinion more the Asterisk way to go.
However, I don't work at Digium, so who am I to judge about that ;-)
But as an Asterisk enhousiast, I like to donate to the project too.
So to be sure I didn't create crap, I joined the IRC #asterisk-dev channel yesterday to discuss this feature and how it would be best to apply this to Asterisk.
I didn't not get any bad responses to the purposed feature and Qwell suggested to but it into the bugtracker as a feature request.(sorry I don't know who's behind the nickname).

Conclusion:
--------------------
If there are no real dangers involved with using this new feature I don't see any point in not making it easier to those who are facing the same issues I did.
Offcourse, I'll leave it up to Digium to decide...
By: Atis Lezdins (atis) 2008-02-07 04:28:59.000-0600

I can agree that this function would be useful in some specific cases. Generally if you're setting other channel variables - you would make sure that names are much more unique than ${i} and are not used in loop control.

For your concerns that this function can cause problems in dialplans of  incompetent people - i can suggest to disable this by default in menuselect.
By: Ramon Peek-Fares (ramonpeek) 2008-02-07 04:59:31.000-0600

I'm not worried about causing problems in the dialplans of incompetent people.
If they are really that incompetent they probably wouldn't know how to use this features ;-)
Besides, I included check and error/notice messages for those who use the feature incorrectly.

The reason why I mentioned 'if there are no real dangers involved' is that I could perhaps have overseen an issue where setting a variable in this manner on a channel might cause unforseen situations.
Perhaps anybody else could try to think about a way that this might cause problems?
By: Tilghman Lesher (tilghman) 2008-02-07 08:07:05.000-0600

The last quoted paragraph denotes why this is a bad idea:

> There's no need.  A database does this just fine already, in a
> completely predictable way.  Setting another channel's variables
> asynchronously is not a smart idea.  For example, if one channel
> is using ${i} to iterate through a loop and you asynchronously change
> the value of ${i} halfway through the loop, the dialplan is going to do
> something unpredictable and completely unreproduceable.

The ONLY way to ensure that a channel variable does not interfere with logic is to allow a channel the privilege of importing a variable from another channel.  In other words, it's fine for you to copy a tattoo I have, but it's incredibly bad for me to show up in the middle of the night and tattoo your back (without asking first).
By: Tilghman Lesher (tilghman) 2008-02-07 08:16:51.000-0600

And the real reason I don't want this added is that I can't distinguish in the bugtracker between competant and incompetant people.  All people know is if Asterisk does something unpredictable, that's bad, and I WILL have to deal with bug reports in the future if this goes in.  That is precisely why I have suggested alternative ways of doing this and why ImportVar was fine, but the corresponding ExportVar was rejected.

So for my own sanity (and the collective sanity of the rest of the development team), this should not go in.  This would be closed already, but Russell has accepted responsibility for this issue, so I'll let him make the final decision.
By: Atis Lezdins (atis) 2008-02-07 08:42:27.000-0600

Corydon76: the way to distinguish people would be menuselect option that's disabled by default and shows warning below it.

Imagine a case where you need to pass data from child channel to parent channel. In parent channel you don't know the name of child channel, so you have to create some complex dialplan macros to make that working.

ramonpeek: it would be a good idea to add "bridged" option to func_chanvar - so that i don't have to keep channel's name, but can just set bridged channel's variable.

Anyway - if this won't make to official releases, i hope that it will still be maintained for those who know why they need this.
By: Atis Lezdins (atis) 2008-02-07 08:44:19.000-0600

Oh, or it could be kept in SVN repository, but removed from tarball releases ;)
By: jmls (jmls) 2008-02-07 11:12:29.000-0600

I was one of the people that originally wanted this, but Corydon convinced me that this was a bad idea.

However, a thought has just struck me: What if some "incompetent" decided to update the UNIQUEID variable, or some other variable that meant something to the channel ? Surely that is as bad as setting a variable in another channel.

One way round this would be to have "readonly" variables. So, any variable created within a channel is readonly from any other channel IOW you would not be able to overwrite or change an existing variable from another channel but you would be able to create one.

This could also be extended to the "system" variables so that they cannot be changed from within the dialplan ...
By: Russell Bryant (russell) 2008-02-07 11:43:50.000-0600

After talking to Tilghman about this on IRC, he came up with a great idea for a compromise.  He proposed creating a SHAREVAR() function which uses some channel datastore magic to have a namespace of channel variables that is explicitly allowed for sharing between channels.

This approach gets my full support.
By: Russell Bryant (russell) 2008-02-07 11:46:20.000-0600

If I would have committed the original approach, it would have been the patch I just attached.  I posted it for the sake of showing what other files needed to be updated, as well as how I changed the code for Asterisk coding guidelines type stuff.
By: Jason Parker (jparker) 2008-02-07 11:51:06.000-0600

small feature request: If you don't specify a channel to the new function, I think it should use the current channel.

from channel1: Set(${SHAREVAR(channel2,somevar)}=${foo})
from channel2: Set(myvar=${SHAREVAR(,somevar)})


By: Jason Parker (jparker) 2008-02-07 12:06:17.000-0600

atis_work on IRC pointed out that channel needn't be the first argument.  That would make it easier to do what I suggested, it would just be Set(myvar=${SHAREVAR(somevar)})
By: Ramon Peek-Fares (ramonpeek) 2008-02-07 12:46:15.000-0600

Well I see a lot of discussion going round whilst I was away ;-)

I now also have to agree with Corydon's point of why the original approach was bad.

So in my opinion we now have three options;
---------------------------------------------
- Close this case and forget about it :-(
- Change the write part of this to only create new variables, not update (I'm happy with that)
- Create the SHARVAR() function which sounds neat too, but I'm afraid would be beyond my programming abilities ;-(

Offcourse in option 2 & 3 it would indeed be usefull to add the logic to use the current channel if no channel is provided.
I also don't see any reason exchanging the <channel> and <var> argument to make this easier.

So what option should we choose?
By: jmls (jmls) 2008-02-08 10:07:06.000-0600

uploaded func_shared.c. This allows you to store / retrieve variables from a channel datastore.

SHARED(var,channel)

i.e.

SET(SHARED(MYINFO)=123)
SET(TEST=${SHARED(MYINFO)})

this is the first time I've written anything this remotely complicated, so it will be full of holes and errors. I would appreciate people looking at it and telling me where I went wrong !
By: Tilghman Lesher (tilghman) 2008-02-08 11:29:52.000-0600

jmls:  you need to check out the coding guidelines, mostly.  You're also not freeing your structure in the free routine.
By: Tilghman Lesher (tilghman) 2008-02-08 17:42:43.000-0600

Patch updated to fix a memory leak and discuss usage in the description field.
By: Tilghman Lesher (tilghman) 2008-02-25 15:03:53.000-0600

Anybody tested this?
By: jmls (jmls) 2008-02-25 15:25:21.000-0600

yes, I did. Seemed to work well. I thought I'd mentioned it to you on -dev
By: Digium Subversion (svnbot) 2008-02-25 15:52:42.000-0600

Repository: asterisk
Revision: 104098

U   trunk/funcs/func_global.c

------------------------------------------------------------------------
r104098 | tilghman | 2008-02-25 15:52:41 -0600 (Mon, 25 Feb 2008) | 7 lines

Shared space for variables (instead of letting other channels muck with your own)
(closes issue ASTERISK-11392)
Reported by: ramonpeek
Patches:
      20080208__bug11943__2.diff.txt uploaded by Corydon76 (license 14)
Tested by: jmls

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

http://svn.digium.com/view/asterisk?view=rev&revision=104098
By: Digium Subversion (svnbot) 2008-02-25 17:19:21.000-0600

Repository: asterisk
Revision: 104105

_U  team/murf/bug11210/
U   team/murf/bug11210/apps/app_voicemail.c
U   team/murf/bug11210/channels/chan_agent.c
U   team/murf/bug11210/channels/chan_iax2.c
U   team/murf/bug11210/channels/chan_sip.c
U   team/murf/bug11210/configs/sip.conf.sample
U   team/murf/bug11210/doc/siptls.txt
U   team/murf/bug11210/funcs/func_global.c
U   team/murf/bug11210/main/config.c

------------------------------------------------------------------------
r104105 | murf | 2008-02-25 17:19:19 -0600 (Mon, 25 Feb 2008) | 133 lines

Merged revisions 104081,104083,104085,104087-104089,104093,104096-104098 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r104081 | file | 2008-02-25 08:12:48 -0700 (Mon, 25 Feb 2008) | 2 lines

Fix building of trunk. dbpass is always going to exist.

................
r104083 | file | 2008-02-25 08:19:58 -0700 (Mon, 25 Feb 2008) | 14 lines

Merged revisions 104082 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104082 | file | 2008-02-25 11:17:18 -0400 (Mon, 25 Feb 2008) | 6 lines

Due to recent changes tag will no longer be NULL if not present so we have to use ast_strlen_zero to see if it's actually blank.
(closes issue ASTERISK-11502)
Reported by: flefoll
Patches:
     chan_sip.c.br14.patch_pedantic_no_totag uploaded by flefoll (license 244)

........

................
r104085 | file | 2008-02-25 09:18:46 -0700 (Mon, 25 Feb 2008) | 14 lines

Merged revisions 104084 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104084 | file | 2008-02-25 12:16:13 -0400 (Mon, 25 Feb 2008) | 6 lines

If a resubscription comes in for a dialog we no longer know about tell the remote side that the dialog does not exist so they subscribe again using a new dialog.
(closes issue ASTERISK-10305)
Reported by: s0l4rb03
Patches:
     10727-2.diff uploaded by file (license 11)

........

................
r104087 | russell | 2008-02-25 11:38:51 -0700 (Mon, 25 Feb 2008) | 12 lines

Merged revisions 104086 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104086 | russell | 2008-02-25 12:38:10 -0600 (Mon, 25 Feb 2008) | 4 lines

Ensure that the channel doesn't disappear in agent_logoff().  If it does, it
could cause a crash.
(fixes the crash reported in BE-396)

........

................
r104088 | bbryant | 2008-02-25 12:00:16 -0700 (Mon, 25 Feb 2008) | 1 line

Adding more tls configuration details to sip.conf sample, with a list of valid ciphers provided in both files. .. First commit since July, woot
................
r104089 | file | 2008-02-25 12:02:33 -0700 (Mon, 25 Feb 2008) | 2 lines

Instead of outputting a verbose message every so often let's make it a debug message.

................
r104093 | qwell | 2008-02-25 13:50:57 -0700 (Mon, 25 Feb 2008) | 19 lines

Merged revisions 104092 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104092 | qwell | 2008-02-25 14:49:42 -0600 (Mon, 25 Feb 2008) | 11 lines

Allow the use of #include and #exec in situations where the max include depth was only 1.
Specifically, this fixes using #include and #exec in extconfig.conf.

This was basically caused because the config file itself raises the include level to 1.

I opted not to raise the include limit, because recursion here could cause very bizarre behavior.

Pointed out, and tested by jmls

(closes issue ASTERISK-11505)

........

................
r104096 | file | 2008-02-25 14:40:30 -0700 (Mon, 25 Feb 2008) | 14 lines

Merged revisions 104095 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104095 | file | 2008-02-25 17:37:20 -0400 (Mon, 25 Feb 2008) | 6 lines

Make it so a users.conf user creates both a SIP peer and a SIP user. The user will be used for inbound authentication for the device, and peer will be used for placing calls to the device.
(closes issue ASTERISK-8785)
Reported by: queuetue
Patches:
     sip-gui-friend.diff uploaded by qwell (license 4)

........

................
r104097 | tilghman | 2008-02-25 14:53:36 -0700 (Mon, 25 Feb 2008) | 13 lines

Merged revisions 104094 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104094 | tilghman | 2008-02-25 15:31:47 -0600 (Mon, 25 Feb 2008) | 5 lines

If the destination folder is full, don't delete a message when exiting.
(closes issue ASTERISK-11506)
Reported by: selsky
Patch by: (myself)

........

................
r104098 | tilghman | 2008-02-25 14:56:19 -0700 (Mon, 25 Feb 2008) | 7 lines

Shared space for variables (instead of letting other channels muck with your own)
(closes issue ASTERISK-11392)
Reported by: ramonpeek
Patches:
      20080208__bug11943__2.diff.txt uploaded by Corydon76 (license 14)
Tested by: jmls

................

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

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