Summary:	ASTERISK-25678: app_confbridge: Add list concise command
Reporter:	jaredmauch (jaredmauch)	Labels:
Date Opened:	2016-01-09 19:06:43.000-0600	Date Closed:	2018-01-02 08:49:51.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Applications/app_confbridge
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) app_confbridge.concise.patch ( 1) confbridge.patch.2.txt
Description:	In preparing to migrate from MeetMe to Confbridge we needed some "comfort" capabilities to emulate some of the meetme cli commands used. This introduces a confbridge list 1234 concise style command that is helpful for machines to read the input.  This is mostly prototype code that allows us to see the talker in the CLI (for hunting noisy lines). I'm not wedded to anything specific except two things: 1) I prefer the channel name to be first 2) I need the talker information, without tracking all the event state. My code certainly isn't perfect but I've tested it and it works.
Comments:	By: Asterisk Team (asteriskteam) 2016-01-09 19:06:45.141-0600 Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report. Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process]. By: Joshua C. Colp (jcolp) 2016-01-09 19:37:28.179-0600 [~jaredmauch] I would suggest putting this on Gerrit for code review, so it can see eventual inclusion into the tree. Otherwise it will be up to someone else to shepherd the change through the process which may take some time. By: jaredmauch (jaredmauch) 2016-01-09 19:45:29.798-0600 I'm willing to do a more full compatible version of the concise and the various options that ConfBridge supports, but wasn't sure what the community interest for this would be. By: Joshua C. Colp (jcolp) 2016-01-09 19:48:49.089-0600 From a community perspective if you'd like end-user input the asterisk-users mailing list would be your best bet for maximum exposure. From a code perspective gerrit is the place for code feedback when you're ready. By: jaredmauch (jaredmauch) 2016-01-09 20:21:52.782-0600 Cool.  Been doing this too long and not enough contribs to the asterisk project to recall how the community responds to these things.  Had bad experiences with a few groups so be gentle with me, have a form of beaten child syndrome. By: Joshua C. Colp (jcolp) 2016-01-09 20:25:51.014-0600 It's all good. Generally people don't crawl the issue tracker, which is why I mentioned the mailing list. By: Richard Mudgett (rmudgett) 2016-01-11 11:16:13.102-0600 Concise CLI commands tend to be fragile in their output as that interface is mainly intended for human interaction.  Why not use AMI where you can easily add more information headers to the output and is intended to be machine interpreted? By: jaredmauch (jaredmauch) 2016-01-11 14:41:52.797-0600 Because neither of those provide the capability.  Did you look at the patch as well?  The user object doesn't track talking so there's no way to get the current state of talkers vs wait for an on/off event. :) I would be happy if AMI or ARI provided a way to pull this information, but at minimum a subset of this needs to be done. By: Richard Mudgett (rmudgett) 2016-01-11 15:03:03.715-0600 Yes I looked at your patch.  My main concern was adding the concise command not the other parts of your patch.  I also stated: {quote} Why not use AMI where you can *easily add* more information headers to the output and is intended to be machine interpreted? {quote} By: jaredmauch (jaredmauch) 2016-01-11 17:31:17.308-0600 This was partly a straw-man, but also to solve a problem that I have.  I'll add concise to all the comparable code points as well, should i expose the entire struct, eg: menus, etc?  What about flags, should those get their own unique columns? These commands haven't been well documented in the past. Re: AMI/ARI, I'm open to also enhancing them, but like I said previously, the user didn't hold the talking/not-talking state, so unless you cached the previous talk/no-talking event I didn't see a way to fetch the current state. Things like ConfBridgeList are not well constrained for a read-only capability of just polling these things.  I'd much prefer that but would need some help/pointers to expose this. With the talker state, it shouldn't be that hard to add that to the ConfBridgeList I guess, but having a working hack was my primary goal so I can migrate away from meetme.  I'm also a bit bummed with the way the users time in conf was done in meetme and the channel start time is the best proxy I can find. Some of it may be the way we're actually using MeetMe/ConfBridge with a dedicated exten per user for their bridges that is part of our unified dialplan.   eg: meetme {noformat} exten => s,n,Set(CONF_ARGS=${IF($[ ${ARG3} = 1]?${CONF_ARGS}I:${CONF_ARGS})}) ; if arg3=1 add I to CONF_ARGS record name at join exten => s,n,Set(CONF_ARGS=${IF($[ ${ARG5} = 1]?${CONF_ARGS}r:${CONF_ARGS})}) ; if arg5=1 add r to CONF_ARGS record exten => s,n,Set(CONF_ARGS=${IF($[ ${ARG7} = 1]?${CONF_ARGS}M:${CONF_ARGS})}) ; if arg7=1 add M to CONF_ARGS hold muzak when empty exten => s,n,Set(CONF_ARGS=${IF($[ ${ARG8} = 1]?${CONF_ARGS}m:${CONF_ARGS})}) ; if arg8=1 add m to CONF_ARGS muted at start {noformat} eg: confbridge {noformat} exten => s,n,Set(CONFBRIDGE(user,announce_join_leave)=${IF($[ ${ARG3} = 1]?yes:no)}) ; if arg3=1 set record name at join exten => s,n,Set(CONFBRIDGE(bridge,record_conference)=${IF($[ ${ARG5} = 1]?yes:no)}) ; if arg5=1 set record exten => s,n,Set(CONFBRIDGE(user,music_on_hold_when_empty)=${IF($[ ${ARG7} = 1]?yes:no)}) ; if arg7=1 set hold muzak when empty exten => s,n,Set(CONFBRIDGE(user,startmuted)=${IF($[ ${ARG8} = 1]?yes:no)}) ; if arg8=1 set muted at start exten => s,n,Set(CONFBRIDGE(user,quiet)=${IF($[ ${ARG9} = 1]?yes:no)}) ; if arg9=1 set no join/leave sound exten => s,n,Set(CONFBRIDGE(user,admin)=${IF($[ ${ARG10} = 1]?$yes:no)}) ; if arg10=1 set admin mode {noformat} as we use all dynamic conferences, I can switch a database flag and convert people from Meetme <-> ConfBridge automatically it just switches the macro used. I'll add the talking flag to the ConfBridgeList AMI, you're certainly right there. By: jaredmauch (jaredmauch) 2016-01-11 17:55:58.116-0600 Updated patch, exposes data via ConfbridgeList command in AMI as well, adds concise to confbrridge list as well as confbridge list 1234 concise By: Corey Farrell (coreyfarrell) 2017-12-15 11:04:45.577-0600 I agree with Richard, CLI is not meant for consumption by other programs.  I would support adding attributes to AMI/ARI responses but I'm -1 on addition of concise CLI commands.  It bloats CLI code and adds little value. If you'd like to submit a change to gerrit adding the attributes you require to AMI/ARI list actions I can help you through the process. ---- Thanks for the contribution! If you'd like your contribution to be included faster, you should submit your patch for code review by the Asterisk Developer Community. To do so, please follow the Code Review \[1\] instructions on the wiki. Be sure to: * Verify that your patch conforms to the Coding Guidelines \[2\] * Review the Code Review Checklist \[3\] for common items reviewers will look for * If necessary, provide tests for the Asterisk Test Suite that verify the correctness of your patch \[4\] When ready, submit your patch and any tests to Gerrit \[5\] for code review. Thanks! \[1\] https://wiki.asterisk.org/wiki/display/AST/Code+Review \[2\] https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines \[3\] https://wiki.asterisk.org/wiki/display/AST/Code+Review+Checklist \[4\] https://wiki.asterisk.org/wiki/display/AST/Asterisk+Test+Suite+Documentation \[5\] https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage By: Corey Farrell (coreyfarrell) 2017-12-18 10:37:23.457-0600 Received by email: {quote} Regarding: https://issues.asterisk.org/jira/browse/ASTERISK-25678 1) I find it quite amusing after nearly 2 years there is a response saying “we should fix it some other method and we don’t like the CLI bloat”. 2) If there is a supported method, please consider exposing that instead.  I’m aware of people using this in production at this moment due to the lack of direct support for this, hence the reason I submitted this upstream to be included. 3) I can’t seem to login to JIRA to submit this comment directly, so here’s hoping this will submit it. \- Jared {quote} # Nearly two years ago Richard indicated that we don't want to add concise CLI commands and he gave a reason why.  He suggested doing AMI *instead* of CLI concise.  No further action has been taken on your patch because you ignored an important part of his feedback. # We already have the {{ConfbridgeList}} AMI action to list channels in a conference, if you want to see fields that are not included you can feel free to submit a patch to add the fields to the response.  Adding fields to an AMI response is something that would not face heavy resistance (assuming your changes do not introduce regressions). # I am a community developer, not an administrator.  You can get help with your JIRA account by emailing [mailto:asteriskteam@digium.com]. By: Asterisk Team (asteriskteam) 2018-01-02 08:49:51.779-0600 Suspended due to lack of activity. This issue will be automatically re-opened if the reporter posts a comment. If you are not the reporter and would like this re-opened please create a new issue instead. If the new issue is related to this one a link will be created during the triage process. Further information on issue tracker usage can be found in the Asterisk Issue Guidlines [1]. [1] https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines