[asterisk-dev] [Code Review] Remove need for registration strings in sip.conf

[Nick Lewis]

> On 2010-06-23 14:42:49, David Vossel wrote:
> > trunk/configs/sip.conf.sample, lines 588-593
> > <https://reviewboard.asterisk.org/r/718/diff/4/?file=11012#file11012line588>
> >
> >     I don't understand the need for this option.  We already have the  'callbackextension' option, and changing the behavior to always default to an extension matching the peer name would not be something I'd expect.
> >     
> >     I'd prefer that the default callback remain 's' unless otherwise specified by the 'callbackextension' option.

This is standard behaviour for most UACs, makes incoming and outgoing calls use the same nomenclature and simplifies the dialplan e.g.

sip.conf:
...
[trunk1]
register=yes
...
[trunk2]
register=yes
...

extensions.conf:
...
[alltogether]
exten = _9.,1,Dial(SIP/trunk1/${EXTEN:1})
exten = _8.,1,Dial(SIP/trunk2/${EXTEN:1})
exten = trunk1,1,Goto(sales,1)
exten = trunk2,1,Goto(support,1)

but I accept that it is an unfamiliar concept to asterisk-dev and will remove it


> On 2010-06-23 14:42:49, David Vossel wrote:
> > trunk/configs/sip.conf.sample, lines 1094-1105
> > <https://reviewboard.asterisk.org/r/718/diff/4/?file=11012#file11012line1094>
> >
> >     If we change don't make the default callback extension the peer name this example will have to change again.

Ok I will put this back to two shared handles between sip.conf and extensions.conf


> On 2010-06-23 14:42:49, David Vossel wrote:
> > trunk/configs/sip.conf.sample, lines 567-570
> > <https://reviewboard.asterisk.org/r/718/diff/4/?file=11012#file11012line567>
> >
> >     Does it make sense for this option to be called 'callbackexpiry'? or should it be something like 'registerexpiry'?

I think I agree with you (and oej). I was originally concerned that with the parameters
regexten
regexpiry
callbackexten

the regexpiry parameter would be associated with regexten rather than callbackexten. Ideally regexten would be called autodialplan or something else without a reg or register prefix to limit confusion but I think it is okay.

Another possibility is to make the register parameter like qualify i.e.
register=no
register=yes (3600 secs)
register=60

What do you think?


> On 2010-06-23 14:42:49, David Vossel wrote:
> > trunk/channels/chan_sip.c, lines 4461-4488
> > <https://reviewboard.asterisk.org/r/718/diff/4/?file=11009#file11009line4461>
> >
> >     This is clever, but I don't think it will work the way you expect it to yet unless that peer is in the 'peers' ao2_container.   When 'the_mark' is set on a reload, only the peers in the 'peers' container are removed that have the_mark.  I don't think build_peer() puts the peer in that container.

This is a bit new and distant for me as I do not yet have any realtime test setup so may I clarify your comment. Are you saying that the code will not result in sip_registry items being created for realtime peers with register=yes or that the flushing out of the realtime peers will not happen just by setting the_mark? 

My assumption at realtime operation was that when an incoming sip request was received a match was made on a peer definition in the database and it was loaded into memory. Once the relevant processing had taken place it was given the_mark and subject to garbage collection to free the memory. Can you please correct my understanding


- Nick


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


On 2010-06-23 11:03:32, Nick Lewis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/718/
> -----------------------------------------------------------
> 
> (Updated 2010-06-23 11:03:32)
> 
> 
> Review request for Asterisk Developers and David Vossel.
> 
> 
> Summary
> -------
> 
> This change adds the ability to specify the sip registration expiry and registrar host to the peer configuration items. This permits all registration details that are available to the registration string to be also available to the peer definitions. The users of sip.conf can enable registration by setting register=yes in the peer and do not need to concern themselves with the strange format of the registration strings
> 
> Suggest inclusion in 1.8
> 
> 
> Diffs
> -----
> 
>   trunk/channels/chan_sip.c 268968 
>   trunk/channels/sip/config_parser.c 268968 
>   trunk/channels/sip/include/sip.h 268968 
>   trunk/configs/sip.conf.sample 268968 
> 
> Diff: https://reviewboard.asterisk.org/r/718/diff
> 
> 
> Testing
> -------
> 
> Compile, run and confirm in cli sip show registry
> 
> 
> Thanks,
> 
> Nick
> 
>