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

GUI: use the MIDI notes provided in the settings #129

Closed
wants to merge 1 commit into from

Conversation

3hhh
Copy link
Contributor

@3hhh 3hhh commented Nov 28, 2023

No description provided.

corrados added a commit that referenced this pull request Nov 29, 2023
…e treated as valid ones

based on Pull Request #129
@corrados
Copy link
Owner

Interesting concept. I've implemented my own version on a update_midi_map branch. I only update on settings load since MIDI notes are very rarely changed and in that case it is ok for me to get the updated MIDI notes after a restart of the GUI.

Anyway, it does not work correctly, unfortunately. Since each pad defines all four MIDI notes but usually only uses a subset of these. But the midi map update assumes that all MIDI notes are valid.

@3hhh
Copy link
Contributor Author

3hhh commented Nov 29, 2023 via email

@corrados
Copy link
Owner

MIDI note 0 cannot be used because it switches different choke methods, see #85. Also, auto pad selection does not work anymore with your implementation.

I have fixed both on my branch and it seems to work OK. Can you please test my branch and give some feedback if you are happy with that implementation, too?

@3hhh
Copy link
Contributor Author

3hhh commented Nov 30, 2023 via email

@corrados
Copy link
Owner

Argh... That always happens if you change something and do not test it. It always fails :-). I'll fix it.
Thanks for testing.

@corrados
Copy link
Owner

Should be fixed now.

@corrados
Copy link
Owner

FYI: Everything is on main branch now. I deleted my intermediate branch.

@3hhh
Copy link
Contributor Author

3hhh commented Nov 30, 2023 via email

@3hhh 3hhh closed this Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants