Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Use speak2mary for MaryTTS integration and enable sound effects #30805

Merged
merged 2 commits into from Jan 23, 2020

Conversation

@Poeschl
Copy link
Contributor

Poeschl commented Jan 15, 2020

Breaking Change:

The codec and locale configuration changed to the official config keys from MaryTTS.

en-GB, en-US -> en_GB,en_US
aiff,au,wav -> AIFF_FILE,AU_FILE,WAVE_FILE

Description:

I extracted the MaryTTS network logic into a external package and added the effects settings.

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#11759

Example entry for configuration.yaml (if applicable):

# Example configuration.yaml entry
tts:
  - platform: marytts
    host: 'localhost'
    port: 59125
    codec: 'WAVE_FILE'
    voice: 'cmu-slt-hsmm'
    language: 'en_US'
    effect:
      Volume: "amount:2.0;",
      TractScaler: "amount:1.5;",
      F0Scale: "f0Scale:2.0;",
      F0Add: "f0Add:50.0;",
      Rate: "durScale:1.5;",
      Robot: "amount:100.0;",
      Whisper: "amount:100.0;",
      Stadium: "amount:100.0",
      Chorus: "delay1:466;amp1:0.54;delay2:600;amp2:-0.10;delay3:250;amp3:0.30",
      FIRFilter: "type:3;fc1:500.0;fc2:2000.0",
      JetPilot: ""

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.
@project-bot project-bot bot added this to Needs review in Dev Jan 15, 2020
@Poeschl Poeschl mentioned this pull request Jan 15, 2020
2 of 2 tasks complete
@Poeschl Poeschl marked this pull request as ready for review Jan 15, 2020
@Poeschl Poeschl force-pushed the Poeschl:marytts branch from 1eeab2c to 6f67e42 Jan 16, 2020
@property
def default_options(self):
"""Return dict include default options."""
return {CONF_EFFECT: DEFAULT_EFFECTS}

This comment has been minimized.

Copy link
@pvizeli

pvizeli Jan 17, 2020

Member

Why is that static? The user can overwrite the defaults per initial config

This comment has been minimized.

Copy link
@Poeschl

Poeschl Jan 19, 2020

Author Contributor

Jup, you are correct. With the latest commit, the defaults are the initial effect settings

@Poeschl Poeschl requested a review from pvizeli Jan 19, 2020
return (None, None)
data = await request.read()
if options is not None and CONF_EFFECT in options:
effects.update(options[CONF_EFFECT])

This comment has been minimized.

Copy link
@pvizeli

pvizeli Jan 20, 2020

Member

that means, you will be never able to overwrite the effects from config. Maybe:
effects = options[CONF_EFFECT]?

This comment has been minimized.

Copy link
@Poeschl

Poeschl Jan 21, 2020

Author Contributor

I understood the options parameter as additional options during the service call.
Can you explain where the options parameter get its value?

I intended to use the config from yaml and let it be overwritten by service call options.

This comment has been minimized.

Copy link
@pvizeli

pvizeli Jan 21, 2020

Member

options are they from the service call. I want only prevent to have a bug :)

This comment has been minimized.

Copy link
@Poeschl

Poeschl Jan 21, 2020

Author Contributor

Since the file config is coming as default, it will change it to your suggestion.

@Poeschl Poeschl force-pushed the Poeschl:marytts branch from c45f2e9 to 796f4e2 Jan 21, 2020
@Poeschl Poeschl requested a review from pvizeli Jan 21, 2020
Dev automation moved this from Needs review to Reviewer approved Jan 23, 2020
@pvizeli pvizeli merged commit 7fed328 into home-assistant:dev Jan 23, 2020
10 checks passed
10 checks passed
CI Build #20200121.54 succeeded
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (FullCheck Pylint) FullCheck Pylint succeeded
Details
CI (Overview CheckFormat) Overview CheckFormat succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
CI (Tests PyTest Python37) Tests PyTest Python37 succeeded
Details
cla-bot Everyone involved has signed the CLA
codecov/patch 100% of diff hit (target 94.58%)
Details
codecov/project 94.6% (target 90%)
Details
Dev automation moved this from Reviewer approved to Done Jan 23, 2020
@lock lock bot locked and limited conversation to collaborators Jan 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.