[asterisk-dev] [Code Review]: Add AMI event documentation

Matt Jordan reviewboard at asterisk.org
Mon Jun 11 09:08:53 CDT 2012 


> On June 10, 2012, 8:20 a.m., Russell Bryant wrote:
> > ./trunk/Makefile, line 318
> > <https://reviewboard.asterisk.org/r/1967/diff/2/?file=28640#file28640line318>
> >
> >     Is 'make full' so python isn't a build dependency?  Or is it to avoid the additional time in the build?

Yes :-)


> On June 10, 2012, 8:20 a.m., Russell Bryant wrote:
> > ./trunk/build_tools/get_documentation.py, line 1
> > <https://reviewboard.asterisk.org/r/1967/diff/2/?file=28649#file28649line1>
> >
> >     If you care about making the formatting conform to PEP8:
> >     
> >     
> >     $ pep8 build_tools/get_documentation.py 
> >     build_tools/get_documentation.py:17:1: E302 expected 2 blank lines, found 1
> >     build_tools/get_documentation.py:26:1: E302 expected 2 blank lines, found 1
> >     build_tools/get_documentation.py:48:18: E702 multiple statements on one line (semicolon)
> >     build_tools/get_documentation.py:57:80: E501 line too long (104 characters)
> >     build_tools/get_documentation.py:57:67: E231 missing whitespace after ','
> >     build_tools/get_documentation.py:79:80: E501 line too long (118 characters)
> >     build_tools/get_documentation.py:80:66: E231 missing whitespace after ','
> >     build_tools/get_documentation.py:108:52: E231 missing whitespace after ','
> >     build_tools/get_documentation.py:109:38: W291 trailing whitespace
> >     build_tools/get_documentation.py:110:80: E501 line too long (81 characters)
> >     build_tools/get_documentation.py:111:80: E501 line too long (91 characters)
> >     build_tools/get_documentation.py:116:80: E501 line too long (120 characters)
> >     build_tools/get_documentation.py:118:1: E302 expected 2 blank lines, found 1
> >     build_tools/get_documentation.py:160:26: W292 no newline at end of file
> >

I do, fixed


> On June 10, 2012, 8:20 a.m., Russell Bryant wrote:
> > ./trunk/build_tools/get_documentation.py, lines 42-43
> > <https://reviewboard.asterisk.org/r/1967/diff/2/?file=28649#file28649line42>
> >
> >     You can also do:
> >     
> >         for line in sys.stdin:

Much better, fixed


> On June 10, 2012, 8:20 a.m., Russell Bryant wrote:
> > ./trunk/build_tools/get_documentation.py, line 45
> > <https://reviewboard.asterisk.org/r/1967/diff/2/?file=28649#file28649line45>
> >
> >     Did you mean if len(line) here?
> >     
> >     If line was None, an Exception would occur on the line before this one.

Yes, fixed


> On June 10, 2012, 8:20 a.m., Russell Bryant wrote:
> > ./trunk/build_tools/get_documentation.py, line 103
> > <https://reviewboard.asterisk.org/r/1967/diff/2/?file=28649#file28649line103>
> >
> >     Did you mean if not len(parameter) ?

Yes, fixed


> On June 10, 2012, 8:20 a.m., Russell Bryant wrote:
> > ./trunk/build_tools/post_process_documentation.py, line 1
> > <https://reviewboard.asterisk.org/r/1967/diff/2/?file=28650#file28650line1>
> >
> >     If you care about PEP8:
> >     
> >     
> >     $ pep8 build_tools/post_process_documentation.py 
> >     build_tools/post_process_documentation.py:18:1: E302 expected 2 blank lines, found 1
> >     build_tools/post_process_documentation.py:26:80: E501 line too long (89 characters)
> >     build_tools/post_process_documentation.py:29:80: E501 line too long (95 characters)
> >     build_tools/post_process_documentation.py:32:80: E501 line too long (117 characters)
> >     build_tools/post_process_documentation.py:36:80: E501 line too long (97 characters)
> >     build_tools/post_process_documentation.py:38:80: E501 line too long (98 characters)
> >     build_tools/post_process_documentation.py:40:1: E302 expected 2 blank lines, found 1
> >     build_tools/post_process_documentation.py:47:1: E302 expected 2 blank lines, found 1
> >     build_tools/post_process_documentation.py:65:1: E302 expected 2 blank lines, found 1
> >     build_tools/post_process_documentation.py:71:44: E251 no spaces around keyword / parameter equals
> >     build_tools/post_process_documentation.py:74:45: E251 no spaces around keyword / parameter equals
> >     build_tools/post_process_documentation.py:93:26: W292 no newline at end of file
> >

I do, fixed


> On June 10, 2012, 8:20 a.m., Russell Bryant wrote:
> > ./trunk/build_tools/post_process_documentation.py, line 38
> > <https://reviewboard.asterisk.org/r/1967/diff/2/?file=28650#file28650line38>
> >
> >     Holy nesting depth, batman

Reduced to a manageable level.


> On June 10, 2012, 8:20 a.m., Russell Bryant wrote:
> > ./trunk/Makefile, line 507
> > <https://reviewboard.asterisk.org/r/1967/diff/2/?file=28640#file28640line507>
> >
> >     It would be good to update validate-docs, too

Well, I'm hoping I don't have to.  doc/full-en_US.xml actually produces two things: full-en_US.xml and core-en_US.xml.  The extraction from the source produces full-en_US.xml, while the post-processing script (should) produce a DTD valid core-en_US.xml.  So the same validation make target should work for both.  (full-en_US.xml has no guarantee about having a valid schema, as its a 'temporary' artifact)

Now, if you run validate-docs before you run doc/full-en_US.xml, there's no promise on what it'll try to validate - an XML file produced from 'make' or an XML file produced from 'make full' - but that's not terribly surprising.

I went with the approach of having 'make' and 'make full' produce the same XML artifact so that I didn't have to update all of the internal references to core-en_US.xml (or core-*.xml in general).  If you think that a different mechanism should be used, let me know.


- Matt


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


On June 8, 2012, 8:33 a.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1967/
> -----------------------------------------------------------
> 
> (Updated June 8, 2012, 8:33 a.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson and wdoekes.
> 
> 
> Summary
> -------
> 
> This patch begins to add AMI event documentation to Asterisk.  This patch includes the following:
> 
> * Pulling AMI event XML documentation out of the Asterisk source and into the XML documentation produced by the build tools
> * Validating the XML when -dev-mode is enabled
> * Reading/parsing the XML documentation when Asterisk starts
> * Two new CLI commands: 'manager show events' and 'manager show event [event_name]'
> * Event documentation for the core applications
> 
> So... AMI events are a bit trickier then other things we've attempted to document in the past.  There were a number of factors to consider when approaching this:
> 1. There are a lot of AMI events, and they are located throughout the code base
> 2. AMI events can occur multiple times with different parameters, but still be the same 'event' type
> 3. AMI events can occur multiple times, in different implementation files
> 4. There is a significant amount of repeated information between events
> 5. Much of the information that a person cares about (AMI event keys) are - more often then not - easily obtained from a method call
> 
> To that end, documentation for AMI events is treated a bit differently then other items.  There are a few major shifts in how the documentation is extracted:
> 1. The previous documentation extraction used an awk script that pulled the documentation from a single comment block (delineated by /*** DOCUMENTATION ... ***/).  That still exists; however, if you want AMI event documentation, you have to use 'make full'.  This calls a different documentation extraction script, written in python.  That script lets us do a few things that the awk script did not:
>  * Documentation can now exist anywhere in the source file
>  * The documentation for manager events can be co-located with the method calls that raise the event.  If the method call is recognized, event parameters are automatically read from the method call and populated in the documentation
>  * Documentation is combined where appropriate.  Thus, if the same event is raised in multiple places, the parameters only need to be defined once.
> 2. Previously, documentation was registered with the application/function/command that it was associated with.  Events are never registered with any controller; requiring them to do so would incur a run-time performance penalty for little gain.  Hence, the xmldoc core now allow documentation for an entire source type to be requested at run-time.  When requested, the documentation is built from the XML read in.
> 
> Example manager events:
> 
> Event: ChanSpyStart
> [Synopsis]
> Raised when a channel has started spying on another channel.
> 
> [Syntax]
> Event: ChanSpyStart
> SpyerChannel: <value>
> SpyeeChannel: <value>
> 
> [See Also]
> ChanSpy(), ExtenSpy(), ChanSpyStop
> 
> -----------------------
> 
> Event: Dial
> [Synopsis]
> Raised when a dial action has started.
> 
> [Syntax]
> Event: Dial
> SubEvent: <value>
> Channel: <value>
> Destination: <value>
> CallerIDNum: <value>
> CallerIDName: <value>
> ConnectedLineNum: <value>
> ConnectedLineName: <value>
> UniqueID: <value>
> DestUniqueID: <value>
> Dialstring: <value>
> 
> [Arguments]
> SubEvent
>     Begin
>     End
>     A sub event type, specifying whether or not the dial action has begun
>     or ended.
> 
> [Synopsis]
> Raised when a dial action has ended.
> 
> [Syntax]
> Event: Dial
> DialStatus: <value>
> SubEvent: <value>
> Channel: <value>
> UniqueID: <value>
> 
> [Arguments]
> DialStatus
>     The value of the ${DIALSTATUS} channel variable.
> SubEvent
>     Begin
>     End
>     A sub event type, specifying whether or not the dial action has begun
>     or ended.
> 
> ---------------------------------
> 
> Event: QueueMemberStatus
> [Synopsis]
> Raised when a Queue member's status has changed.
> 
> [Syntax]
> Event: QueueMemberStatus
> Queue: <value>
> Location: <value>
> MemberName: <value>
> StateInterface: <value>
> Membership: <value>
> Penalty: <value>
> CallsTaken: <value>
> LastCall: <value>
> Status: <value>
> Paused: <value>
> 
> [Arguments]
> Queue
>     The name of the queue.
> Location
>     The queue member's channel technology or location.
> MemberName
>     The name of the queue member.
> StateInterface
>     Channel technology or location from which to read device state
>     changes.
> Membership
>     dynamic
>     realtime
>     static
> Penalty
>     The penalty associated with the queue member.
> CallsTaken
>     The number of calls this queue member has serviced.
> LastCall
>     The time this member last took call, expressed in seconds since 00:00,
>     Jan 1, 1970 UTC.
> Status
>     The status of the queue member.  This will be a device state value.
> Paused
>     0
>     1
> 
> 
> Diffs
> -----
> 
>   ./configure UNKNOWN 
>   ./trunk/Makefile 367982 
>   ./trunk/apps/app_chanspy.c 367982 
>   ./trunk/apps/app_confbridge.c 367982 
>   ./trunk/apps/app_dial.c 367982 
>   ./trunk/apps/app_meetme.c 367982 
>   ./trunk/apps/app_queue.c 367982 
>   ./trunk/apps/app_stack.c 367982 
>   ./trunk/apps/app_userevent.c 367982 
>   ./trunk/apps/app_voicemail.c 367982 
>   ./trunk/build_tools/get_documentation.py PRE-CREATION 
>   ./trunk/build_tools/post_process_documentation.py PRE-CREATION 
>   ./trunk/configure.ac 367982 
>   ./trunk/doc/appdocsxml.dtd 367982 
>   ./trunk/include/asterisk/xmldoc.h 367982 
>   ./trunk/main/manager.c 367982 
>   ./trunk/main/xmldoc.c 367982 
>   ./trunk/makeopts.in 367982 
> 
> Diff: https://reviewboard.asterisk.org/r/1967/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120611/3075f168/attachment-0001.htm>

More information about the asterisk-dev mailing list