CF ringtimer

This patch introduces the cf ringtimer. It's only affecting macro-dial-one, dialparties is a not easy to get around, i would vote for only displaying the Call Forward Rint Time selectbox when we are using macro-dial-one, ie 1.6 or 1.4 with extension_state.

Left to do is to add a selectbox in ARI.

The patch includes the patch for #4332

pnlarsson,

if we are going to make changes to the behavior of CF/CFU/CFB then we need to be consistent which means however an extension is ringing (dial-one or dialparties.agi) it needs to be consistent.

Added support for dialparties as well - only for ring method: none.

Tested and it's working

Patch for ARI attached to #4332

(In [10657]) use ringtimer from astdb instead of sql re #4332 (no ari update yet and nothing removed yet from sql), also some minor fixes to logic and removal of unneeded set commands in macro, also re #4652 affects some patches but further comments to come in the ticket

pnlarsson,

let's see when we can sync up on the IRC or live. There are some issues I notice in some of this that will be easier to go through interactively. Here is a summary but I might be missing something:

• macro-simple-dial would need updating
• current implementation has the goto within a subroutine, this may result in negative consequences, we will have to change that code
• this probably needs some general settings additions, since you may want to configure the entire PBX so that all CFs are '-1' and not have to do this with every extension (basically make that the default)
• there may be some issues in the dialparties.agi, I need to look closer and think about the implications of only addressing the 'none' case (which won't exist much longer since those should always go through macro-dial-one in the future.

my recent updates are going to make the patches have to be changed anyhow but we should go over the described points above and resolve them and then just get this in.

Replying to p_lindheimer:

* macro-simple-dial would need updating

Will take a lock at that

* current implementation has the goto within a subroutine, this may result in negative consequences, we will have to change that code

Adding a Stackpop before Goto should bdo the trick.

* this probably needs some general settings additions, since you may want to configure the entire PBX so that all CFs are '-1' and not have to do this with every extension (basically make that the default)

Another amportal setting - turned into a global dialplan var. Maybe a subroute or dealing with the goto stuff or a bunch of nested if()'s

* there may be some issues in the dialparties.agi, I need to look closer and think about the implications of only addressing the 'none' case (which won't exist much longer since those should always go through macro-dial-one in the future.

The point is that we are only using the cfringtimer when we are dealing with dialing a single extension, ringgroups are using the ringgroup timer, followme it's own timer etc. Or i might hae missed something...

Nicely done with the ringtimer - sometimes the solution is to simple.

pnlarsson,

I've spent a bunch of time looking over the changes. (Unfortunately, a lot more time then you would think would be necessary for making such a "simple" change, but this sits in some pretty core critical areas so it's important to be really careful.

First off macro-exten-vm changes. I've made a variation of what you did in your changes and added the StackPop? call that you suggested. It's included in my partial patch along with the macro-dial-one changes that I'll discuss next.

On the macro-dial-one side, first off there were a couple issues in the logic that you put in. It would also need a StackPop?, you are skipping the CFIGNORE, and you were counting on EXTTOCALL to be set which may not be the case depending on who is calling this macro. (Today it's probably safe because of macro-exten-vm, but not necessarily in the future).

In any event, my main concern is that you go straight to the goto if CF is -1, regardless of some of the other tasks that macro-dial-one may be doing further on before the dial() command. (MOH class, ALERT_INFO, etc.). The other thing that concerns me is that there is a lot of 'delicate' setting of various variables sometimes inheritable by a single channel and sometimes infinitely. I am extremely reluctant to use a goto in this case because of that issue. It's likely I'm being paranoid but knowing the time that it can track down these subtleties I think this time around I want to stick with the Dial() through local channel. If there is a really strong reason to use a goto then a lot of careful analysis needs to be done plus the other issues I addressed still apply. (Which means you would have to do the goto() further down in the dialplan roughly at the point where it otherwise would have done the dial(). I'm happy for us to talk further about this if you have put additional thought into this and have some other data.

On the macro-simple-dial that I brought up, I'm having second thoughts about if we should do anything there or not after looking at the dialplan and some of the comments in the code. It is a very limited macro used by ringgroups only (I think only with followme) and I'm not sure that the CFB/CFU's should follow the cfringtimer rules there. We can also discuss this further, it's been a long time since I've thought about that part of the code.

At this point I have not looked at the dialparties.agi code or thought further about it, nor the other changes to the GUI you made which I suspect is pretty straight forward. I wanted to make sure to get this part of the feedback to you to think about and then we can try to chat tomorrow to go over it.

The partial suggested changes per my above comments are in cf.diff

pnlarsson,

I've had a look at dialparties.agi. I agreen now that the ringtimer should only be manipulated for rgmethod of 'none.' If it's a ringgroup, it's going to be dictated by the ringgroup timer or earlier if the called extension has a shorter timer, but in many cases, I believe people set CFIGNORE anyhow to avoid CF scenarios for groups.

Given this, I believe the changes are more straight forward. If we decide it's CF then we reset the main timer. So I think it is just the following code. Let me know if you think I missed something, I have not tested this yet:

Index: dialparties.agi
===================================================================
--- dialparties.agi	(revision 10658)
+++ dialparties.agi	(working copy)
@@ -301,11 +301,26 @@
 			    $AGI->set_variable('DIALSTATUS','NOANSWER');
 		    } else {
 			    $ext[$count] = $cf.'#';  
+			    debug("Extension $k has call forward set to $cf", 1);
 					// This only really needs to be set if we are setting Diversion Headers, but it's not worth the hassle of
 					// checking the amportal.conf settings here and there is no harm done in setting it other than minor overhead
 					//
-					$AGI->set_variable('__DIVERSION_REASON','unconditional');
-			    debug("Extension $k has call forward set to $cf", 1);
+          // Set DIVERSION_REASON only when rmethod is none, otherwise it's a ringgroup/findmefollow and
+          // if we want to set diversion headers they should be set by the group.
+          // 
+          // For CF timer, we change the timer value if rgmethod is none meaning a single extension is being called.
+          // CFB and CFU are handled in macro-exten-vm for single extensions. (And this script is being phased out in
+          // favor of macro-dial-one for single extensions when function EXTENSION_STATE is available).
+          //
+          if ($rgmethod == "none") {
+            $AGI->set_variable('__DIVERSION_REASON','unconditional');
+            $cfrt = $AGI->database_get('AMPUSER',$k . '/cfringtimer'); 
+            $cfrt = $cfrt['data'];
+            if (!empty($cfrt)) {
+              $timer = $cfrt < 0 ? "" : $cfrt;
+              debug("Ring timer changed to CF ringtimer value of ".($cfrt < 0 ? "Always":"$cfrt sec"), 1);
+            }
+          }
 		    }
   
 		    // if this is the primary extension and CF enabled, then cancel mastermode

So my current 'full patch' from my tweaks to your patch and various comments looks like the new attached file. It includes an amportal.conf setting for someone to set default behavior for extensions being created.

replaced, added mods to do GOTO in CF Always cases

pnlarsson,

I've modified the patch file I attached earlier and modified macro-dial-one to do a goto in the case of CF with (Always) setting for cfringtimer (-1). I'm still a bit nervous for the reasons mentioned but we may as well do some testing on it and see how it goes…

philippel,

two changes: dialparties - when CW disabled and user busy - we will reach a busy thing in dialparties - added the CFRINGTIMERDEFAULT is never used since $cfringtimer is always set - fixed

Tested with and without extstate on 1.4, all worked as it should

pnlarrson,

I see what you mean on dialparties.agi however I'm wondering if that is not a 'bug' that it does the cfb in dialparties.agi for rgmethod = 'none', we should make sure it is consistent with macro-dial-one.

on the CFRINGTIMERDEFAUL, per our IRC discussion that is only for new extension defaults.

(In [10683]) Adding cfringtimer to dialparties and macro-dial. Addning gui to set cfringtimer. Re #4652

(In [10685]) Adding amportal default for CFRINGTIMERDEFAULT. Re #4652

(In [10686]) Adding ringtimer and cfringtimer settings in ARI. Closes #4332 and #4652