feat: Adding more natural sounding voices to phone calls #2575

AlaricWhitney · 2022-08-18T22:30:22Z

Identified the issue which this PR solves.
Read the CONTRIBUTING document.
Code builds clean without any errors or warnings.
Added appropriate tests for any new functionality.
All new and existing tests passed.
Added comments in the code, where necessary.
Ran make check to catch common errors. Fixed any that came up.

Description:
This PR updates the application to have be able to set the Twilio Phone call language and Text To Speech engine.

Currently the default language is "" (which Twilio interprets as us-EN). This PR allows you to adjust it to whatever you need.
Currently the default TTS engine is Twilio Basic Man, which sounds like a robot. This PR allows you to adjust it to whatever you need.
The greeting is also adjusted to make it more natural sounding by adjusting the verbiage, and adding a pause between the greeting and the alert message.

Which issue(s) this PR fixes:
Fixes #2574
Fixes #2576

Out of Scope:
NA

Screenshots:
NA

Describe any introduced user-facing changes:
By default, nothing changes for the user, however the user does have the new ability to change the TTS voice engine and the language for phone calls.

Describe any introduced API changes:
NA

Additional Info:
NA

config/config.go

notification/twilio/twiml.go

notification/twilio/voice.go

mastercactapus

I'm going to look into using the XML encoder to deal with the escaping issue (as a separate PR), I already started some of that with the mocktwilio refactor work, so I think it will be straightforward.

config/config_test.go

notification/twilio/twiml.go

notification/twilio/twiml_test.go

notification/twilio/voice.go

config/config.go

AlaricWhitney · 2022-08-24T14:09:33Z

@mastercactapus Cool. I'll wait until your XML PR #2582 goes in before making more updates to this PR.

mastercactapus · 2022-09-27T19:32:08Z

@AlaricWhitney the XML PR has been merged

…en-calls

Co-authored-by: Nathaniel Caza <[email protected]>

stale · 2022-11-12T03:15:50Z

This pull request has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

mastercactapus · 2022-11-28T16:56:44Z

@AlaricWhitney, just a note on this, I haven't had a chance to dig in, but it doesn't seem to work with the Polly voices. The <break> blocks get read out literally. I think it all needs to be wrapped in <speak> or something like that to use the "advanced" syntax (I forget the acronym)

…lert callback

AlaricWhitney · 2022-12-01T23:00:26Z

@mastercactapus This is ready for review again. I've implemented the message to include an extra optional query parameter that indicates where in the string slice to insert pauses. I've also added unit tests and benchmark tests to show how it works. I also moved some of the message functionality into their own separate functions to make unit testing easier.

mastercactapus

Having to manage/track indexes all over the place is going to be error-prone and noisy; we should be using a slice of strings throughout our application code to manage chunks of text.

The request itself can have the slice encoded in a string + []int format, but we should parse that as soon as we get the request into a more usable format of []string, and only switch to indexes at the last moment when we build the URL.

Doing so will let us simplify a lot of this.

notification/twilio/voice.go

mastercactapus · 2022-12-06T19:32:23Z

notification/twilio/voice.go

 	switch call.Digits {
 	default:
 		resp.SayUnknownDigit()
 		fallthrough
 	case "", digitRepeat:
-		resp.Say(call.msgBody)
+		processSayBody(resp, call.msgBody, call.msgPauseIndex)


This feels like it should just be a method

I can do it if you want, although I do think since it's voice specific, so I will make it part of the Voice object.

notification/twilio/voice_test.go

AlaricWhitney · 2022-12-07T16:56:47Z

Having to manage/track indexes all over the place is going to be error-prone and noisy; we should be using a slice of strings throughout our application code to manage chunks of text.

The request itself can have the slice encoded in a string + []int format, but we should parse that as soon as we get the request into a more usable format of []string, and only switch to indexes at the last moment when we build the URL.

Doing so will let us simplify a lot of this.

I chose to do it this way to ensure separation of functionality between what each object is supposed to do, and ensure backwards compatibility. More specifically, I really think that the message injection processing should occur immediately next to where the message is defined. Indexes are ordered, so it shouldn't be an issue to pass it around a little bit.

tony-tvu

Everything is working fine but I'm seeing these logs fatal: No names found, cannot describe anything. which I'm not seeing when running on master branch. Is anyone else seeing this on their on when running on this branch?

Forfold · 2023-02-02T20:33:21Z

Testing this now and not running into that @tony-tvu 🤔 Was this right after running make start, or after performing some action?

config/config.go

…-to-phoen-calls

AlaricWhitney · 2023-02-03T15:20:29Z

@tony-tvu I'm seeing that as well. When I looked it up, it's a git thing. Not sure if it's local to my computer or not.

tony-tvu

I was able to functionally test this with a real phone call and change the voices. Hopefully the issue with build | fatal: No names found, cannot describe anything logs resolves itself when running on GoAlert's repo.

AlaricWhitney added 3 commits August 18, 2022 17:10

Adding more natural sounding voices to phoen calls

4e74658

Linted

6725e0f

removed comment

c7dfe73

mastercactapus requested changes Aug 19, 2022

View reviewed changes

Updates to add voice config

08c939f

AlaricWhitney requested a review from mastercactapus August 19, 2022 20:10

mastercactapus requested changes Aug 22, 2022

View reviewed changes

mastercactapus mentioned this pull request Aug 22, 2022

twilio: use encoding/xml to generate TwiML response #2582

Merged

AlaricWhitney and others added 5 commits October 3, 2022 10:00

Merge branch 'master' into Adding-more-natural-sounding-voices-to-pho…

fbab30c

…en-calls

Update config/config_test.go

7956cbd

Co-authored-by: Nathaniel Caza <[email protected]>

Update notification/twilio/twiml_test.go

e6f4e41

Co-authored-by: Nathaniel Caza <[email protected]>

Updated for voice

612c1e2

changed how the pause occurs

9074ceb

AlaricWhitney requested a review from mastercactapus October 3, 2022 17:53

stale bot added the stale This is inactive label Nov 12, 2022

stale bot removed the stale This is inactive label Nov 28, 2022

AlaricWhitney and others added 6 commits November 29, 2022 11:18

reworking voice response server

6f7489a

Finish the voice TestSent test and began work on testing the Twilio a…

e9f746a

…lert callback

fix voice types to use a new query parameter

35f9a22

fix formatting

3bd19b8

func BenchmarkSpellNumber

038f22d

fix benchmark test

56c6a8b

mastercactapus requested changes Dec 6, 2022

View reviewed changes

updated functions as per feedback

e9540e7

mastercactapus added 2 commits January 12, 2023 11:22

don't close dialog on err

82ebd11

use var for empty declarations

fd51f59

mastercactapus previously approved these changes Jan 12, 2023

View reviewed changes

tony-tvu reviewed Jan 25, 2023

View reviewed changes

Forfold requested changes Feb 2, 2023

View reviewed changes

config/config.go Show resolved Hide resolved

config/config.go Show resolved Hide resolved

AlaricWhitney and others added 2 commits February 3, 2023 08:51

Merge branch 'target:master' into Adding-more-natural-sounding-voices…

f98823d

…-to-phoen-calls

added links to valid TTS voice names

0070a32

AlaricWhitney dismissed mastercactapus’s stale review via 0070a32 February 3, 2023 15:18

AlaricWhitney requested a review from Forfold February 3, 2023 15:19

AlaricWhitney requested review from mastercactapus, tony-tvu and Forfold and removed request for Forfold, mastercactapus and tony-tvu February 3, 2023 15:20

Forfold previously approved these changes Feb 3, 2023

View reviewed changes

AlaricWhitney requested a review from mastercactapus February 7, 2023 23:03

fixing the map config

30b059f

AlaricWhitney dismissed Forfold’s stale review via 30b059f February 8, 2023 17:10

AlaricWhitney requested review from Forfold and mastercactapus and removed request for mastercactapus and Forfold February 8, 2023 17:13

Forfold approved these changes Feb 21, 2023

View reviewed changes

tony-tvu approved these changes Feb 23, 2023

View reviewed changes

mastercactapus approved these changes Feb 23, 2023

View reviewed changes

AlaricWhitney merged commit 1dcdf4c into target:master Feb 23, 2023

AlaricWhitney deleted the Adding-more-natural-sounding-voices-to-phoen-calls branch February 23, 2023 20:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Adding more natural sounding voices to phone calls #2575

feat: Adding more natural sounding voices to phone calls #2575

AlaricWhitney commented Aug 18, 2022 •

edited

Loading

mastercactapus left a comment

AlaricWhitney commented Aug 24, 2022

mastercactapus commented Sep 27, 2022

stale bot commented Nov 12, 2022

mastercactapus commented Nov 28, 2022

AlaricWhitney commented Dec 1, 2022

mastercactapus left a comment

mastercactapus Dec 6, 2022

AlaricWhitney Dec 7, 2022

AlaricWhitney commented Dec 7, 2022

tony-tvu left a comment

Forfold commented Feb 2, 2023 •

edited

Loading

AlaricWhitney commented Feb 3, 2023

tony-tvu left a comment

feat: Adding more natural sounding voices to phone calls #2575

feat: Adding more natural sounding voices to phone calls #2575

Conversation

AlaricWhitney commented Aug 18, 2022 • edited Loading

mastercactapus left a comment

Choose a reason for hiding this comment

AlaricWhitney commented Aug 24, 2022

mastercactapus commented Sep 27, 2022

stale bot commented Nov 12, 2022

mastercactapus commented Nov 28, 2022

AlaricWhitney commented Dec 1, 2022

mastercactapus left a comment

Choose a reason for hiding this comment

mastercactapus Dec 6, 2022

Choose a reason for hiding this comment

AlaricWhitney Dec 7, 2022

Choose a reason for hiding this comment

AlaricWhitney commented Dec 7, 2022

tony-tvu left a comment

Choose a reason for hiding this comment

Forfold commented Feb 2, 2023 • edited Loading

AlaricWhitney commented Feb 3, 2023

tony-tvu left a comment

Choose a reason for hiding this comment

AlaricWhitney commented Aug 18, 2022 •

edited

Loading

Forfold commented Feb 2, 2023 •

edited

Loading