[Home]

Summary:ASTERISK-03287: [patch] function to handle more easily, argument for applications
Reporter:mochouinard (mochouinard)Labels:
Date Opened:2005-01-16 13:37:51.000-0600Date Closed:2011-06-07 14:05:20
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) ast_argprocess.c.txt
( 1) ast_paramprocess.diff.txt
Description:This is not a patch by itself, but more a design sample. Im sure mark and other will propose other function name or stuff, so please let me know what you think so I can update and post a real patch and maybe a diff for app_dial also

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

Disclamed
Comments:By: mochouinard (mochouinard) 2005-01-16 13:39:10.000-0600

Current app use duplicate code to do repetitive function.  Also it block the usage of X as an argument, and also it require to have the check for X() type argument, check first.  This patch replace this.  Let me know what you think
By: Kevin P. Fleming (kpfleming) 2005-01-16 14:59:43.000-0600

I have been considering doing something like this as well, but I'd like to see it be even more aggressive:

1) The parsing rules for a given application should be an array of structs of type "ast_app_argument", so that it will not have to be processed at runtime for each call to the application. "struct ast_app_argument" would define the name of the arg, its type (for possible validation of numbers, etc.), whether it is optional or not, etc. Look in the Linux kernel sources for the way that they build PCI device ID arrays for an example of what I mean.

2) Since Asterisk apps don't take named arguments, I don't think there's a reason to have to extract them by name as well... it could just as easily be done by position (ordinal index into the array from item #1).

3) I would like to see the app just call ast_parse_arguments(data) and get back a pointer to an array of the arguments that were found... possibly an array of type ast_arg_value, in the same order as they arguments were supplied in item #1 (with optional arguments that were not supplied having a value of NULL).

Thoughts?
By: Kevin P. Fleming (kpfleming) 2005-01-16 15:00:39.000-0600

Additionally, this gives you a potential place to start supporting more complete "quoting" of argument values, and potentially named (rather than positional) arguments in the future (which is what I would like to).
By: mochouinard (mochouinard) 2005-01-16 15:29:14.000-0600

ast_paramprocess.diff.txt is an example patch that will modify how app_dial read those variables.
By: mochouinard (mochouinard) 2005-01-16 18:10:57.000-0600

This change, is to make the code cleaner.  Nothing more rightnow.  But we could modify this function to handle other type options if we want.

edited on: 01-16-05 21:30
By: Anthony Minessale (anthm) 2005-02-11 13:30:17.000-0600

have you seen
int ast_seperate_app_args(char *buf, char delim, char **array, int arraylen)
in app.c that I added last month?

char *argv[5];
int argc;
char *mydata;
if(data && ! ast_strlen_zero(data) && (mydata = ast_strdupa(char *data)))
argc = ast_seperate_app_args(mydata, '|', args, sizeof(argv) / sizeof(argv[0]));

If you do want to maintain your new invention, may I suggest that you just use the ast_variables which are identical to your params and have alloc/dealloc routines already.  

I always wanted to see the args passed in as or processed into a hash as I am a bigger fan of named params and have done so in the past on apps with lots o options by hand with strsep.

Also I think the X() arguements should all die they should get replaced with named arguements.

Now that you got me started on app_dial this is what I think:

The core ability to dial a channel/channels should be made into a core API call

ast_dial_multi(struct ast_channel *chan, int timeout, ...);

then do varargs to take a list of dial strings.

once that's done 1/2 of app dial goes away and becomes available to other mods.

next we create a registerable bridge_function you can implement from your module and access from any app and front end it

in load_module:
bridge_function_t *bridgefn = ast_get_bridge_func("standard");
the proto is (struct ast_channel*, struct ast_channel *, void *)

bridge_function(chan, peer, config);


parse features like this: ( +\s* tells the parser to read multi-line named args

exten => 1,1,NiftyDial(
macro    => mymacro
timeout  => 100
dial     => IAX2/user@peer1
dial     => Zap/10
announce => demo-congrats
)

back compat can be maintained by making a new app schema and registrar that coesists with the old one.

if the app registers the new way the pbx will know to pass the args as a list/hash if the new way is called on an old app the pbx can either yell at you or just convert the data to a big string.


extensions could be made into entities that could be passed around so a switch could ask for the entire extension as a whole and be done looking in the dialplan.

Something like that, this is a brainstorm right...?
There is a reason even unix started using named args.
By: Kevin P. Fleming (kpfleming) 2005-02-11 19:03:04.000-0600

I'm already working on splitting up app_dial into API functionality and app functionality, for the very reason that anthm mentioned (currently there are a number of modules reimplementing this code, not all of them the same way). Expect to see some patches in the next few days :-)
By: Kevin P. Fleming (kpfleming) 2005-02-11 19:04:43.000-0600

Oh, and I also wholeheartedly agree that Asterisk should move towards named arguments for apps, and the apps should be able to pass a complete list of possible arguments (along with types, optional/non-optional, maximum appearance counts, valid value ranges, etc.) to the parser, so that all the common functionality can be done once, and not repeated in the apps.
By: Anthony Minessale (anthm) 2005-02-12 11:26:58.000-0600

Waddya think about making a new app framework that is independant from the other one? We could add flags a registrar or a version number to the app obj so the original one would just not touch those settings and a new register system that did touch them could be implemented (with hashes for both the args and the registration itself) then the new apps could be passed a hash table to access the app data.

int new_app(struct ast-channel *chan, struct ast_hash *data)


While we're at it chan should have events

int myonhangup(struct ast_channel *chan) {
 ...
}

ast_addevent(chan, AST_EVENT_HANGUP, myonhangup);

Then if a chan hits your app it could plant code to be executed later on certian events


plus how bout we add a hash table to cdr too so you an set as many arbitrary name value pairs as you want, I already do this with the chan's vars but if you could somehow set the cdr to auto inherit chan vars into it's own set of vars that would not suck.
By: Kevin P. Fleming (kpfleming) 2005-02-12 11:44:27.000-0600

A new app system like that would be useful; it would mean that all the argument parsing and validation would be done before the app's "exec" method was ever called. That's a nice improvement.

The "channel events" idea is interesting too... I have a major channel method change patch coming later today, once that is in Mantis let's look into it.

I also have a major re-work of CDR in my local trees; it's not backwards compatible, though, but I think it has some major advantages. I'm not yet ready to post it, so if you want to implement something simpler in the existing CDR :-)
By: Anthony Minessale (anthm) 2005-02-12 11:55:18.000-0600

If we conbine the ideas maybe it could be like moh where there is just a void cdr pointer and you can stick anything there and register all kinds of difft cdr engines maybe.
By: Kevin P. Fleming (kpfleming) 2005-02-12 12:25:35.000-0600

That's possible, except in the CDR case you have to have the right sort of backend loaded as well, so it can properly dump the CDR structures out. I'll get on IRC later today and we can chat about this if you like.
By: Mark Spencer (markster) 2005-02-13 01:42:16.000-0600

We already virtually have named arguments with the "options" fields.  Look at how many options (including options which take a value) we have specified there.  Essentially, this is something like a command line.
By: Anthony Minessale (anthm) 2005-02-14 09:15:32.000-0600

Right but the 2 flaws are:

1) they are 1 char options with string arguments competing with the 1 char options all for the same 1 char namespace some args are better explained with multi chars

2) they are manually parsed over and over again in every app.

In unix even cat has named args nowadays (try cat --help)

Some apps need a better interface, some don't.


What I'm thinking about is possibly an alternate way to handle extensions similar to how a macro works where you take the extension as an entity and request it all at once not come back to the dial plan on each line.

This would make switches work better (more like dns) you look up an exten the system gets the whole exten (like a dns zone) and gives it to you to go off and execute, you can then destroy your copy of the exten and never look back, if the requestor or the mechinism that delivers the exten so chooses, it can implement a cache and/or a TTL.

The pbx has already been designed to have mutiple extension handler pbx_config.so is a modlue right, so i'm just suggesting an alternative if you could pass your dialplan around the same way dns passes around zones it'd make giant complicated dialplans easier.  It *is* possible to keep it simple and still be creative ya know =D.
By: mochouinard (mochouinard) 2005-02-17 14:44:59.000-0600

The idea of this code was to simplify what we currently have.  I agree we should design a better way that would be the standards for every app.  But this patch was to standardize all the applications. I found we had alot of duplicate code.  But we could implement something like this, and add something else over this latter.

Because I dont know how else we should parse it. using single letter make it very simple, and fix in the screen, adding string instead might not be as easy to read.
By: Brian West (bkw918) 2005-03-14 23:05:10.000-0600

Lets get something up for people to look at and get into CVS... did we finally agree on something!?

/b
By: mochouinard (mochouinard) 2005-03-17 13:09:25.000-0600

Mark dont like the design of this one