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

Make TTSCache safer #2914

Merged
merged 2 commits into from
Jun 10, 2021
Merged

Conversation

forslund
Copy link
Collaborator

@forslund forslund commented Jun 4, 2021

Description

Add __contains__ method to TTSCache, The cache contains a SHA if the SHA is
known and all expected files exists on disk.

This is handles unexpected system events in a more consistent manner and
will still be fast for the case where a new sentence needs to be
synthesized.

How to test

  • Ask Mycroft to "say hello"
  • remove all files in /tmp/mycroft/cache/Mimic2/ (or the TTS you're using)
  • ask Mycroft to "say hello" again
  • verify that an audio is produced

Contributor license agreement signed?

CLA [ yes ]

@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Jun 4, 2021
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

Copy link
Contributor

@krisgesling krisgesling left a comment

Choose a reason for hiding this comment

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

Nice one!

I also verified that removing only the phoneme files works as expected.

Are there any improvements to the TestCache unit tests that we could be making?

@krisgesling krisgesling added the Type: Bug - quick Bug fixes that are quick to review and the implications of the change are clear and contained. label Jun 9, 2021
@forslund
Copy link
Collaborator Author

forslund commented Jun 9, 2021

One can always create more tests :) I'll try to add some

@forslund
Copy link
Collaborator Author

A couple of tests has been added!

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

Copy link
Contributor

@krisgesling krisgesling left a comment

Choose a reason for hiding this comment

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

Yay for tests!

Thanks for putting that together, I think we caught an edge case.

mycroft/tts/cache.py Outdated Show resolved Hide resolved
test/unittests/tts/test_cache.py Show resolved Hide resolved
forslund added 2 commits June 10, 2021 09:33
Add __contains__ method to TTSCache, The cache contains a SHA if the SHA is
known and all expected files exists on disk.

This is handles unexpected system events in a more consistent manner and
will still be fast for the case where a new sentence needs to be
synthetisized.
@forslund forslund force-pushed the feature/safer-tts-cache branch from 49485b6 to 4799caa Compare June 10, 2021 07:33
@forslund
Copy link
Collaborator Author

Thanks for noticing that mistake, Added the "gobo" check and corrected the logic snafu

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@krisgesling krisgesling merged commit 7a9c762 into MycroftAI:dev Jun 10, 2021
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) Type: Bug - quick Bug fixes that are quick to review and the implications of the change are clear and contained.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants