[asterisk-dev] [Code Review] Merge Called party identification changes into trunk.

[Mark Michelson]

> On 2009-03-23 15:04:35, Russell Bryant wrote:
> > /trunk/include/asterisk/channel.h, lines 2215-2235
> > <http://reviewboard.digium.com/r/201/diff/6/?file=3532#file3532line2215>
> >
> >     As a general comment, I think we should try to be consistent in the way we name new API calls.  I know that our existing code is not very consistent in this area, but we should make an effort going forward.
> >     
> >     In the quoted section, there are API calls that are of both ast_<API>_<action> and ast_<action>_<API>.  Personally, I prefer the ast_<API>_<action> variant.
> >     
> >     So, in this example, I like ast_connected line_update().  I would like to see things like ast_parse_connected_line_data() and ast_queue_connected_line_update() changed to be more consistent.
> >     
> >     This is an area that we need to address in our coding guidelines, I think.

I'll get this first thing tomorrow. Assuming Richard knocks out the chan_misdn items from the previous set of comments. I'll also post a new diff then, too.


> On 2009-03-23 15:04:35, Russell Bryant wrote:
> > /trunk/channels/chan_sip.c, lines 20061-20067
> > <http://reviewboard.digium.com/r/201/diff/6/?file=3518#file3518line20061>
> >
> >     This code assumes current->chan2 and target.chan2 are locked.  It doesn't look like this is actually true.

Got it. Fixed!


> On 2009-03-23 15:04:35, Russell Bryant wrote:
> > /trunk/channels/chan_sip.c, lines 20083-20085
> > <http://reviewboard.digium.com/r/201/diff/6/?file=3518#file3518line20083>
> >
> >     This code assumes target.chan1 is locked, but it does not appear to be.

I think I've got this one too.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/201/#review598
-----------------------------------------------------------


On 2009-03-23 09:52:15, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/201/
> -----------------------------------------------------------
> 
> (Updated 2009-03-23 09:52:15)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This diff encompasses the changes between the issue8824 branch and Asterisk trunk. These changes include all the changes necessary to allow for the transmission and reception of remote called party identification (COLP/CONP).
> 
> 
> This addresses bug 8824.
>     http://bugs.digium.com/view.php?id=8824
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_dial.c 183691 
>   /trunk/apps/app_directed_pickup.c 183691 
>   /trunk/apps/app_queue.c 183691 
>   /trunk/channels/chan_agent.c 183691 
>   /trunk/channels/chan_dahdi.c 183691 
>   /trunk/channels/chan_h323.c 183691 
>   /trunk/channels/chan_iax2.c 183691 
>   /trunk/channels/chan_local.c 183691 
>   /trunk/channels/chan_mgcp.c 183691 
>   /trunk/channels/chan_misdn.c 183691 
>   /trunk/channels/chan_phone.c 183691 
>   /trunk/channels/chan_sip.c 183691 
>   /trunk/channels/chan_skinny.c 183691 
>   /trunk/channels/chan_unistim.c 183691 
>   /trunk/channels/misdn/chan_misdn_config.h 183691 
>   /trunk/channels/misdn/isdn_lib.h 183691 
>   /trunk/channels/misdn/isdn_lib.c 183691 
>   /trunk/channels/misdn/isdn_lib_intern.h 183691 
>   /trunk/channels/misdn/isdn_msg_parser.c 183691 
>   /trunk/channels/misdn_config.c 183691 
>   /trunk/configs/misdn.conf.sample 183691 
>   /trunk/configs/sip.conf.sample 183691 
>   /trunk/funcs/func_connectedline.c PRE-CREATION 
>   /trunk/funcs/func_redirecting.c PRE-CREATION 
>   /trunk/include/asterisk/callerid.h 183691 
>   /trunk/include/asterisk/channel.h 183691 
>   /trunk/include/asterisk/frame.h 183691 
>   /trunk/main/callerid.c 183691 
>   /trunk/main/channel.c 183691 
>   /trunk/main/dial.c 183691 
>   /trunk/main/features.c 183691 
>   /trunk/main/rtp.c 183691 
> 
> Diff: http://reviewboard.digium.com/r/201/diff
> 
> 
> Testing
> -------
> 
> Digium's Product Quality department has extensively tested a remarkably similar version of these enhancements. In addition, we know of a customer who has been using this branch for some months now in a production environment. I would also be willing to bet that some of those who have been monitoring issue 8824 have also done some testing as well.
> 
> 
> Thanks,
> 
> Mark
> 
>