Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

Feature/mic_config - Add configuration option for microphone settings #2536

Merged
merged 1 commit into from
Apr 17, 2020

Conversation

AIIX
Copy link
Collaborator

@AIIX AIIX commented Apr 14, 2020

Description

Adds a configuration option to set a custom timeout for microphone settings

How to test

Set configuration in USER / SYSTEM level mycroft.conf file to override default values

"microphone": {
    "recording_timeout": 10.0,
    "recording_timeout_with_silence": 3.0
}

Contributor license agreement signed?

CLA [x] (Whether you have signed a CLA - Contributor Licensing Agreement

@pep8speaks
Copy link

pep8speaks commented Apr 14, 2020

Hello @AIIX! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-04-17 09:26:31 UTC

@devs-mycroft devs-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Apr 14, 2020
@AIIX AIIX requested a review from forslund April 14, 2020 08:08
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

Copy link
Collaborator

@forslund forslund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a couple of minor comments, mainly organizational things.

mycroft/configuration/mycroft.conf Show resolved Hide resolved
@@ -199,11 +201,13 @@ class ResponsiveRecognizer(speech_recognition.Recognizer):

# The maximum seconds a phrase can be recorded,
# provided there is noise the entire time
RECORDING_TIMEOUT = 10.0
RECORDING_TIMEOUT = MIC_CONFIG.get(
'microphone', {}).get('recording_timeout')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In python code style capital letters indicate constants, since these are no longer constants maybe we should make them lowercase object member and set them in __init__()?

self.recording_timeout = config['recording_timeout']

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved in newer commit to class init and followed code style of existing properties

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

listener->recording_timeout: Maximum length of recording
listener->recording_timeout: Maximum length of silence before stopping
recording
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@forslund forslund merged commit d4f59bf into MycroftAI:dev Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants