[asterisk-dev] [Code Review] 3811: Move main/manager_*.c to loadable modules

[Corey Farrell]

> On Aug. 1, 2014, 6:29 p.m., Corey Farrell wrote:
> > /trunk/res/res_manager_channels.c, lines 253-261
> > <https://reviewboard.asterisk.org/r/3811/diff/2/?file=66030#file66030line253>
> >
> >     channel_state_change has a comment stating that Newchannel, Newstate and Hangup events are mutually exclusive.  I think this may also apply to Newexten, NewCallerID and NewAccountcode.  If so this should be moved back to stasis_channels.c as a .to_ami callback.
> 
> Matt Jordan wrote:
>     Hm. These are a bit different, in that the events are inferred from the delta between two snapshots. Generally, a format handler on a Stasis message type is a bit more straight forward: there is a one to one relationship between the message type and the format you want to convert to. A .to_ami handler for cached messages may be possible, but I'm not sure how necessary it is since there is a one to many relationship with the cache message changes and the events it creates (and, in fact, there are times when a change in channel state does not generate an AMI event).

A .to_ami can always return NULL.  I realize most stasis message type handlers are very simple, but that's not a rule.  From what I can tell the only rule for .to_ami is that one stasis message produces zero or one AMI messages.  I bring this up since we've decided that .to_ami is the "best" way to go.  At this point I have no objection to leaving these as is.


- Corey


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


On Aug. 1, 2014, 6:22 p.m., Corey Farrell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3811/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2014, 6:22 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24068
>     https://issues.asterisk.org/jira/browse/ASTERISK-24068
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This change moves main/manager_*.c to loadable modules, allowing those events to be disabled by not loading the modules.  This can be accomplished by eventfilter, but eventfilter has a couple issues.  It actually adds more overhead to asterisk since the outbound events must be parsed for each AMI user.  Additionally it causes skips in SequenceNumber, preventing use of that tag to determine if any events were missed during a reconnect.
> 
> Besides converting from built-in units to modules, changes are made to VarSet, ChannelTalkingStart and ChannelTalkingStop.  They no longer use .to_ami callbacks, but instead subscribe to the stasis events like the rest of the res_manager_channels events.  A couple functions were also moved from manager_bridging.c and manager_channels.c to manager.c since they are still needed even if these modules are noload'ed.
> 
> AST_MODULE_INFO_STANDARD for all modules will be updated once r3802 is committed.  This or r3812 will need to be updated depending on which is committed first.
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_manager_system.c PRE-CREATION 
>   /trunk/res/res_manager_mwi.c PRE-CREATION 
>   /trunk/res/res_manager_endpoints.c PRE-CREATION 
>   /trunk/res/res_manager_channels.c PRE-CREATION 
>   /trunk/res/res_manager_bridges.c PRE-CREATION 
>   /trunk/main/stasis_channels.c 419804 
>   /trunk/main/stasis_bridges.c 419804 
>   /trunk/main/manager_system.c 419804 
>   /trunk/main/manager_mwi.c 419804 
>   /trunk/main/manager_endpoints.c 419804 
>   /trunk/main/manager_channels.c 419804 
>   /trunk/main/manager_bridges.c 419804 
>   /trunk/main/manager.c 419804 
>   /trunk/main/logger.c 419804 
>   /trunk/main/channel.c 419804 
>   /trunk/include/asterisk/manager.h 419804 
>   /trunk/build_tools/get_documentation 419804 
> 
> Diff: https://reviewboard.asterisk.org/r/3811/diff/
> 
> 
> Testing
> -------
> 
> Ran some testsuite's to verify some of the events were still being sent to AMI:
> tests/manager/originate
> tests/apps/channel_redirect
> tests/bridge/connected_line_update
> tests/feature_call_pickup
> tests/apps/dial/dial_answer
> tests/apps/chanspy/chanspy_barge
> tests/funcs/func_push
> 
> This did not provide complete coverage for all effected events, but does verify many events from res_manager_channels.c.  Events from other files were not tested, though res_manager_channels.c was the most likely to cause problems.
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140806/c32d7c90/attachment.html>