[asterisk-dev] [Code Review] 4301: res_pjsip_outbound_registration: Add 'pjsip send register' command and update behavior of 'send unregister'

[Mark Michelson]

> On Dec. 30, 2014, 3:39 p.m., Joshua Colp wrote:
> > branches/13/res/res_pjsip_outbound_registration.c, lines 1155-1159
> > <https://reviewboard.asterisk.org/r/4301/diff/1/?file=70050#file70050line1155>
> >
> >     I'm concerned that the behavior that used to happen was explicitly documented, it wasn't overlooked. We're now changing it - why was it the previous way in the first place?
> 
> George Joseph wrote:
>     Well opticron wrote the original unregister stuff over a year ago and it never stopped the timer.  sgriepentrog added the comments only about 2 months ago.  If they're around, they can comment but if you call unregister why would you WANT it to start again automatically (other than there was no way to restart it)?
>     
>
> 
> jbigelow wrote:
>     When I dug into this area of code for the patch I wrote that sgriepentrog committed in 426924, I also thought it would be best to have it stay unregistered until the user decided to start registrations again with a command. I believe at the time it was decided for it not to be expanded to include what this very review is for. Thus I added a note to the CLI usage about how it will eventually attempt to re-register. So the patch here has my vote but I can't speak for what was originally intended.
> 
> Kevin Harwell wrote:
>     Just a note on this, but if the behavior ends up changing be sure to update the wiki on configuring outbound registration as well: https://wiki.asterisk.org/wiki/display/AST/Configuring+Outbound+Registrations
>     
>     Under "Manually Unregistering" near the bottom there is a box that states: "After manually unregistering, the specified outbound registration will continue to reregister based on its last registration expiration."
> 
> George Joseph wrote:
>     Yep, I've got that on my list.

I'm all for having this stop future attempts at registrations. I'd recommend that this gets a mention in CHANGES as well.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4301/#review14053
-----------------------------------------------------------


On Dec. 30, 2014, 10:09 p.m., George Joseph wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4301/
> -----------------------------------------------------------
> 
> (Updated Dec. 30, 2014, 10:09 p.m.)
> 
> 
> Review request for Asterisk Developers, opticron and Scott Griepentrog.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The current behavior of 'pjsip send unregister' is to send the unregister (REGISTER with 0 exp) but let the next scheduled register proceed normally.  I don't think that's a good idea.  If you unregister, it should stay unregistered until you decide to start registrations again.  So this patch just adds a cancel_registration call to the current unregister_task to cancel the timer.
> 
> Of course, now you need  a way to start registration again so I've added a 'pjsip send register' command that unregisters and cancels any existing registration (the same as send unregister), then sends an immediate registration and starts the timer back up again.
> 
> Both changes also ripple to AMI.  There's a new PJSIPRegister command.
> 
> There's no harm in calling either command repeatedly.  They don't care about the actual state.
> 
> I did discover a previously existing ref count leak for state though.  Every time asterisk sends a register, the count gets incremented but never decremented.  I'll look at this separately.
> 
> 
> Diffs
> -----
> 
>   branches/13/res/res_pjsip_outbound_registration.c 430163 
> 
> Diff: https://reviewboard.asterisk.org/r/4301/diff/
> 
> 
> Testing
> -------
> 
> The current tests/channels/pjsip/registration/outbound/unregister tests run fine.  I'm working on additional tests to exercise the register command.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150105/8a4233bf/attachment.html>