christofmuc / KnobKraft-orm

The KnobKraft Orm - The free modern cross-platform MIDI Sysex Librarian
GNU Affero General Public License v3.0
200 stars 25 forks source link

Interest in Yamaha Reface CP adaptation? #325

Closed milnak closed 2 months ago

milnak commented 4 months ago

I hacked together one for the Reface CP, which is a bit weird as the CP has 1 bank, 1 program (essentially just an "edit buffer"), and no program names.

One issue I'd like to fix is that I'm using a hash to provide a generated name, but it would be nice if I could use the filename from the saved sysex (when imported). Any way to do that?

Let me know if you're interested in a PR and I'll send one.

Andy2No commented 4 months ago

A function for a Python adaptation to retrieve the sysex filename would be good, yes.

Have you done it in Python? If so, you could just attach the file to a comment and anyone who wants to try it could add it manually. Options -> Set User Adaptation Dir... opens a File Open dialog to that folder, but it's usually in C:\Users{YOU}\KnobKraft-Adaptations, in Windows.

You may need to put it in a .zip file, to be allowed to attach it.

milnak commented 4 months ago

Here's my hack -- it seems to work.

christofmuc commented 4 months ago

Wow, awesome! I have to dig into this a bit more. I still have a CP lying around somewhere myself, so I'll definitely have to give it a try.

About the name - the import dialog displays a bulk rename dialog when importing from more than one file. You can get the same functionality by using the Edit.. Bulk rename patches... function in the menu. There is a generate from filename button there. Is a bit indirect, but does what you want, and it even works with a single patch.

milnak commented 4 months ago

"There is a generate from filename button there."

I tried that, but I get "0 files renamed" after choosing it. Is it because my adaptation overrides nameFromDump()?

Edit: That was the issue. I've removed the implementation of that method for now and updated the gist.

christofmuc commented 4 months ago

Interesting. Maybe it is a combination of nameFromDump() and a missing implementation of renamePatch()? I would assume you need both, but would need to check on the concrete logic implemented.

christofmuc commented 2 months ago

Confirmed this was a bug - when nameFromDump() was implemented, but renamePatch() was not, adaptations would not accept the rename command because they would try to change the stored patch data and fail, and not update the name field which is only stored in the database. Not sure how many adaptations were affected, but that was certainly not ideal. Changed it so you would now change the name in the database.

This can now lead to the interesting situation that a patch has two names - one stored in the patch and retrieved by nameFromDump(), and the one in the database. If you now implement renamePatch() later, things will get interesting.