[Home]

Summary:ASTERISK-03935: [patch] Change parking operation to remember offset
Reporter:darkskiez (darkskiez)Labels:
Date Opened:2005-04-13 17:51:55Date Closed:2008-01-15 15:32:19.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Resources/res_features
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) parking_v3_findslot.patch
Description:This patch makes the parked calls always increase in number and then wrap to the beginning, and not find the lowest numbered first free slot to reduce the chances of the senario below.

Mrs X calls Mr X at work, operator parks call - 701, tells Mr X his wife is on 701

Mrs X hangs up for whatever reason, eg mobile goes dead, etc.

Serious businessman calls, operator parks call - 701

Mr X picks up 701 and talks dirty to the unsuspecting business man, hilarity ensues.

Comments:By: Christopher L. Wade (clwade) 2005-04-14 00:13:12

I like it, make it an option in features.conf and it the core guru's will likely accept it more easily :)

By: darkskiez (darkskiez) 2005-04-14 03:15:17

Sure, i'll add a config item, I didnt for a couple of reasons:

Firstly, it doesnt actually change the behaviour visible to the user, except
they might notice its not always 701 anymore, but they always should have listened to the park announcement anyway. With 701, or the first park slot, being the default, people didn't even listen and assumed it would be, this provoked mistakes of the users.

Secondly, and most importantly, I cant think of a decent name, I dont think the digium folks would accept parkingorderingthinger=yes as a config option, I may submit that anyway and let them change it at their discretion, unless you have any ideas?

By: darkskiez (darkskiez) 2005-04-14 03:34:43

parking_v2.patch adds a "parkorderingloop" configuration option to features.conf

Feel free to rename the configuration option, in fact, I actively encourage it.

By: darkskiez (darkskiez) 2005-04-14 07:32:11

Oh, Disclaimer is on file :)

By: Christopher L. Wade (clwade) 2005-04-14 08:35:28

Excellent, just one minor change I can think of... instead of "parkorderingloop" being a yes/no option, maybe it would be better to explicitly have "first|next" as your options.  Or, even simpler, call it "usenextavailableparkingslot" so that "yes|no" has real meaning.

PS. note I'm not a bug marshall, just liking this option and wanting it to make it into CVS.

By: darkskiez (darkskiez) 2005-04-14 08:49:49

I'm not going to upload a squillion patches with different option names, run the patch through sed with your personal favourite, I went for a yes|no as the parsing for that was easy, andIdidntWantALongOptionNameThatMadeTheConfigFileUgly.

Glad you like it, its been an annoyance of mine for a while, so I decided to fixit.

By: Kevin P. Fleming (kpfleming) 2005-04-20 12:07:14

I like this feature too, but I have a couple of thoughts:

1) you need to ensure that parkingoffset updates/reads are safe against multiple calls to Park()... from a quick review of the code they appear to be, but there may be a lock required to ensure that they are

2) the option should be called "findslot" with options "first" and "next" (defaulting to "first", obviously)... there is no need for 'available' to be in the option name or description, since it's pretty obvious we only look for available slots :-)

By: darkskiez (darkskiez) 2005-04-20 15:13:48

In response to kpfleming:

1) the offset is only updated in one place which is in the middle of the existing parking lock

2) name changed, config updated

Thanks for the positive feedback,

new patch uploaded v3, disclaimer is on file.

By: Kevin P. Fleming (kpfleming) 2005-04-20 23:44:30

Looks good to me, I'll schedule this one for Mark to review.

By: Kevin P. Fleming (kpfleming) 2005-04-20 23:45:55

Mark, this adds a minor new option, looks ready to commit to me.

By: Mark Spencer (markster) 2005-04-26 23:08:54

Added to CVS, thanks!

By: Digium Subversion (svnbot) 2008-01-15 15:32:19.000-0600

Repository: asterisk
Revision: 5512

U   trunk/configs/features.conf.sample
U   trunk/res/res_features.c

------------------------------------------------------------------------
r5512 | markster | 2008-01-15 15:32:19 -0600 (Tue, 15 Jan 2008) | 2 lines

Add option to park in the next slot (bug ASTERISK-3935)

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

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