> 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 > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev