[Asterisk-code-review] ARI: Re-implement the ARI dial command, allowing for early b... (asterisk[master])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: ARI: Re-implement the ARI dial command, allowing for early bridging.
......................................................................


Patch Set 6:

> > (1 comment)
 > 
 > I've got the same question.  As I was updating the test i realized
 > you now had to specify the dialstring as 'endpoint' on channel
 > create and then again as 'dialstring' on dial because they're both
 > required.
 > It also appears that dial 'dialstring' is completely ignored.  If I
 > don't specify it I get an error but I can put garbage in it the
 > test still works.

I'm not a fan of requiring the dialstring in the dial method and can make it work without requiring it with a bit of futzing.

The reason why it's needed is that the channel creation method is a loose wrapper around ast_request() and the dial method is a wrapper for ast_call(). When I looked at ast_call(), I noticed that it required a "dest" string to be passed to it. I was surprised by this. The channel drivers I'm familiar with set destination information during ast_request(), and ast_call() just kicks off the actual call attempt. However, some channel drivers require the dialstring to be present during ast_call(). So, since different channel drivers have different requirements for making a call, I went with having the ARI dial require a dialstring as well.

Now, here's the fun part. If you look in the code at any example where ast_request() and ast_call() are called, you'll see that they *always* pass the same string to both methods. So there is no realistic situation where you'd want to pass one string to ast_request() and a separate one to ast_call(). It's just that certain channel drivers use the string for different purposes during the requesting phase and dialing phase.

In ARI, I can get around the current requirement by saving the endpoint name/dialstring used in the create method into a channel datastore. Then when it comes time to dial the channel, I can pull the dialstring from the datastore and use that where necessary. It wouldn't be difficult to do that, and it probably would make the API less murky.

And to George, the reason why you saw the gibberish dialstring have no effect is because only some channel drivers do anything with the dialstring passed to ast_call(). If you were using a Local channel or a PJSIP channel, for instance, then the dialstring passed to ast_call() is unused.

-- 
To view, visit https://gerrit.asterisk.org/2790
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7750359ddf45fcd45eaec749c5b3822de4a8ddbb
Gerrit-PatchSet: 6
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: No