[asterisk-dev] [Code Review] 2588: features: call pickup stasis refactoring

[jrose]

> On June 5, 2013, 2:02 p.m., opticron wrote:
> > /trunk/main/features.c, line 8305
> > <https://reviewboard.asterisk.org/r/2588/diff/1/?file=39014#file39014line8305>
> >
> >     This should be an ast_register_cleanup, not an atexit (registered at the end of features_stasis_init).

I think I got this right anyway.


> On June 5, 2013, 2:02 p.m., opticron wrote:
> > /trunk/main/features.c, lines 7167-7169
> > <https://reviewboard.asterisk.org/r/2588/diff/1/?file=39014#file39014line7167>
> >
> >     ast_json_null() is non-refcounted singleton and can be passed directly into the blob creation function.

Oh, neat.


> On June 5, 2013, 2:02 p.m., opticron wrote:
> > /trunk/main/features.c, lines 7139-7142
> > <https://reviewboard.asterisk.org/r/2588/diff/1/?file=39014#file39014line7139>
> >
> >     If this is an expected condition, remove the assert, otherwise:
> >     
> >     ast_assert(chan != NULL && target != NULL);

It's unexpected, but I wanted to be able to fail gracefully if it happened. I guess that's not the right way to do things though.
Changed.


- jrose


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


On June 3, 2013, 10:33 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2588/
> -----------------------------------------------------------
> 
> (Updated June 3, 2013, 10:33 p.m.)
> 
> 
> Review request for Asterisk Developers, kmoore, Matt Jordan, and rmudgett.
> 
> 
> Bugs: ASTERISK-21544
>     https://issues.asterisk.org/jira/browse/ASTERISK-21544
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> I verified that all the changes to bridging didn't break pickupexten and pickupfailsound since they had nothing to do with that. pickupsound is currently broken and this patch doesn't address that since Richard's https://reviewboard.asterisk.org/r/2582/ fixes that.
> 
> This patch provides a message type sent out to the channel's topic when performing the pickup. The pickup AMI event is now sent out in response to this message type.
> 
> The fields for the AMI event are still compatible with what they were before. It just adds the rest of the channel snapshot information for the Channel and TargetChannel.
> 
> I think the masquerade being performed here already isn't doing anything to inform stasis core that it's happening and there aren't any changes needed there. If I'm wrong on that, feel free to assail me with wassail.
> 
> I still need to write Testsuite tests for call pickup, so that's coming next.
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/features.h 390315 
>   /trunk/main/features.c 390315 
> 
> Diff: https://reviewboard.asterisk.org/r/2588/diff/
> 
> 
> Testing
> -------
> 
> Made sure everything pickup still works or is fixed by other stuff.
> 
> 
> Thanks,
> 
> jrose
> 
>

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