[asterisk-dev] [Code Review] Changes to Manager Interface: Make Originate Action more consistent with other actions behavior

Jeff Peeler jpeeler at digium.com
Thu May 6 17:16:12 CDT 2010


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


Just a first pass nitpick...


/trunk/main/manager.c
<https://reviewboard.asterisk.org/r/624/#comment4212>

    Declare the variable at the top of this function and change to:
    
    fast = ast_calloc(...
    if (!fast) {
     return 0;
    ...
    
    Then take out the if(fast) to reduce indentation


- Jeff


On 2010-04-22 20:58:35, rrb3942 wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/624/
> -----------------------------------------------------------
> 
> (Updated 2010-04-22 20:58:35)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Currently the Originate action for the manager interface behaves very inconsistently when compared to other manager actions.
> 
> The major problems I see are: 
> 
> 1. The output and information returned is different between 'Async: 0' and 'Async: 1'
> 2. With 'Async: 1' it returns two packets that are a 'Response' packet, the second is actually a 'Response' and an 'Event' packet.
> 3. If you do not have call events enabled and have 'Async: 1' you will never get the event generated by action. This is not the case with other actions, which will send events regardless of the eventmask for the connection.
> 
> This patch makes the following changes:
> 
> 1. It will always create the outgoing call asynchronously. The Async option has been removed.
> 2. It will always respond on the manager connection, regardless of the eventmask.
> 3. It checks the validity of the tech for the channel, existence of the exten/context/priority, existence of the application. If these fail it immediately returns with an error.
> 4. It includes the Application and Data, or Exten/Context/Priority in the response event depending on which is used. (Old behavior always included Exten/Context/priority regardless of what was used)
> 5. The 'Response' line in the event is removed in favor of 'Result' which contains the text equivalent of 'Reason'.
> 6. It will always respond to an action as follows: Response Packet, EventResponse, EventComplete. If there is an error before the outgoing call is made it will only respond with a Response Packet.
> 
> The bug report contains some sample output.
> 
> I also believe that the old action would send the action response out on all manager connections when 'Async: 1'. The event contained the ActionID which could cause some odd problems when multiple users are involved. The new one only sends the response to the appropriate manager session.
> 
> 
> This addresses bug 0017221.
>     https://issues.asterisk.org/view.php?id=0017221
> 
> 
> Diffs
> -----
> 
>   /trunk/main/manager.c 257809 
> 
> Diff: https://reviewboard.asterisk.org/r/624/diff
> 
> 
> Testing
> -------
> 
> Tested by Contributer.
> 
> 
> Thanks,
> 
> rrb3942
> 
>




More information about the asterisk-dev mailing list