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

fix(color): Fix search for model specific audio files for switches. #4162

Merged
merged 4 commits into from
Oct 30, 2023

Conversation

philmoz
Copy link
Collaborator

@philmoz philmoz commented Oct 5, 2023

Another victim of the switch/ADC refactor PR.

Incorrect indexing of switches when looking for model specific audio file (e.g. SOUNDS/en/modelname folder).
Crashes simulator if model audio file exists.
Loads incorrect audio files on radio.

@raphaelcoeffic
Copy link
Member

raphaelcoeffic commented Oct 5, 2023

@philmoz it seems the old method had a lot of string copies (for each and every file) on top of re-fetching the model audio directory (which accesses the file system) for each and every try. The new method should be much faster. I added some unit-tests as well.

@pfeerick pfeerick added the bug/regression ↩️ A new version of EdgeTX broke something label Oct 6, 2023
@pfeerick pfeerick added this to the 2.10 milestone Oct 6, 2023
@pfeerick pfeerick self-requested a review October 13, 2023 10:44
@pfeerick
Copy link
Member

Did anyone try this on hardware after the last changes? As this isn't working for me on TX16S with a test file set that I just verified still works with 2.9.1. for fpv250, all three positions of SC should say something, as well as SF and SH. SL was a poorly named FM, but still works. Wondering if this is another ugly case of simu not behaving the same as hardware.

pr4162.zip

@philmoz
Copy link
Collaborator Author

philmoz commented Oct 30, 2023

Did anyone try this on hardware after the last changes?

Confirmed - works in simulator, does not work on TX16S.

Cleanup compiler warnings.
@philmoz
Copy link
Collaborator Author

philmoz commented Oct 30, 2023

Should work now.

@pfeerick
Copy link
Member

Yup, that fixed it! :)

@pfeerick pfeerick merged commit 1bb624c into EdgeTX:main Oct 30, 2023
37 checks passed
@philmoz philmoz deleted the fix-model-audio branch February 3, 2024 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/regression ↩️ A new version of EdgeTX broke something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants