> On Aug. 1, 2014, 5:29 p.m., Corey Farrell wrote:
> > /trunk/main/logger.c, line 839
> > <https://reviewboard.asterisk.org/r/3811/diff/2/?file=66020#file66020line839>
> >
> > Is it appropriate / acceptable to document "LogChannel" in two places?
> > Or do the two documentation sections need to be combined?
> >
> > OTOH it feels like these events really should have been called
> > LoggerChannelStart and LoggerChannelStop.
Keep in mind that they are documented in two places because the same event is
raised under multiple conditions. Note the different synopsis elements:
<synopsis>Raised when a logging channel
is re-enabled after a reload operation.</synopsis>
<synopsis>Raised when a logging channel
is disabled.</synopsis>
Additionally, there is an additional field when the LogChannel event is raised
when being disabled (Reason).
The Python scripts combine these into a single event documentation element. You
can see this in the Asterisk 11 event documentation:
https://wiki.asterisk.org/wiki/display/AST/Asterisk+11+ManagerEvent_LogChannel
Whether or not these should be renamed into two separate events is a fair
point. It's certainly the approach we've gone with for the majority of events
in AMI 2.X, and for any new events that are added. However, splitting these up
now would be a breaking change to the API. If we're going to go do that, we
should (a) do it in Asterisk 14, and (b) make sure that we do one comprehensive
change.
> On Aug. 1, 2014, 5: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.
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).
- Matt
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3811/#review12965
-----------------------------------------------------------
On Aug. 1, 2014, 5: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, 5: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
>
>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
