[asterisk-dev] [Code Review]: Move Newexten and UserEvent AMI events to Stasis; adding JSON messages to stasis-core

[rmudgett]

> On March 14, 2013, 11:52 a.m., opticron wrote:
> > /trunk/main/manager_channels.c, line 95
> > <https://reviewboard.asterisk.org/r/2381/diff/3/?file=34483#file34483line95>
> >
> >     This should probably use manager_build_channel_state_string since it's a message being generated from a channel snapshot and it should be processed like the other channel state AMI updates.
> 
> David Lee wrote:
>     Newexten has 'Extension' instead of 'Exten'. Is it really worth making that change?
>     
>     Newexten also has 'Application' and 'AppData' fields, which aren't currently in the channel_state_string. Should we add it for all events that use that function, or just optionally for Newexten?
> 
> opticron wrote:
>     For Application and AppData, that will depend on whether we consider them critical channel information.  If not, it can use the same mechanism as varset and userevent to add non-standard headers to the channel state event.
>     
>     As for Exten vs Extension, the standard channel event information needs to be consistent across channel state AMI events and I'd go with Extension since there isn't similar abbreviation elsewhere in the message.  If we want to be slightly more backwards compatible, the Newchannel event would need an additional/redundant Exten header, but I'd prefer to change it and document that it was changed.

Asterisk has historically used exten when it is referring to context/exten/priority.  I seem to recall that exten is a lot in the dialplan. :)


- rmudgett


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


On March 15, 2013, 6:39 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2381/
> -----------------------------------------------------------
> 
> (Updated March 15, 2013, 6:39 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch started out simply as fixing the bouncing tests introduced
> in r382685, but required some other changes to give it a decent
> implementation.
> 
> To fix the bouncing tests, the UserEvent and Newexten AMI events
> needed to be refactored to dispatch via Stasis. Dispatching directly
> to AMI resulted in those events sometimes getting ahead of the
> associated Newchannel events, which would understandably confuse anyone.
> 
> I found that instead of creating a zillion different message types and
> structures associated with them, it would be preferable to define a
> message type that has a channel snapshot and a blob of structured data
> with a small bit of additional information. The JSON object model
> provides a very nice way of representing structured data, so I went
> with that.
> 
>  * Move JSON support from res_json.c to main/json.c
>    * Made libjansson-dev a required dependency
>  * Added an ast_channel_blob message type, which has a channel
>    snapshot and JSON blob of data.
>  * Changed UserEvent and Newexten events so that they are dispatched
>    via ast_channel_blob messages on the channel's topic.
>  * Got rid of the ast_channel_varset message; used ast_channel_blob
>    instead.
>  * Extracted the manager functions converting Stasis channel events to
>    AMI events into manager_channel.c.
> 
> 
> This addresses bug ASTERISK-21096.
>     https://issues.asterisk.org/jira/browse/ASTERISK-21096
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 383261 
>   /trunk/apps/app_userevent.c 383261 
>   /trunk/configure UNKNOWN 
>   /trunk/configure.ac 383261 
>   /trunk/include/asterisk/autoconfig.h.in 383261 
>   /trunk/include/asterisk/channel.h 383261 
>   /trunk/include/asterisk/manager.h 383261 
>   /trunk/main/channel.c 383261 
>   /trunk/main/json.c PRE-CREATION 
>   /trunk/main/manager.c 383261 
>   /trunk/main/manager_channels.c PRE-CREATION 
>   /trunk/main/pbx.c 383261 
>   /trunk/pbx/pbx_realtime.c 383261 
>   /trunk/res/res_json.c 383261 
>   /trunk/res/res_json.exports.in 383261 
>   /trunk/tests/test_json.c 383261 
> 
> Diff: https://reviewboard.asterisk.org/r/2381/diff
> 
> 
> Testing
> -------
> 
> Ran bouncing testsuite tests 10 times without failure.
> 
> 
> Thanks,
> 
> David
> 
>

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