[Bug?] Custom voice alerts assume contiguous and ascending ordering in voice.ini

[somewhatlurker]

Not necessarily a huge issue, but I noticed it when playing around in emulator.
Custom alerts in voice_map seem to be created by writing to indexes 200+ in the order alerts are found in the file, not corresponding to the key in voice.ini.
While the firmware seems to handle this fine internally (the order of alerts in the selector changes, but they're played using the correct id from voice_map), model files just store the internally used index and will break if voice.ini is merely reordered.

For example, two voice.ini files containing 200=0,300\n201=1,300 and 201=1,300\n200=0,300 in the custom section are handled differently. (In the second case, alert 200 in model files is actually 1 (mp3 file 201) and alert 201 is 0 (mp3 file 200))
It's a very non-intuitive behaviour and I assume it's not intentional.

　

It seems pretty simple to fix; ids in the model file should just have to be converted to the proper internal id:

when saving: (src/config/model.c line 1385 -- near fprintf(fh, "[%s]\n", SECTION_VOICE); ?) look up the correct id of music in voice_map

when loading: (src/config/model.c line 1030 -- near if (MATCH_SECTION(SECTION_VOICE)) { ?) iterate on voice_map until an entry with id == val is found (a little slow, but probably fine) if no match return 0

[eried]

That is why I just append new voices to the end and have a excel to create the file just in case. But it would be nice if the file supported non-contiguous

[TheRealMoeder]

I was never overly satisfied with the whole voice handling, but it grew out of limits due to unidirectional communication with dfplayer and ram limitations on some targets. If we use internal id's in the model files, those can't be edited manually anymore, which is also not a good solution.

[somewhatlurker]

The current implementation already uses internal ids in model files, they just happen to look like the real mp3 ids. Manual editing shouldn't get any harder after the change.

Although on the topic of causing new issues, I realise now that that naive fix could screw up model files for users who have non-standard edited voice files that unknowingly hit this bug.
That could be fixed by being able to read two types of value, such as HOLD0=200 for old loading behaviour and HOLD0=mp3-200 for new loading behaviour -- then old config can't be broken (I'm not sure what an appropriate prefix would be though; mp3- or ini- seem appropriate, but aren't very descriptive)

[somewhatlurker]

I went ahead and made a branch with this fixed: https://github.com/somewhatlurker/deviation/tree/model-voiceid-fix Feel free to play around with it and see if you think it makes sense.

[TheRealMoeder]

Can you make a PR? It's always easier to have Travis checking things early on.

[somewhatlurker]

Sure