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

Support hierarchic menus #61

Merged
merged 5 commits into from
Apr 3, 2022
Merged

Support hierarchic menus #61

merged 5 commits into from
Apr 3, 2022

Conversation

rsta2
Copy link
Contributor

@rsta2 rsta2 commented Mar 30, 2022

This introduces a new menu engine implemented in the class CUIMenu,
which can be configured with C-tables. This should make it easier to
extend the menus, without modifying the code too much. The UI provides
a main menu, which selects the TG to be edited and a TG sub-menu, which
presents the already known TG parameters (voice bank, voice, volume,
pan, detune and channel). A sub-menu is entered with single click and
left with double click. There are arrows displayed on the LCD, which
show the direction(s), to which the knob can be moved. All TG related
parameters are maintained in the class CMiniDexed now.

This introduces a new menu engine implemented in the class CUIMenu,
which can be configured with C-tables. This should make it easier to
extend the menus, without modifying the code too much. The UI provides
a main menu, which selects the TG to be edited and a TG sub-menu, which
presents the already known TG parameters (voice bank, voice, volume,
pan, detune and channel). A sub-menu is entered with single click and
left with double click. There are arrows displayed on the LCD, which
show the direction(s), to which the knob can be moved. All TG related
parameters are maintained in the class CMiniDexed now.
@probonopd
Copy link
Owner

probonopd commented Mar 31, 2022

Thanks @rsta2. Yes, this is what I had in mind. It works well for me. 👍

Using this UI, I can already imagine the next cool feature...

The only thing that got a bit harder to do than before is to change something for more than one TG. Say, you have 4 TGs active and want to change the voice for all of them. Maybe it's an overstretch, but maybe we could add a press-and-turn-knob shortcut that would go from TG to TG but stay in the currently selected submenu?

@rsta2
Copy link
Contributor Author

rsta2 commented Mar 31, 2022

Maybe it's an overstretch, but maybe we could add a press-and-turn-knob shortcut that would go from TG to TG but stay in the currently selected submenu?

Probably possible. I will deal with this tomorrow and add eventual new commits to this PR. The "Menu home" function can be mapped to triple click then and the system reboot after 5 seconds hold.

@probonopd
Copy link
Owner

What do you think about press-and-turn-knob? To be precise, I mean turn-knob-while-button-is-pressed

@rsta2
Copy link
Contributor Author

rsta2 commented Apr 1, 2022

Good idea. I will see, what I can do.

@rsta2
Copy link
Contributor Author

rsta2 commented Apr 1, 2022

Moving from one TG to another in a sub-menu is in effect a jump from one leaf in the tree structure of the menus over several nodes to another leaf. I was looking for a way to implement this, based on the existing menu stack data structure in the class CUIMenu.

Actually it worked to some degree, but when one left one sub-menu, after changing the TG inside this sub-menu, and entered this sub-menu again, one were in the same TG again, where he were before, when changing the TG, even when in the upper menu, a different TG was selected (difficult to explain).

That's all is a mess, and no one will understand this code after a while. I also have no idea, how this can be realized with a different data structure, maybe with lots of code, but this is something, I tried to prevent for the menu.

So unfortunately there is no solution for the turn-knob-while-switch-pressed function at the moment. Detecting these events itself (switch pressed and turn knob up or down) was not a problem.

@probonopd
Copy link
Owner

probonopd commented Apr 2, 2022

This is how I would imagine one could do it (totally made up pseudo-code):

# When the user goes to any (sub)-menu, save where in the menu structure we are and whether that submenu is global (affects all TGs) or not:
current_menu = [0,3,5] # save where in the menu structure we are
current_menu_is_global = 1 # save whether that submenu is global
# Sixth sub-submenu in fourth sub-menu of the first menu

# On turn-knob-while-switch-pressed, check whether we are in one of the submenus that are TG specific
if current_menu_is_global = 0 {
    if knob_turned_right {
        go_to_menu = current_menu[0], current_menu[1]+1, current_menu[2];
        gotomenu(go_to_menu);
        }
    if knob_turned_left {
        go_to_menu = current_menu[0], current_menu[1]-1, current_menu[2];
        gotomenu(go_to_menu);
    }
}

func gotomenu(menu, submenu, subsubmenu) {
    ...
}

@rsta2
Copy link
Contributor Author

rsta2 commented Apr 2, 2022

This may work with a different implementation of the class CUIMenu, but I cannot see, how it should fit into the provided one. The current menu position is not described by an array of indices here. There are pointers to the parent menu and the current menu, the index of the current menu item in the parent menu, the index of the current selection in the current menu and a parameter, which holds the TG number, the OP number or the index of the voice parameter (the latter two not used yet). These variables all are pushed onto a stack, when the next menu level is entered, and popped from the stack, when it is left.

I do not think, that this must be the perfect implementation. But it's the result of more than one day of work, while I did not know the requirement to change the TG inside a sub-menu. I had more in mind to make it easy, to add all the voice parameters, without the need of much code.

@probonopd
Copy link
Owner

I see. If it's too much work then I don't think it's a must have. I was hoping it could be done without too much extra work.

@rsta2
Copy link
Contributor Author

rsta2 commented Apr 2, 2022

I could add the voice parameters (OP-related and non-OP-related) under "Edit params" (or "Edit Voice"?) to the menu to this PR, to have some added value here. Should not be that much effort. I don't know, if this makes sense at this stage?

@probonopd
Copy link
Owner

Wow, that would be huge 👍

@rsta2
Copy link
Contributor Author

rsta2 commented Apr 3, 2022

;) OK, I will add it.

@rsta2
Copy link
Contributor Author

rsta2 commented Apr 3, 2022

I will add a "Reverb" sub-menu now, and push it together with the "Edit Voice" menu, which is finished already, to be added to this PR.

* Map reverb float parameters to range 0 .. 99
* minidexed: Add global (non-TG) parameters
* minidexed: Protect reverb module with spin lock
@rsta2
Copy link
Contributor Author

rsta2 commented Apr 3, 2022

Here are the added sub-menus "Edit Voice" (after "TG#" > "Channel") and "Reverb" (after "TG8"). The menu items can be re-sorted, if necessary. I cannot hear much influence of the different reverb parameters. The load reaches 80% here on a Raspberry Pi 3B without fast=true.

@probonopd
Copy link
Owner

This is way cool @rsta2! Can't stop playing with it. 👍

@probonopd probonopd merged commit 4610a2a into probonopd:main Apr 3, 2022
@probonopd
Copy link
Owner

I updated the documentation at https://github.com/probonopd/MiniDexed/wiki/Menu and added the menu tree at the bottom.

@rsta2 rsta2 deleted the new-ui branch April 3, 2022 18:05
@rsta2
Copy link
Contributor Author

rsta2 commented Apr 3, 2022

Unfortunately I found a problem in the OP# menus. They always work with TG1, not with the one selected in the main menu. The fix is not difficult. Will do this soon.

@probonopd
Copy link
Owner

Would it be easier to implement the following:
While editing a voice parameter, turn-knob-while-button-is-pressed edits the parameter not just for this TG but for all TGs that are on the same MIDI channel?

@rsta2
Copy link
Contributor Author

rsta2 commented Apr 4, 2022

This would be even more complicated, because the class CUIMenu does not know about the MIDI channel mapping. It can edit the channel for one TG, but the mapping is saved in CMiniDexed and CMIDIDevice for the different devices.

If this is function really important, I will think about it again. Some ideas come after some time.

@probonopd
Copy link
Owner

Would "turn-knob-while-button-is-pressed edits the parameter not just for this TG but for all TGs" be more reasonable for now? (My main use case is still having the same voice play "stacked" on multiple TGs like in the default performance.)

@rsta2
Copy link
Contributor Author

rsta2 commented Apr 4, 2022

I found a solution now for the initially wanted function: Turning the knob, while editing a parameter, switches to the next TG up or down. I could send a PR, if this is OK. Changing a parameter for all TGs at once, would be more complicated again.

@probonopd
Copy link
Owner

Let's go for the less complicated solution then :)

@rsta2
Copy link
Contributor Author

rsta2 commented Apr 4, 2022

See #70.

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