|Summary:||ASTERISK-15957: [patch] Updates to Application Documentation|
|Reporter:||David Chappell (chappell)||Labels:|
|Date Opened:||2010-04-14 11:52:21||Date Closed:||2012-01-28 11:55:54.000-0600|
|Environment:||Attachments:||( 0) core_doc_20100414.diff|
( 1) core_docs-220.127.116.11-20100416.diff
( 2) core_docs-trunk-20100416.diff
|Description:||Everybody knows that programmers would rather code than write documentation. As a result, much of the documentation for Asterisk applications, functions, etc., is either skeletal or written from the perspective of an Asterisk programmer rather than that of a dialplan author.|
This patch attempts to rewrite the documentation for the core applications in order to make it more precise and informative.
****** STEPS TO REPRODUCE ******
1) At CLI enter "core show application congestion"
2) Using only the information provided, answer the following questions:
Q) What is congestion?
Q) Will the "calling channel" (whatever that means) inform the calling party's phone of the "congestion" or does this just twidle some internal state of the channel driver?
Q) What is likely to happen if Congestion is executed?
Q) Why would one want to do this?
Q) Is there any suggestion that the likely effect of executing this application is that the caller will hear an annoying noise?
****** ADDITIONAL INFORMATION ******
I have been writing Asterisk dialplans and hacking the source code for several years now. I have frequently been frustrated by incomplete or confusing documentation. By trial and error and reading the source code I have figured out a good chunk of the missing information. I would now like to share what I have learned with others by working on the internal documentation.
As I went through rewriting the documentation for these applications I realized that there are still things I do not know about even these core components. As a result, I have had to leave these points vague. For example:
* Answer() has a parameter called delay. The documentation suggests that this is an unconditional delay like that imposed by Wait(), but my reading of the code suggest that it is actually a timeout which specifies how long Answer() should wait for the audio channel to come up.
* What is the purpose of the s option to BackGround()? (I am not asking what it does. I am asking why such a feature is needed.)
* When does a channel enter the "up" state mentioned in the description of Background()? Certainly when Answer() is called, but what if Progress() is called?
* What is the purpose of the nocdr option to Answer()? It sounds like it enables us to answer a call while letting the CDR show that it is still ringing. Why would we want to do that? Can we later call Answer a second time in order to change the call state in CDR? Is this a hack? If so, for what?
* What is the format for the time specification required by GotoIfTime and ExecIfTime?
I am willing to do more rewriting work and read the code to answer my own questions, but I wouldn't mind having a mentor who could tell me if my conclusions are correct or not and who could shed light on mysterious features.
|Comments:||By: Paul Belanger (pabelanger) 2010-04-14 12:24:57|
Did you forget to attach your patch?
By: Leif Madsen (lmadsen) 2010-04-14 12:47:19
First off, I want to say thank you for whatever (if anything!) you will submit. I'm willing to work with you as closely as possible to help advance this documentation because you're certainly right that some of the question you've asked are not answered by the documentation. This could be a great first start in getting the documentation of dialplan applications updated to something beyond 1999 :)
I don't want to volunteer anyone in particular, but I think Tilghman would be a great mentor to this effort. However, I know he is extremely busy, so I think we'll just need to employ our collective minds and get someone else to sign off on the documentation with a, "yep, makes sense to me!"
This issue has actually gotten me pretty excited :) With the amount of work pabelanger has been doing on writing documentation lately, I'm sure he'll be a fine candidate to help review and move this forward.
Question: is there a particular reason this issue is marked as private? I'm guessing that was just a mistake.
Whenever you can get your patch uploaded, I can start reviewing it. I think the important thing about this project will be more of what questions need to be asked, and then we can go out and get the answers. The questions will be the hard part; getting answers should be significantly easier I hope (I mean, it's just code right? :))
By: Leif Madsen (lmadsen) 2010-04-14 12:47:34
Setting to feedback upon answer from the reporter.
By: David Chappell (chappell) 2010-04-14 15:02:36
I marked the bug as private in the mistaken belief that it would not be visible to other users until I made it public. I did that because I had not yet attached the patch. I now vaguely recall that private might be intended to mean that the bug report contains confidential information which should be available only to trusted developers. If someone could make the bug report public, I would appreciate it.
I am attaching the patch now. Since writing the original description, I have written documentation for the time specification parameters in GotoIfTime and ExecIfTime. I have also changed the description of the delay parameter of Answer() to reflect my interpretation of the code.
The patch which I am submitting contains a rough draft of the revised documentation. I am also pretty sure it contains errors that prevent it from loading into Asterisk. Does anyone know how to 1) validate the XML and 2) reload the documentation file without restarting Asterisk?
By: Leif Madsen (lmadsen) 2010-04-15 12:32:35
Great thanks! I will move this back to public. Your interpretation is correct about private -- it's really meant for things like security issues, or those which contain confidential information.
To answer you questions:
1) yes! just run 'make validate' to validate your XML documentation
2) I don't think there is a way to reload the documentation without restarting Asterisk. You may be able to do a simple 'module reload' though -- give that a shot.
By: Leif Madsen (lmadsen) 2010-04-15 12:34:13
Setting to Ready for Testing until we're ready to review the entire thing prior to commit.
By: dovid (dovid) 2010-04-16 04:43:15
Sorry for my ignorance but in general are these patches against trunk ? If I am running 1.6.1.X can I apply it and test ?
By: David Chappell (chappell) 2010-04-16 08:35:33
I have uploaded new patches against 18.104.22.168 and against trunk. Unlike the first patch, this one has valid XML which will load into Asterisk.
If you are using 1.6.1.X, there is no harm in trying to apply the patch for 22.214.171.124. It patches a single file, main/pbx.c and it only patches comments. Just save a copy of main/pbx.c in case it doesn't work.
In reply to lmadsen: Thanks for the hint. The actual command is "make validate-docs". I have looked at the code that loads the documentation. The load function is called during Asterisk startup. There does not appear to be a way to reload the documentation. I wonder why the documentation is loaded into RAM. Couldn't we just load the needed piece from the files when called for? Just something to think about.
By: Leif Madsen (lmadsen) 2011-11-02 14:39:36.532-0500
I've dropped the ball on this one a bit. Is there any more documentation that needs to be updated here? What are the chances that someone could provide a new patch that applies cleanly?
By: David Chappell (chappell) 2011-11-21 13:02:14.019-0600
I will update the patch.
By: Leif Madsen (lmadsen) 2011-11-30 13:38:32.684-0600
Thanks David. Please ping me on IRC #asterisk-dev or #asterisk-bugs on irc.freenode.net when you're ready so I don't drop the ball again.
By: Paul Belanger (pabelanger) 2012-01-28 11:55:44.290-0600
Suspended due to lack of activity. Please request a bug marshal in #asterisk-bugs on the IRC network irc.freenode.net to reopen the issue should you have the additional information requested. Further information can be found at http://www.asterisk.org/developers/bug-guidelines