[asterisk-dev] [Code Review] Initial work for res_sip and res_sip_session: Inbound and outbound calls work

[Mark Michelson]

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



/trunk/channels/chan_gulp.c
<https://reviewboard.asterisk.org/r/2285/#comment14738>

    I started thinking about this this morning and I've realized this could be problematic. If chan_gulp handles an incoming SIP request and starts a PBX thread, then other supplements might run afterwards and do manipulations to the channel.
    
    A possible behavior you might see is that the supplement that extracts callerID might run after chan_gulp's supplement. In the PBX thread, though, the person might call the CALLERID() function in order to try to get or set caller ID on the channel. This presents a conflict.
    
    So on the one hand, we could try to make chan_gulp's supplements be the final ones to be registered so the PBX thread is started as the final step.
    
    On the other hand, having a channel created may be useful for supplements on incoming INVITEs. So maybe PBX thread starting should be its own separate supplement that runs last.


- Mark


On Jan. 25, 2013, 12:26 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2285/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2013, 12:26 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This is the first batch of code to review for the new SIP channel driver work. In this, we have the workings of res_sip, the core application-agnostic SIP API, and res_sip_session, the API and workhorse for SIP INVITE sessions. The goal while writing this code was to get into a state where we could run a call through Asterisk using SIP. This code is far from complete, but we reached the milestone of having working incoming and outgoing calls. The calls are currently limited to coming from a hardcoded endpoint and calling out to a hardcoded endpoint, and only ulaw audio is supported.
> 
> When you are reviewing this, the feedback we're interested in is the design. Do you foresee troubles with what is here? Is the documentation of the API clear? What we're not interested in at this point are things that are obviously incomplete and nitpicks.
> 
> Here's a short list of things we know are in need of improvement:
> 1) Any place where there is a XXX or TODO comment. In most cases, these are not necessary for reaching the goal of having working calls or are awaiting for sorcery integration.
> 2) res_sip_endpoint_identifier_constant is not intended to ever actually go into Asterisk. The real endpoint identifiers will use sorcery in order to find endpoints based on data in the SIP request. If the code there is shoddy, it's not a huge deal because it's going to be vaporized eventually.
> 3) The threadpool infrastructure is present, but it currently is not used. There are two reasons why:
>     a) It's easy to add in threadpool usage after the fact, given how this is designed.
>     b) We will need to use a newer version of PJSIP than what currently is in Asterisk.
> 4) The INVITE session state handler is not especially sophisticated. It currently is a switch statement for different event types, which is just the type of thing that could expand to unreasonable size if not kept in check. An array of callbacks either based on INVITE session state or event type would be much better.
> 5) The internal functions in res_sip and res_sip_session do not have doxygen yet. They will, eventually.
> 6) chan_gulp is a working name. It is not necessarily going to be the permanent name for the channel driver.
> 7) So far, every file that uses PJSIP has required an additional line to be added to the Makefile so that the PJ_FLAGS are added to the _AST_CFLAGS. This probably should be automated in some way so that we do not have to keep adding lines to the Makefile for every file we add.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/Makefile 379431 
>   /trunk/channels/chan_gulp.c PRE-CREATION 
>   /trunk/include/asterisk/res_sip.h PRE-CREATION 
>   /trunk/include/asterisk/res_sip_session.h PRE-CREATION 
>   /trunk/main/taskprocessor.c 379432 
>   /trunk/res/Makefile 379431 
>   /trunk/res/pjproject/Makefile 379431 
>   /trunk/res/pjproject/build.mak.in 379431 
>   /trunk/res/pjproject/pjmedia/build/Makefile 379431 
>   /trunk/res/pjproject/pjsip/build/Makefile 379431 
>   /trunk/res/res_sip.c PRE-CREATION 
>   /trunk/res/res_sip.exports.in PRE-CREATION 
>   /trunk/res/res_sip_endpoint_identifier_constant.c PRE-CREATION 
>   /trunk/res/res_sip_sdp_audio.c PRE-CREATION 
>   /trunk/res/res_sip_session.c PRE-CREATION 
>   /trunk/res/res_sip_session.exports.in PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/2285/diff
> 
> 
> Testing
> -------
> 
> Joshua and I have been sending calls to Asterisk and receiving calls from Asterisk using chan_gulp. We have also tested error scenarios like calling into a nonexistent extension. 
> 
> 
> Thanks,
> 
> Mark
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130126/57cf042c/attachment.htm>