christofmuc / KnobKraft-orm

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

Yamaha reface cp adaptation rewrite #346

Closed milnak closed 3 weeks ago

milnak commented 1 month ago

Cleaned up code from my original implementation.

Andy2No commented 1 month ago

The new automatic naming scheme is the sort of thing I had in mind for the Reface CS, but have you checked that you can rename patches after they're created? It seems just defining nameFromDump() blocks that ability, at least it does in V2.2.3. AFAIK, there haven't been any changes to "The Orm" since that address that.

Here's what I mean by renaming them. On the right of the Library tab, there's a box with details of the selected patch (selected by clicking on it in the grid), which includes a name. You should be able to set a new name there but with nameFromDump() defined, that gets immediately overridden because The Orm thinks it can take it from the patch after the renaming - which it expects to have changed the name in the patch.

See attached sceenshot, with some scribbling to indicate the rename box:

RefaceCS-rename

milnak commented 1 month ago

@Andy2No

The new automatic naming scheme is the sort of thing I had in mind for the Reface CS, but have you checked that you can rename patches after they're created? It seems just defining nameFromDump() blocks that ability, at least it does in V2.2.3. AFAIK, there haven't been any changes to "The Orm" since that address that.

Yeah, I didn't try that. The SysEx literally doesn't have a name, so there's no place to store the name in the message. This is all a virtual name. I haven't found a better way to handle this.

christofmuc commented 1 month ago

@milnak I trust this is ready to be merged and released with the next release?

milnak commented 1 month ago

Yes

christofmuc commented 3 weeks ago

@Andy2No I think I know understand the confusion happening with nameFromDump(): nameFromDump() should be implemented if the name is stored in the sysex, and then renamePatch() is required as well. I think this is not clearly stated in the programming guide.

There is a friendlyProgramName() function, but that is somewhat a misnomer, as it is meant to provide a better name for the program number, not the patch name. it is used to e.g. produce something like "B1-099" instead of the default 199.

There seems to be a "createDefaultName()" function missing which could be used to calculate a nice name until the user has given a better name manually. This could also be used together with isDefaultName() to create a better name in case the patch still has the synth's default name like "BasicProgram".

I will open a new issue for this topic!

christofmuc commented 3 weeks ago

Closed in favor of PR #359, where I could resolve the merge conflict.

Andy2No commented 3 weeks ago

... seems to link to #360