| Summary: | ASTERISK-15992: [patch] Originate Action output is inconsistent with other manager actions | ||||
| Reporter: | Ryan Bullock (rrb3942) | Labels: | patch | ||
| Date Opened: | 2010-04-21 12:58:08 | Date Closed: | 2018-01-02 08:49:52.000-0600 | ||
| Priority: | Minor | Regression? | No | ||
| Status: | Closed/Complete | Components: | Core/ManagerInterface | ||
| Versions: | Frequency of Occurrence | ||||
| Related Issues: |
| ||||
| Environment: | Attachments: | ( 0) orignatepatch.diff ( 1) orignatepatch2.diff | |||
| Description: | The Orginate action output is inconsistent with how other manager actions respond, and is even inconsistent with how itself responds with/without Async: 1, supplying different information depending on if Async is used or not. It also will not send the response event (when using async) back to the manager session if that manager session has events turned off. Other actions that respond with events will do so even if events are turned off. ****** ADDITIONAL INFORMATION ****** Responses should be consistent whether Async is used or not, and it should always respond on the manager connection regardless of the connections eventmask. | ||||
| Comments: | By: Ryan Bullock (rrb3942) 2010-04-21 13:11:52 The attached patch makes a number of changes to how the originate action behaves. 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: Reponse Packet, EventResponse, EventComplete. If there is an error before the outgoing call is made it will only respond with a Response Packet. Here is some example output of the new behavior: Bad Application: Action: Originate Channel: SIP/4000 Application: Doesnotexist Response: Error Message: Invalid or missing Application Good Application: Action: Originate Channel: SIP/4000 Application: Voicemail Response: Success Message: Response will follow Event: OriginateResponse Result: Remote end has Answered Channel: SIP/4000-00000000 Application: Voicemail Reason: 4 Uniqueid: 1271872076.0 CallerIDNum: <unknown> CallerIDName: <unknown> Event: OriginateComplete Good Exten: Action: Originate Channel: SIP/4000 Exten: 4000 Context: from-internal Priority: 1 Response: Success Message: Response will follow Event: OriginateResponse Result: Remote end is Busy Channel: SIP/4000 Exten: 4000 Context: from-internal Priority: 1 Reason: 5 Uniqueid: <null> CallerIDNum: <unknown> CallerIDName: <unknown> Bad Context: Action: Originate Channel: SIP/4000 Exten: 4000 Context: bad Priority: 1 Response: Error Message: Invalid or missing 'Exten', 'Context', or 'Priority' ActionIDs are included in the responses if they are used. This patch applies to trunk ok, but I was unable to get it to apply to 1.6.2.6. This will most likely break things that were written to work with the old Originate behavior. By: Ryan Bullock (rrb3942) 2010-04-21 13:12:59 There is a 'Event: OriginateComplete' for the 'Good Exten' example, I just can't copy and paste. By: Ryan Bullock (rrb3942) 2010-04-22 21:00:25 Reviewboard Link: https://reviewboard.asterisk.org/r/624/ By: Paul Belanger (pabelanger) 2010-04-23 08:14:14 This patch seems to break backwards compatibility for existing applications using Originate. At a minimum you will need to include updates to documentation (CHANGES and UPGRADE.txt). As for the change, your best to jump on #asterisk-dev and talk with developers about this. By: Leif Madsen (lmadsen) 2010-04-26 12:40:24 Marking as Confirmed for now (and not Ready for Testing) until we hear back from a developer stating this is the right way forward. I feel like this is probably the right way, but the removal of the synchronous Originate could potentially break some existing applications. For sure the UPGRADE.txt and CHANGES files need to be updated. By: Ryan Bullock (rrb3942) 2010-04-27 09:21:45 Uploaded a new patch that contains entries in UPGRADE.txt and CHANGES. I will make a post to the asterisk-dev mailing list about these proposed changes. By: Leif Madsen (lmadsen) 2010-04-27 11:06:09 Thanks for the quick turn around! When you post to the asterisk-dev mailing list and can then link back to the thread here? Just find the link via http://lists.digium.com Thanks! By: Ryan Bullock (rrb3942) 2010-04-27 13:26:46 Discussion on asterisk-dev mailing list: http://lists.digium.com/pipermail/asterisk-dev/2010-April/043786.html By: Ryan Bullock (rrb3942) 2010-05-13 09:22:42 I am trying here, what else can I do to help this along? I would love to see changes made to this action for 1.8 so that the behavior doesn't hang around for another number of years. By: Leif Madsen (lmadsen) 2010-05-17 11:08:01 Currently there isn't anything further to do here other than perhaps try and get a community developer to move this forward. You could try in #asterisk-dev on the Freenode IRC network. Otherwise this issue will be moved forward as soon as time and resources allow. By: Corey Farrell (coreyfarrell) 2017-12-14 13:31:30.712-0600 Are you interested in pursuing this further? If so the patch will need to be rebased so it can apply the the current {{master}} branch and it will need to go through code review. ---- 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: Asterisk Team (asteriskteam) 2018-01-02 08:49:52.275-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 | ||||