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

Switchable performance files and saving all voice data parameters. #228

Merged
merged 23 commits into from May 28, 2022
Merged

Switchable performance files and saving all voice data parameters. #228

merged 23 commits into from May 28, 2022

Conversation

arsamus
Copy link
Contributor

@arsamus arsamus commented May 16, 2022

This feature allows to save on Performance.ini all voice parameters edited via UI or SysEx.

@probonopd
Copy link
Owner

Thanks @arsamus. What is supposed to happen if VoiceDataN is not there, e.g., in the default performance.ini? Strangely, that sounds different now than without this patch.

@arsamus
Copy link
Contributor Author

arsamus commented May 17, 2022

Thanks @arsamus. What is supposed to happen if VoiceDataN is not there, e.g., in the default performance.ini? Strangely, that sounds different now than without this patch.

For this example, nothing, just random data is apply to parameters, then if you want to load a former performance.ini file you have to go to voice menu and move the encoder 1 step up and 1 step down then save Performance to fill VoiceDataN. On a final release it is possible to define that don't set anything if the value is not properly formated or missing.

Sounds different maybe because for the example I paste the same voice parameter for all TG. But nothing would have change in relation to sound.

@arsamus
Copy link
Contributor Author

arsamus commented May 20, 2022

Hi @probonopd I have an improvement for this feature. If VoiceDataN is not there it will priorice BankNumber and VoiceNumber values, so no random data will be load. I will commit this soon.

Moreover, I also have a solution for #125 switchable performance files (load, save and create), I can commit all in this PR. Maybe after #238 will be merged in order to get that improvement.

@probonopd
Copy link
Owner

Looking forward to it @arsamus.

@arsamus
Copy link
Contributor Author

arsamus commented May 22, 2022

Looking forward to it @arsamus.

Hi @probonopd I've just PR the code for manage switcheable performances and save all voice data parameters on performance, with this patch it's possible to edit all paramenters and saved on the actual performance file or in a new one. On the other hand the new Performance menu allows to switch from diferent saved performance files, very usefull on live performance. To load selected performance it is necessary to double click on it, performance is loaded when [Ld] flag apears on left side of the LCD.

The enhacement is retrocompatible so, the only thing that it have to be done is create a folder named "performance" on root directory. performance.ini is the default performance file and has to be located on root directory, other performances files must be saved in a "performance" named folder. The format file name must be: XXXXXX_NAME.ini, XXXXXX is the incremental index and Name can't exced 14 characters. Example: 000001_Perf1.ini.

If VoiceData# parameter is not set on the file, voice and bank parameters are loaded.

Future improvements:
-Edit name of new performance via UI (now is automatically generated and can only be edited via some file manager application on a computer)
-Edit voice name via UI
-read / save performance from / to USB Pendrive.

@arsamus arsamus changed the title Saving voice data parameters on Performance.ini Switcheable performance files and saving all voice data parameters. May 22, 2022
@probonopd
Copy link
Owner

Thank you very much @arsamus.
Let's test MiniDexed_2022-05-22-acfb6d4.

@miotislucifugis
Copy link

This is super! Trying it now.

@probonopd
Copy link
Owner

probonopd commented May 22, 2022

Works for me. 👍 This is really, really useful.

Couple of questions:

  • Wouldn't it be more logical to have one menu entry "Performance" with submenus "Save" and "Load"?
  • When loading performances, can we make it so that performances are already loaded when the name is displayed, without having to double-click it? Like it works with Voices?
  • If the performance/ directory does not exist yet, can we create it? Currently it says "Complete" but it doesn't seem to save anything when that directory doesn't exist
  • Are the menu entries ordered by number? I have
performance/000002_4xE.Piano+More.ini
performance/000001_4xE.Piano.ini

Would have expected the order in the menu to be: Default, 4xE.Piano, 4xE.Piano+More but it is Default, 4xE.Piano+More, 4xE.Piano.

  • When one tries to save but there is no SD card in the device, it still says "Complete". Can we make it say "Card error" instead?

@arsamus
Copy link
Contributor Author

arsamus commented May 22, 2022

  • Wouldn't it be more logical to have one menu entry "Performance" with submenus "Save" and "Load"?

It could be but from my point of view I think load performance needs to be very easy to get. So first level it could be the best option.

  • When loading performances, can we make it so that performances are already loaded when the name is displayed, without having to double-click it? Like it works with Voices?

I've asked myself this question at the begining, but there are two things here. In live performance if you want to switch from one perf.. to another you would have to pass for all perf.. in between, with de current implementation you activate the new performance just when double click it. The way that Voices works ok for voices and banks because musicians used it in an "experimental mode" when want to find a proper sound, but in performance, they would need to switch between different configs. The other thing here is that, in order to get a easy viable solution and to avoid to do agressive code interventions, performance is loaded from file at the moment you select it and it is not preloaded like voices and banks. Change this implies re-think Performanceconfig class and it dependents, creating arrays for all variables, etc.

  • If the performance/ directory does not exist yet, can we create it? Currently it says "Complete" but it doesn't seem to save anything when that directory doesn't exist

Yes, I've saw that, I think this is easy to fix.

  • Are the menu entries ordered by number? I have
    performance/000002_4xE.Piano+More.ini
    performance/000001_4xE.Piano.ini
    ``
    Would have expected the order in the menu to be: Default, `4xE.Piano`, `4xE.Piano+More` but it is `Default`, `4xE.Piano+More`, `4xE.Piano`.

This would have been easy to implement, let me see how to do it.

  • When one tries to save but there is no SD card in the device, it still says "Complete". Can we make it say "Card error" instead?

Yes, same as 3rd question, this is related to the "saving performance fix" #238 where the function always returns a true value, but I thing it could be avoided in some way.

@miotislucifugis
Copy link

This seems to be working well. no problems here

@arsamus
Copy link
Contributor Author

arsamus commented May 23, 2022

I've just update some minimum changes in order to fix or improve the following things:

  • if "performance" directory does not exist, attemp to create it. If it can't do it, saving new performance show "error" message.
  • Performance files are listed on menu ordered them by ID number (first six numbers) and not by creation date.

Respect to the performance menu. Do you think that Load and Save would have to be grouped in a new menú, or both would have to be on first level menu as it is now? I've commented my opinion above.

@probonopd
Copy link
Owner

Thanks @arsamus. Let's test MiniDexed_2022-05-23-55242cd

@probonopd
Copy link
Owner

probonopd commented May 24, 2022

I am terribly sorry @arsamus but it seems my latest merge has introduced merge conflicts. Before I butcher it, may I ask you to have a look at these? Thanks and sorry for the extra hassle.

@arsamus
Copy link
Contributor Author

arsamus commented May 25, 2022

I am terribly sorry @arsamus but it seems my latest merge has introduced merge conflicts. Before I butcher it, may I ask you to have a look at these? Thanks and sorry for the extra hassle.

No problem @probonopd, done it! actually there was no real conflict

@probonopd
Copy link
Owner

Let's test: MiniDexed_2022-05-25-838edab

@anycolorsmith
Copy link

Wow! It works!

@probonopd probonopd changed the title Switcheable performance files and saving all voice data parameters. Switchable performance files and saving all voice data parameters. May 26, 2022
@JoaquinSainz
Copy link

JoaquinSainz commented May 27, 2022

I just tried this version it works very well, excellent implementation 👍

@arsamus
Copy link
Contributor Author

arsamus commented May 28, 2022

It works as expected!

@probonopd probonopd merged commit 60ab4a5 into probonopd:main May 28, 2022
@probonopd
Copy link
Owner

probonopd commented May 28, 2022

Thank you very much @arsamus 👍

unsigned nLastFileIndex;
unsigned nActualPerformance = 0;
unsigned nMenuSelectedPerformance = 0;
std::string m_nPerformanceFileName[40];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this limit the number of performances to 40? #379

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.

None yet

5 participants