[asterisk-dev] [Code Review] 3129: chan_pjsip: Provide a means for updating device state when going on/off hold

[Mark Michelson]

> On Jan. 16, 2014, 6:29 p.m., Jonathan Rose wrote:
> > /branches/12/channels/chan_pjsip.c, line 1042
> > <https://reviewboard.asterisk.org/r/3129/diff/1/?file=52936#file52936line1042>
> >
> >     I used a magic number here because I couldn't find an appropriate definition for device name lengths. If one is known, I'll be happy to update this.

Well, the device name will be shorter than the channel's name, so you could use alloca and set the device_buf to strlen(ast_channel_name(chan)). You can also limit the alloca() to be called for the appropriate control frames to reduce unnecessary stack growth.


> On Jan. 16, 2014, 6:29 p.m., Jonathan Rose wrote:
> > /branches/12/channels/chan_pjsip.c, lines 2051-2057
> > <https://reviewboard.asterisk.org/r/3129/diff/1/?file=52936#file52936line2051>
> >
> >     Not really sure how necessary the RWLOCK is for this container. It makes sense to use it since there are polling actions that could take place synchronously and modification actions that probably shouldn't, but I'm not sure.

This should be fine. The operations to link and unlink will write-lock and the operations to find will read-lock. 


- Mark


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


On Jan. 16, 2014, 6:26 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3129/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2014, 6:26 p.m.)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Maintains a list of on hold channel uniqueids.  When a snapshot is used with chan_pjsip's device state evaluating function, this list is queried to see if the channel in the snapshot is on hold. As the channel receives indications of HOLD/UNHOLD, this list is updated and the device state is updated. When the session ends, the session channel is removed from the list if it is still in it.
> 
> 
> Diffs
> -----
> 
>   /branches/12/channels/chan_pjsip.c 405641 
> 
> Diff: https://reviewboard.asterisk.org/r/3129/diff/
> 
> 
> Testing
> -------
> 
> Manual testing where I watched the device state with a websocket, polled for device state with Manager command GetVar and DEVICE_STATE function, and also modified an automated test (channels/pjsip/hold) to have a hint which checks the device state of a PJSIP channel and ExtensionStatus event is monitored to evaluate device state changes as they had.
> 
> The test modifications will be in another review.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140116/745e3c17/attachment-0001.html>