[asterisk-dev] [Code Review]: Rewrite skinny dialing to remove threaded simpleswitch

[wedhorn]

> On Dec. 13, 2012, 12:19 p.m., opticron wrote:
> > Unrelated minor changes or tweaks should probably be committed so as to avoid scope creep in this review.

Yep, first four comments below have now been committed and are not part of the review.


> On Dec. 13, 2012, 12:19 p.m., opticron wrote:
> > /branches/11/channels/chan_skinny.c, line 4407
> > <https://reviewboard.asterisk.org/r/2240/diff/6/?file=32374#file32374line4407>
> >
> >     This should probably be addressed before commit via removal of the branch or ensuring that the tone is heard.

I have left this in the new patch, but happy to remove the comment.

Sending the reorder tone is correct. The issue is a more general on in that skinny hangs up the device when a channel is hung up which results in all these notifications that should be heard being cut off. I do intend to fix the hangup issue which will resolve this, but a fair bit of work will be required (ie splitting tech_pvt from chan and keeping it around for a bit longer).

As such I really want to keep the code because it is correct, just some later code doesn't allow it to function (from the users perspective) as it should.


> On Dec. 13, 2012, 12:19 p.m., opticron wrote:
> > /branches/11/channels/chan_skinny.c, line 5712
> > <https://reviewboard.asterisk.org/r/2240/diff/6/?file=32374#file32374line5712>
> >
> >     Remove this.

Done.


- wedhorn


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


On Dec. 13, 2012, 3:54 p.m., wedhorn wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2240/
> -----------------------------------------------------------
> 
> (Updated Dec. 13, 2012, 3:54 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This rewrite changes skinny dialing from the threaded simpleswitch to a scheduled timeout approach. There were some underlying issues with the threaded simple switch with occasional corruption and possible segfaults.
> 
> 
> Diffs
> -----
> 
>   /branches/11/channels/chan_skinny.c 377991 
> 
> Diff: https://reviewboard.asterisk.org/r/2240/diff
> 
> 
> Testing
> -------
> 
> Lots of dialing. Incomplete numbers timeout, complete numbers dial and behave as expected.
> 
> 
> Thanks,
> 
> wedhorn
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20121213/fbb08ef7/attachment-0001.htm>