[asterisk-dev] [Code Review]: Move NewCallerid, HangupRequest and SoftHangupRequest to Stasis

[opticron]

> On March 21, 2013, 11:46 a.m., opticron wrote:
> > /team/dlee/json_main/main/manager_channels.c, line 339
> > <https://reviewboard.asterisk.org/r/2405/diff/2/?file=34895#file34895line339>
> >
> >     Are NewCallerid events also expected to be triggered simultaneously with other manager events from the same snapshot?
> 
> David Lee wrote:
>     Yes. I'm not sure if it actually happens (OOM dropped events, maybe), but the code handles it in case it does.

Given the direction that the channel snapshot AMI event handling code is going and that more AMI events will be added to this soon, I'd prefer to handle all channel snapshot AMI events as if they could occur simultaneously (without adding a new variable for each one).  As is, we're handling this two different ways depending on the AMI event to be generated and it will only make this callback more complicated as new AMI events are converted over to Stasis.


- opticron


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


On March 21, 2013, 10:52 a.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2405/
> -----------------------------------------------------------
> 
> (Updated March 21, 2013, 10:52 a.m.)
> 
> 
> Review request for Asterisk Developers, opticron and Matt Jordan.
> 
> 
> Summary
> -------
> 
> This patch builds upon the work in https://reviewboard.asterisk.org/r/2381/.
> 
> This moves the NewCallerid, HangupRequest and SoftHangupRequest events to be
> based off of Stasis events.
> 
> HangupRequest and SoftHangupRequest are now ast_channel_blob Stasis messages,
> with the cause code as an optional field in the blob.
> 
> NewCallerid now simply watches for changes in the callerid information in
> channel snapshots, and creates the AMI event appropriately.
> 
> Since the original NewCallerid event honored the channelvars setting in
> manager.conf, the channel variables configured there had to become a part of the
> channel snapshot. These are now a part of every snapshot based event, making the
> configuration description "every time a channel-oriented event is emitted" less
> of a lie.
> 
> There a a few other changes wrapped up in here as well.
> 
> When ast_channel_topic() is given NULL for a channel, it returns the
> ast_channel_topic_all() topic instead of NULL. This can clean up a lot of NULL
> checking we're doing currently.
> 
> Additionally, the fields Cause and Cause-txt were removed from the base channel
> information and put only on the Hangup events, since those fields are
> meaningless outside of a Hangup event.
> 
> Oh, yeah, and I removed the pipe-delimiter processing of the channelvars field,
> since that's been deprecated forever.
> 
> 
> This addresses bug ASTERISK-21096.
>     https://issues.asterisk.org/jira/browse/ASTERISK-21096
> 
> 
> Diffs
> -----
> 
>   /team/dlee/json_main/CHANGES 383514 
>   /team/dlee/json_main/include/asterisk/channel.h 383514 
>   /team/dlee/json_main/main/channel.c 383514 
>   /team/dlee/json_main/main/channel_internal_api.c 383514 
>   /team/dlee/json_main/main/manager.c 383514 
>   /team/dlee/json_main/main/manager_channels.c 383514 
> 
> Diff: https://reviewboard.asterisk.org/r/2405/diff
> 
> 
> Testing
> -------
> 
> Manually verified events looked compatible with original events.
> 
> 
> Thanks,
> 
> David
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130321/0e9f1635/attachment.htm>