[Home]

Summary:ASTERISK-05083: [patch] [post 1.2] Modified gotoiftime to add a branch if not in time range
Reporter:Benjamin Lawetz (benthos)Labels:
Date Opened:2005-09-14 10:42:17Date Closed:2005-12-01 13:59:25.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) asterisk-gotoiftime-negative-branch.patch.CVS-HEAD
( 1) asterisk-gotoiftime-negative-branch.patch.CVS-Stable
Description:Changed the gotoiftime so that the branching ressembles the gotoif
GotoIfTime(<times>|<weekdays>|<mdays>|<months>?[[matchcontext|]matchextension|]matchpri[:[[nomatchcontext|]nomatchextension||]nomatchpri])

Either the match or notmatch can be ommited (will continue on to the next priority.
The colon can also be ommited this replicates the current functionnality of gotoiftime (so reverse compatible)

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

gotoiftime(9:00-18:00|*|*|*?4:9)
if the time is between 9AM and 6PM, branch to priority 4
if the time is not between 9AM and 6PM branch to priority 9

gotoiftime(9:00-18:00|*|*|*?test,s,4)
if the time is between 9AM and 6PM, branch to context test, extension s and priority 4
if the time is not between 9AM and 6PM continue with next priority

gotoiftime(9:00-18:00|*|*|*?:0,3)
if the time is between 9AM and 6PM, continue with next priority
if the time is not between 9AM and 6PM, branch to extension 0 and priority 3


Comments:By: Michael Jerris (mikej) 2005-09-14 11:05:21

Unfortunately we are unable to accept undisclaimed patches such as this.  If you would like to persue this please fill out a disclaimer and submit it to digium.  Please re-open this bug when the disclaimer is filed.  Thanks for the contribution.
By: Benjamin Lawetz (benthos) 2005-09-14 11:12:26

faxed in disclaimer
By: Michael Jerris (mikej) 2005-09-14 11:16:59

Back to my question on IRC, you can alredy do this functionally, what does this add other than the ability to do it in one line instead of 2.  Do people want this.
By: Tilghman Lesher (tilghman) 2005-09-14 11:48:47

Frankly, I'm surprised it didn't already have the else.
By: Benjamin Lawetz (benthos) 2005-09-14 12:30:05

Actually the function I needed was a gotoifnottime (a little easier for my dialplan and the logic we're using)

But this functionnality gives you better flexibility on what you can do with the gotoiftime (And doesn't change anything if you don't need it)

you can have a dialplan like:
s,1,gotoiftime(9:00-18:00|*|*|*?3)
s,2,goto(4)
s,3,DIAL(SIP/office,20)
s,4,DIAL(SIP/cell,20)

but I much prefer:
s,1,gotoiftime(9:00-18:00|*|*|*?:3)
s,2,DIAL(SIP/office,20)
s,3,DIAL(SIP/cell,20)
By: Tilghman Lesher (tilghman) 2005-09-14 13:08:03

You know that you can do time wraps, right?

GotoIfTime(18:00-9:00|*|*|*?3)

or more accurately:

GotoIfTime(18:02-8:58|*|*|*?3)

since time specified is only accurate to the nearest even minute.
By: Benjamin Lawetz (benthos) 2005-09-14 13:21:12

didn't know you can do a wrap around.

So I guess what I want to do is feasible without my patch,
but I still prefer the gotoif format, makes more sense to me :-)

And thanks for the precise to the even minute, would explain a problem we were having here!

Ok, but what happens to the wrap around on things like:

gotoiftime(18:00-9:00|mon-fri|*|*?default,s,1)

I need to add a
gotoiftime(*|sat-sun|*|*?default,s,1) to cover the 2 others


By: Kevin P. Fleming (kpfleming) 2005-09-14 19:14:21

I would have to agree with MikeJ here... given that there are multiple other ways of accomplishing the desired behavior, what is the value of putting more stuff into this application?
By: Benjamin Lawetz (benthos) 2005-09-15 09:15:53

I just found it useful this way. I find it saves time and extra lines.
And in the spirit of standardizing the syntax of gotoif and gotoiftime would be similar.

It doesn't change the syntax in any way if you don't want to use it. But if you want to use it it's there.

I also find it's easier to use and to read the extensions this way.

But if you don't think so and don't want to include it, fine up to you guys.
By: Kevin P. Fleming (kpfleming) 2005-09-15 10:18:24

Mark and I were just discussing the value (or lack thereof) of having GotoIfTime/ExecIfTime/etc. at all, since the same functionality can be obtained by using two lines instead of one... but that's not something we need to change right now. In my mind it makes a lot more sense to have all the components you need to build up these operations, rather than having lots of applications that combine the functionality in different ways (and then have different bugs because they aren't kept in sync with each other).

I'll leave this one up to the rest of the community; if you want to add the 'no' branch to GotoIfTime I'll put it in.
By: Michael Jerris (mikej) 2005-09-15 10:25:28

Another way we could go here is to add a time function that returns true or false depending upon if the current time is in the range passed to the function.  This would reduce the code duplication and allow us to deprecate gotoiftime, execiftime, and possibly others, while still retaining the functionality, and normalizing how it is done in all places.  Thoughts?
By: Michael Jerris (mikej) 2005-12-01 13:48:15.000-0600

Can we please get an updated patch for this for current svn trunk including recommended changes or response to those comments.  Thanks.
By: Tilghman Lesher (tilghman) 2005-12-01 13:54:17.000-0600

Given that we're now into 1.2, I'd recommend that you use the IFTIME function:

Goto(${IFTIME(9:00-18:00|*|*|*?2:3)})

It's exactly what you need, and I'm tempted to deprecate GotoIfTime altogether.
By: Michael Jerris (mikej) 2005-12-01 13:59:06.000-0600

I hadn't realized that iftime went into the tree... seeing that it is, I am going to close this out as already dooable with current functionality.  If you still beleive that this is important to have in, please re-open and state your case.