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

Save proper external voice IDs in model files #1000

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

somewhatlurker
Copy link
Contributor

This is one possible fix for #999.

If merged, model files will now save the alert IDs from voice.ini/mp3 file names for custom alerts, instead of the internal index in voice_map.

To ensure old models are loaded correctly, an "mp3-" prefix is added to alert values saved in the new way (all models will be saved with new IDs).
This does have a side effect of making new model files incompatible with old firmware though. Alternatives would be to either not use a prefix and make it possible for old models to load with incorrect IDs when used with a poorly formatted voice.ini (probably not desirable) or use a prefix/suffix that atoi won't error out on (eg, "+" prefix), although that would make the file less readable without writing an explanation comment.

uses "mp3-" prefix to avoid potential issues from loading old models with new logic
(all models will be saved with new IDs)
@TheRealMoeder
Copy link
Contributor

TheRealMoeder commented Sep 16, 2020

From a quick glance this looks good. I actually like the idea of using the mp3-prefix.

This could also be the way to enhance functionality by adding a "beep" prefix to set certain alerts to the beeps defined in sound.ini and making that somehow configurable in the voice config menu. Maybe also an option to integrate vibration setting per alert? Or a combination of them? We at least should be discussing things so we're not losing backwards compatibility in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants