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

[David Lee]

> On March 21, 2013, 4:27 p.m., opticron wrote:
> > /team/dlee/json_main/main/manager_channels.c, line 461
> > <https://reviewboard.asterisk.org/r/2405/diff/3/?file=34923#file34923line461>
> >
> >     Use ARRAY_SIZE if possible.

I couldn't find ARRAY_SIZE used anywhere else in our code, so I didn't want to rock the boat.

Looks like we have ARRAY_LEN defined in utils.h instead. Changing.


> On March 21, 2013, 4:27 p.m., opticron wrote:
> > /team/dlee/json_main/main/manager_channels.c, line 269
> > <https://reviewboard.asterisk.org/r/2405/diff/3/?file=34923#file34923line269>
> >
> >     The way that this is being used, there is no reason to have it be an AO2 object. 
> >     
> >     Alternatively, you could take advantage of the destructor to free stringfields which you could use to handle the format string in place of ast_vasprintf and avoid the extra allocation and string shuffling.

Stringfields, FTW!


> On March 21, 2013, 4:27 p.m., opticron wrote:
> > /team/dlee/json_main/main/manager_channels.c, line 263
> > <https://reviewboard.asterisk.org/r/2405/diff/3/?file=34923#file34923line263>
> >
> >     Missing va_end.

fixed


> On March 21, 2013, 4:27 p.m., opticron wrote:
> > /team/dlee/json_main/main/manager_channels.c, line 233
> > <https://reviewboard.asterisk.org/r/2405/diff/3/?file=34923#file34923line233>
> >
> >     This doesn't need a list entry.

removed


- David


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


On March 21, 2013, 3:12 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2405/
> -----------------------------------------------------------
> 
> (Updated March 21, 2013, 3:12 p.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 383541 
>   /team/dlee/json_main/include/asterisk/channel.h 383541 
>   /team/dlee/json_main/main/channel.c 383541 
>   /team/dlee/json_main/main/channel_internal_api.c 383541 
>   /team/dlee/json_main/main/manager.c 383541 
>   /team/dlee/json_main/main/manager_channels.c 383541 
> 
> 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/20130322/4f07cdf7/attachment.htm>