Fix Mimic 2 cache directory error #2800
Conversation
…y exists but the cache_text.txt file does not.
devops-mycroft
added
the
CLA: Yes
|
Voight Kampff Integration Test Failed (Results). |
krisgesling
added
Status: Accepted
mycroft/tts/cache_handler.py
Outdated
| @@ -45,28 +45,26 @@ | |||
| cache_dialog_path = [res_path, wifi_setup_path] | |||
|
|
|||
|
|
|||
| def generate_cache_text(cache_audio_dir, cache_text_file): | |||
| def generate_cache_text(cache_text_file): | |||
There was a problem hiding this comment.
this is deprecating the old API, maybe throw a deprecation warning and support the argument for a while
There was a problem hiding this comment.
Ah yeah good point.
Should we also make them private methods and only expose a single cache_handler? Or are there uses for these independently?
Actually it would definitely need a little more refactoring to be properly generic.
There was a problem hiding this comment.
i'm not sure this even fits in mycroft-core tbh, feels like an helper script that should not be there in the first place.
but now that its there, there might be users in the wild depending on it, eg, OVOS could be using that to generate its own cache (it isn't afaik)
making them private would also break the api compatibility, so still needs a deprecation warning, that way next major version tells everyone to stop depending on it, and the one after can make them private/remove them
There was a problem hiding this comment.
Yeah, agree we shouldn't break the API.
Given it's currently hardcoded to Mimic2, we could extract it to that plugin. At the moment this seems like the most likely course of action to me.
Alternatively, if there is interest and someone willing to do it, they could refactor it for use in any TTS in which case we could leave it in core as a common method.
krisgesling
added
Status: Change requested
and removed
Status: Accepted
|
Fixed backwards compatibility issue. |
chrisveilleux
force-pushed
the
bugfix/mimic2-cache-dir
branch
from
51d2a26 to
a52d45f
Compare
|
Voight Kampff Integration Test Succeeded (Results) |
|
Voight Kampff Integration Test Failed (Results). |
|
Voight Kampff Integration Test Succeeded (Results) |
mycroft/tts/cache_handler.py
Outdated
| "exists before executing this function. support for this argument " | ||
| "will be removed in version 21.08" | ||
| ) | ||
| os.makedirs(cache_audio_dir) |
There was a problem hiding this comment.
do we need to check if this directory exists before making it?
Description
Fixed an error that occurs when the pre-loaded cache directory already exists but the cache_text.txt file does not.
How to test
Remove the cache_text.txt file from the pre-loaded cache directory and reboot the device. There should be no errors regarding creating the file.
Contributor license agreement signed?
CLA [Y]