Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

voice helpers #15

Closed
ianjuma opened this issue May 10, 2016 · 13 comments
Closed

voice helpers #15

ianjuma opened this issue May 10, 2016 · 13 comments

Comments

@ianjuma
Copy link
Contributor

ianjuma commented May 10, 2016

the voice helpers are very interesting; what approach do we take? plain xml response? or command to construct xml ? The helpers will be great, this will make voice so easy 👍

voice.say("X is a letter! It's not a word") 

Here the approach is to wrap say around root xml response - what of nested commands? and attributes, how to create the xml response?

Of course this is for incoming calls, with the callback url handler.
voice.call is ok. I don't think there's much there. I mean initiating a call.

voice.say("say something", voice="woman") // ?

Say then Play a URL

/*
<Say>Please listen to our awesome record</Say>
<Play url="http://www.myvoicemailserver.com/audio/vmail.wav"/>
*/
voice.say("Hi, listen to", play=url)

and how do we wrap getDigits when we say?

voice.say("say something I'm giving ... ", voice=woman, getDigits=True, record=True) // ?
// sadly without named parameters
// also, recording - maybe an attribute to be set

call redirect?

if (dtmfDigit === '9') {
   voice.redirect('http://some_url_callback');
}

then, this would be expected.

<Response>
  <Dial phoneNumbers="+254711XXXYYY,+254733YYYZZZ,test@ke.sip.africastalking.com" ringBackTone="http://mymediafile.com/playme.mp3" record="true" sequential="true"/>
</Response>

maps to

// callerId specific to calling out from SIP
voice.dial(phoneNumbers=phones, maxDurationInSec=10, record=True, 
                ringBackTone="some_url", sequential="True", callerId="some_num");

Problem with this is that we need all possible mappings, and I think there's many cases, we might miss a few and not generate a proper functional XML string, but the XML response will still work.

So, basically we generate this by passing attributes on a command, is this a good approach?
we also need a SIP section, will look at this later.

@ianjuma ianjuma added this to the Version 1 milestone May 10, 2016
@ianjuma
Copy link
Contributor Author

ianjuma commented May 10, 2016

also, ES6 does not support named parameters 👎 - so, maybe pass an options object instead. and have this as the only argument, to just have this as the only named parameter object.

then the only drawback is that with that we won't have default values for parameters?

@pbombo and also how can we return the responses in commands? When we are done with the response build - maybe a var, with endSession = True

so, basically this would be an xml Response builder.

var options = {
  ...
}
var response0 = voice.say("Hey, whatever", options);
var response1 = voice.command(options);

resp.end(response);

@ianjuma ianjuma assigned ianjuma and aksalj and unassigned aksalj and ianjuma May 10, 2016
@pbombo
Copy link
Contributor

pbombo commented May 11, 2016

Passing an options object seems like a decent compromise and it could still have default values something like below is still possible

function (options) {
    var someSetting = options.someSetting || defaultValue;
}

@pbombo
Copy link
Contributor

pbombo commented May 11, 2016

This is also looking interesting, default parameters in ES6

@ianjuma
Copy link
Contributor Author

ianjuma commented May 11, 2016

I think my reaction was "JS can do that?" lol, I assumed it would, it's very pythonic. Better than ||

Problem is, the default value is within options, so it's not on call. This would have been great with named parameters.

@aksalj
Copy link
Collaborator

aksalj commented May 11, 2016

I think it can be done well even without named parameters. Something like this would be nice:

let str = "Salut!", anotherStr, url;
let options = { // action's secondary attributes as needed
   voice: 'dude',
   record: false
   //...
};
voice.say(str, options)
     .play(url)
     .then(success)
     .catch(error);

voice.getDigits(options)
     .say(str, options)
     .then(success)
     .catch(error);

But quick question: Is the documentation at http://docs.africastalking.com/voice up to date? I remember @ianjuma you said there were some big changes coming to voice... 😕

@pbombo
Copy link
Contributor

pbombo commented May 11, 2016

@aksalj I think the idea was to avoid user having to do this

let options = { // action's secondary attributes
   voice: 'dude',
   record: false
   //...
};

that is the real value of default parameters IMO.

Ideally you'd just have javascript voice.say(str) without the need to set options

@aksalj
Copy link
Collaborator

aksalj commented May 12, 2016

@ianjuma is the problem named parameters or default parameters?

With named params (which is what I think @ianjuma was referring to) you can have both this:
voice.say(str, voice='Female', record=false)
and this:
voice.say(str, record=false, voice='Female')
be the exact same call.
That way you can avoid passing params in a given order (therefore passing only those you need). JavaScript does not support named parameters yet afaik. But we can "simulate" this behavior with objects like this:

let options = { // only set the options you need, otherwise default values will be used
   voice: 'Female',
   record: false
   //...
};
voice.say(str, options);
// if you don't need to override the default options, just do
voice.say(str); 

Then say() is implemented like @pbombo said with something = options.something || defaultValue.

@ianjuma
Copy link
Contributor Author

ianjuma commented May 13, 2016

I think both, ES6 doesn't support it, this could have been more readable.
I wish we didn't have to do that; I think the order of params should be free from the user, so
we might have the options set. My problem is with attributes, it just seems un-conventional to have those in options. Python problems :( but that's the way to go I agree.

@aksalj some of the commands were updated, and there parameters too. Say, Play, Record, Redirect, Reject should be ok now. I've been slow with that, sadly 😞 Users on the API have been creating XML responses, I really want this helper approach, I think it will make working with Voice easy.
Also @aksalj @pbombo I can map a number to your account, for testing with this?

now, order of commands? How are we making this response. Responses are usually simple and straight forward. So, are we grabbing the response string and returning it?

voice.say(str, options)
     .play(url)
     .then(success)
     .catch(error);

@habbes
Copy link
Contributor

habbes commented May 15, 2016

May I contribute to this discussion. I also go for having an options object. Considering the lack of default params and strong intellisense-like features, one ends up looking up the docs a lot just to remember the order of params.

@ianjuma
Copy link
Contributor Author

ianjuma commented May 16, 2016

@habbes That's the way to go now; @aksalj had already highlighted that.

@aksalj
Copy link
Collaborator

aksalj commented Jan 13, 2017

Could something like this be a good solution?

// example (express)
app.post('/callback/voice', new AfricasTalking.VOICE.CallHandler((phoneCall) => {
    
    var session = phoneCall.session;
    //...
    
    if (phoneCall.isActive) {
        //...
    } else if (phoneCall.isCompleted){
        //...
    }
    
    
    var response = voice.builder()
        .say("UI", {voice: "woman"})
        .play("DEd")
        .getDigits("SW", {say:{voice: "man"}})
        //... more say(), play(), record(), etc.
        .build();
        
    phoneCall.respond(null, response);
   
    
}));

@ianjuma
Copy link
Contributor Author

ianjuma commented Jan 13, 2017

that's perfect! awesome! this will make it so easy to build voice apps on node 💯

@aksalj aksalj removed this from the Version 1 milestone Nov 16, 2017
@ianjuma ianjuma closed this as completed Jan 19, 2018
@danmbeyah
Copy link

danmbeyah commented Aug 26, 2018

@ianjuma is the voice library in AT node package updated with CallHandler()?

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

5 participants