[Home]

Summary:ASTERISK-16327: AGI manager command is not consistent with other commands
Reporter:Ryan Bullock (rrb3942)Labels:
Date Opened:2010-07-07 09:37:31Date Closed:2012-01-25 12:46:31.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Resources/res_agi
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:There are a few issues with the AGI manager command.

1. Does not appear to use ActionIDs at all (Very Bad).
2. Uses a CommandID, but this is only used in some of it's response events. This should be replaced with the ActionID used with the command.
3. The message or response line in the initial acknowledgement should should indicate that the results will follow.
4. Response Events require you to have 'agi' events enabled in order to receive them. They are also sent to everyone listening to those events. This could cause CommandID/ActionID collision.
5. The command should end with a 'Complete' event.

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

For compatibility sack the CommandID could be left in so long as the ActionID is also used properly.
Comments:By: Leif Madsen (lmadsen) 2010-07-07 09:45:52

This kind of borders on the level of feature request. While it may not be consistent, it's not really considered a bug.

Are you able to provide a patch to implement the functionality you describe?

By: Ryan Bullock (rrb3942) 2010-07-07 12:16:36

The complete lack of the use of ActionIDs seems like a bug to me. This command is different enough that it can cause compatibility issues with some of the libraries used for working with the AMI if the author does not make serious exceptions to handle it.

I can do some work on a patch for this. I just have not done any work towards it

If you like I could also break this up into two patches.

Patch 1 - Add ActionIDS and make it and the CommandID present throughout all response. Change responses to only be sent back on the appropriate manager connection without the need to have events enabled. I don't think is would break compatibility.

Patch 2 - Rest of it, making it more consistent with other commands.

By: Leif Madsen (lmadsen) 2010-07-08 09:47:08

I agree with the approach. The first part/set of patches would be closest to a bug fix, and I think that is where this issue should be resolved at.

For the second part, if you are willing to do the work, then that would be fantastic. I would encourage you to file that as a new issue with the severity set to Feature and then we can relate it back to this issue.

By: Richard Mudgett (rmudgett) 2012-01-25 12:46:31.983-0600

{quote}
1. Does not appear to use ActionIDs at all (Very Bad).
{quote}

It does use AstionID's.  The ActionID handling is done in the standard
response functions: astman_send_error() and astman_send_ack().

{quote}
2.  Uses a CommandID, but this is only used in some of it's
response events.  This should be replaced with the ActionID used with the
command.
{quote}

The CommandID is used in all AsyncAGI generated events which apply to the
command.  The CommandID serves a similar but different purpose than the
ActionID and must be globally unique as pointed out by item 4.  

{quote}
3.  The message or response line in the initial acknowledgement should
indicate that the results will follow.
{quote}

The AMI action is just for queuing the command for the channel to process.
You are trying to fit a square peg into a round hole here.  You are trying
to use a format that does not apply here because there is no mechanism to
do what you are requesting.

Queued commands are not guaranteed to be executed because the associated
channel could be disconnected before it is executed.

{quote}
4.  Response Events require you to have 'agi' events enabled in order to
receive them.  They are also sent to everyone listening to those events.
This could cause CommandID/ActionID collision.
{quote}

This is a consequence of the assumption made in item 3.  Any ID collisions
are not the responsibility of Asterisk.

{quote}
5. The command should end with a 'Complete' event.
{quote}

This is also a consequence of the assumption made in item 3.

----

Since this is a feature request without a patch I am closing this issue.