[Home]

Summary:ASTERISK-08674: [post-1.4][branch] CLI command audit
Reporter:Leif Madsen (lmadsen)Labels:
Date Opened:2007-01-28 11:48:06.000-0600Date Closed:2008-02-08 15:24:23.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Documentation
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20071110_8925_asterisk.diff
( 1) 20071110_8925_cdr.diff
( 2) 20071110_8925_res_agi.diff
( 3) 20071110_8925_softhangup.diff
( 4) 20071113_8925_jon.diff
( 5) 20071119_8925_jon.diff
( 6) cli_tree_2.txt
( 7) cli_tree.txt
( 8) CONSOLE_COMMAND_BUGS
Description:While Jim van Meggelen and I were going over the CLI commands for the 2nd edition of TFoT, we've found some discrepancies. Our attempt here is simply to determine how the CLI commands should be documented. Either they are to be documented as they exist now, or should they be documented following the new format for CLI commands.

This bug is opened for discussion of the CLI commands, and we are simply trying to determine how the CLI commands should be documented, either as-is, or as ideal.

Update: It's too late to fix this in 1.4 now, so lets assume we're in a transition and get these fix in trunk.

****** ADDITIONAL INFORMATION ******

Find attached a text file with the commands which we've found discrepancy in based on the <module> <verb> <argument> format which seems to be the direction the CLI is moving towards for a standardized CLI methodology.
Comments:By: Tilghman Lesher (tilghman) 2007-02-07 12:33:21.000-0600

"soft hangup" can become "core hangup".  The reason for the word "soft" was to avoid a conflict with the console driver's "hangup".

These look okay, but what we need is approval for an exception for 1.4 from Russell (or disapproval, in which case, these go only into trunk).
By: Russell Bryant (russell) 2007-02-07 15:15:22.000-0600

I'm ok with updating these commands, as long as anything that is there, still works, and is just marked as deprecated.
By: Michiel van Baak (mvanbaak) 2007-09-04 11:15:25

if noone rejects I'll start working on it
By: Michiel van Baak (mvanbaak) 2007-09-23 06:50:21

uploaded patch that deprecates restart, stop and abort in favor of 'core restart ...', 'core stop ...' and 'core abort ...'
By: Michiel van Baak (mvanbaak) 2007-09-23 07:07:04

"soft hangup" => "core hangup" patch uploaded.
By: Michiel van Baak (mvanbaak) 2007-09-23 07:27:38

ael debug fixes patch
By: Michiel van Baak (mvanbaak) 2007-09-23 07:42:53

agi debug => agi set debug patch
By: Michiel van Baak (mvanbaak) 2007-09-23 07:49:55

cdr status => cdr show status patch
By: Tilghman Lesher (tilghman) 2007-09-23 11:15:02

Uh, you shouldn't rename "soft hangup", as the word "soft" means something VERY specific.
By: Michiel van Baak (mvanbaak) 2007-09-23 11:18:17

ok, no problem. I was going by your first comment on this issue so renamed it as you stated there. Going to remove the patch then :)
By: Russell Bryant (russell) 2007-09-23 13:26:48

I think "core soft hangup" would be ok
By: Tilghman Lesher (tilghman) 2007-09-23 15:44:47

I dunno, "core soft hangup" doesn't really fit with "module verb args", maybe "core send soft hangup"?
By: Michiel van Baak (mvanbaak) 2007-09-23 16:52:36

how about this one (20070924_8925_hangup.diff) ?
By: Michiel van Baak (mvanbaak) 2007-10-19 13:46:08

now that seanbright finished his work on the NEW_CLI/AST_CLI stuff I will redo all the patches and finish this one.
By: Jon Webster (jon) 2007-10-29 11:26:23

No warning that 'add extension' is depreciated, although the new command is 'dialplan add extension'
By: Michiel van Baak (mvanbaak) 2007-11-07 11:47:53.000-0600

Deleted old and deprecated patches.
Work on new ones is started now I have a working laptop again.
By: Michiel van Baak (mvanbaak) 2007-11-07 12:55:00.000-0600

First 3 patches with newstyle AST_CLI_DEFINE that deprecates:
restart <when>
stop <when>
abort <when>
agi debug
ael debug
By: Michiel van Baak (mvanbaak) 2007-11-10 04:32:47.000-0600

soft hangup => core send soft hangup
By: Michiel van Baak (mvanbaak) 2007-11-10 04:34:37.000-0600

jon: trunk does not have 'add extension' anymore
By: Michiel van Baak (mvanbaak) 2007-11-10 04:45:15.000-0600

cdr status => cdr show status patch
By: Michiel van Baak (mvanbaak) 2007-11-10 06:03:21.000-0600

note to self: if something compiles it not always means that it will run too.
Redid patches so asterisk actually starts.
By: Jon Webster (jon) 2007-11-12 23:54:52.000-0600

Here's another one,

--- pbx.c 2007-11-13 00:50:24.000000000 -0500
+++ pbx.c        2007-11-13 00:51:16.000000000 -0500
@@ -3050,7 +3050,7 @@
"       Describe a particular dialplan function.\n";

static char show_dialplan_help[] =
-"Usage: dialplan show [exten@][context]\n"
+"Usage: dialplan show [[exten@]context]\n"
"       Show dialplan\n";

static char set_global_help[] =
By: Jon Webster (jon) 2007-11-13 14:20:04.000-0600

I always thought GotoIf only worked on labels (priorities with names). After trying to work some magic with GosubIf it became apparent that they both follow the same format as their non-condition handling counterparts.

Here's the current usage description of both...
 Goto([[context|]extension|]priority):
 Gosub([[context|]exten|]priority):

 GotoIf(condition?[labeliftrue]:[labeliffalse]):
 GosubIf(condition?labeliftrue[:labeliffalse]):

 GotoIfTime(<times>|<weekdays>|<mdays>|<months>?[[context|]exten|]priority):

I suggest using either 'extension' or 'exten', not both.
Also, labeliftrue isn't required, and neither is :labeliffalse, so both GotoIf and GosubIf are both wrong. :P

Meaning the following 3 cases are valid
GotoIf(0?context|exten|prio)
GotoIf(0?:context|exten|prio)
GotoIf(0?context|exten|prio:context|exten|prio)


I know this is ugly...
GotoIf(condition?[[context|]extension|]priority][:[context|]extension|]priority])

So another option is to add the following note to the end of description of these two applications:

Labels take the form '[context|]extension|]priority'

The uploaded file uses the first suggestion.
By: Leif Madsen (lmadsen) 2007-12-06 13:47:02.000-0600

I think you need to do:

rtp debug [off] => rtp set debug [on|off]
By: Jon Webster (jon) 2007-12-06 15:04:33.000-0600

In response to the newly added relationship, I'm with corydon that all the <module> reload commands should be depreciated in favor of module reload <module>.

But since we're considering adding a 'voicemail reload', another command to consider is 'queue reload'
By: Eliel Sardanons (eliel) 2007-12-06 15:42:25.000-0600

jon, this was already discussed on the mailing list and we were all agree to continue with the '<module> reload' commands, in favor tu usability. Administrators aren't developers, and administrators don't know in which module is implemented every command.
By: Leif Madsen (lmadsen) 2007-12-06 22:35:47.000-0600

ael no debug --> should go to --> ael set debug off

ael nodebug --> should be removed
By: Leif Madsen (lmadsen) 2007-12-06 22:38:31.000-0600

stun debug [off] --> should be --> stun set debug [off]
By: Leif Madsen (lmadsen) 2007-12-06 22:40:22.000-0600

udptl debug [off] --> should be --> udptl set debug [off]

udptl debug ip --> should be ==> udptl set debug ip

udptl no --> to be removed



Edit: (instead of sending a new email, I'll just modify this note)

A couple more things:

- help abort halt: usage shows as 'abort shutdown' which is invalid. Probably not an issue since abort halt got moved to core abort halt


- sip history [off] --> should be --> sip set history [off]


By: Leif Madsen (lmadsen) 2007-12-07 11:23:50.000-0600

I think this bug is a candidate for a team branch...
By: Leif Madsen (lmadsen) 2007-12-09 18:55:12.000-0600

Found another one:

agi debug --> should be --> agi set debug
agi no debug --> should be --> agi set debug off
By: Eliel Sardanons (eliel) 2007-12-09 18:57:18.000-0600

blitzrage: I think the best way to start this "massive" change (deprecate old commands) is to post a message on the mailing list about a new janitor project.
By: Leif Madsen (lmadsen) 2007-12-09 19:01:28.000-0600

Ya, that probably makes sense. I think I'm going to try and take on this janitor project as I'm in the middle of documenting the entire command line syntax. I've started with a text file of the command line tree, and will hold off on further updates to this bug until I complete that tree and documentation, then I'll post the whole tree with my notes to this bug.

Once that is done, then we can make sure we're all in agreement with the syntax, then go ahead and someone can code it (whether it be me or someone else) and finally get the CLI syntax done once and for all.
By: Tilghman Lesher (tilghman) 2007-12-09 20:02:05.000-0600

blitzrage:  generally, we've been trying to use the keyword 'on', as well as 'off', to specify the direction.  I know that it is optional in some circumstances, but the canonical form should be, e.g. 'agi set debug on' and 'agi set debug off', to ensure that both directions are clear in the intent.
By: Leif Madsen (lmadsen) 2007-12-09 20:47:21.000-0600

Corydon: OK, I had thought about that as well, but didn't notice any of the commands showing it documented as such. I'll be sure to note that.
By: Leif Madsen (lmadsen) 2008-01-05 08:55:19.000-0600

OK, I've attached my CLI audit as 'cli_tree.txt' which has some notes about the CLI. This is based off of ABE C.1, which is basically Asterisk 1.4 (I can't imagine the CLI is any different between the versions).

If you have any questions, let me know. There are some notes at the bottom. I should probably run through this again, but I think what I did was to build this out as how it should be with notes and arrows showing the deprecated commands "new" command.
By: Michiel van Baak (mvanbaak) 2008-01-05 15:58:10.000-0600

I think I'll have to redo most of my patches again.
I have no idea how close 1.6 is but I think for now we will have to work against trunk since we are going to deprecate a lot of commands.
By: Leif Madsen (lmadsen) 2008-01-06 09:31:09.000-0600

I would agree with that. No point in changing things in 1.4 -- we're much too far along with 1.4 releases to deprecate and change all that stuff I think...
By: Digium Subversion (svnbot) 2008-01-31 14:26:59.000-0600

Repository: asterisk
Revision: 101527

A   team/mvanbaak/cli-command-audit/

------------------------------------------------------------------------
r101527 | mvanbaak | 2008-01-31 14:26:58 -0600 (Thu, 31 Jan 2008) | 2 lines

Create branch for issue ASTERISK-8674

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=101527
By: Digium Subversion (svnbot) 2008-01-31 16:02:55.000-0600

Repository: asterisk
Revision: 101579

U   team/mvanbaak/cli-command-audit/main/asterisk.c
U   team/mvanbaak/cli-command-audit/main/cdr.c
U   team/mvanbaak/cli-command-audit/res/res_agi.c

------------------------------------------------------------------------
r101579 | mvanbaak | 2008-01-31 16:02:52 -0600 (Thu, 31 Jan 2008) | 8 lines

add patches from mantis to this branch

(issue ASTERISK-8674)
Patches:
20071110_8925_cdr.diff uploaded by mvanbaak (license 7)
20071110_8925_asterisk.diff uploaded by mvanbaak (license 7)
20071110_8925_res_agi.diff uploaded by mvanbaak (license 7)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=101579
By: Michiel van Baak (mvanbaak) 2008-02-03 08:42:51.000-0600

blitzrage and others: can you test my branch?
I took your cli-tree.txt and all the comments on this issue and I think it's all done now.
By: Leif Madsen (lmadsen) 2008-02-06 12:44:14.000-0600

Attached a new CLI audit file as cli_tree_2.txt.

This audit is based on mvanbaak's branch. I tried to compile in as many modules as possible, but was missing mISDN, JACK, and H.323 stuff.
By: Michiel van Baak (mvanbaak) 2008-02-06 13:54:59.000-0600

thanks for the second audit.
Updated branch to reflect those that needed update.

The ael module has no 'ael set debug on' because you have to specify what you want to debug
'core set verbose on' is also not there because you have to pick a verbose level

I think this ends our journey and this can be merged
By: Digium Subversion (svnbot) 2008-02-08 15:24:18.000-0600

Repository: asterisk
Revision: 103171

U   trunk/apps/app_minivm.c
U   trunk/apps/app_playback.c
U   trunk/apps/app_rpt.c
U   trunk/apps/app_voicemail.c
U   trunk/channels/chan_iax2.c
U   trunk/channels/chan_mgcp.c
U   trunk/channels/chan_sip.c
U   trunk/channels/chan_skinny.c
U   trunk/channels/chan_usbradio.c
U   trunk/funcs/func_devstate.c
U   trunk/main/asterisk.c
U   trunk/main/cdr.c
U   trunk/main/pbx.c
U   trunk/main/rtp.c
U   trunk/main/udptl.c
U   trunk/pbx/pbx_ael.c
U   trunk/pbx/pbx_dundi.c
U   trunk/res/res_agi.c
U   trunk/res/res_jabber.c

------------------------------------------------------------------------
r103171 | russell | 2008-02-08 15:24:16 -0600 (Fri, 08 Feb 2008) | 13 lines

Merge changes from team/mvanbaak/cli-command-audit
(closes issue ASTERISK-8674)

About a year ago, as Leif Madsen and Jim van Meggelen were going over the CLI
commands in Asterisk 1.4 for the next version of their book, they documented
a lot of inconsistencies.  This set of changes addresses all of these issues
and has been reviewed by Leif.

While this does introduce even more changes to the CLI command structure, it
makes everything consistent, which is the most important thing.

Thanks to all that helped with this one!

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=103171