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

Yamaha reface cp adaptation rewrite #346

Closed
wants to merge 8 commits into from

Conversation

milnak
Copy link
Contributor

@milnak milnak commented Oct 8, 2024

Cleaned up code from my original implementation.

@milnak milnak closed this Oct 8, 2024
@milnak milnak reopened this Oct 8, 2024
@milnak milnak changed the title Yamaha reface cp adaptation rewitre Yamaha reface cp adaptation rewrite Oct 8, 2024
return True from needsChannelSpecificDetection for better detection.
@Andy2No
Copy link

Andy2No commented Oct 9, 2024

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
Copy link
Contributor Author

milnak commented Oct 10, 2024

@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
Copy link
Owner

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

@milnak
Copy link
Contributor Author

milnak commented Oct 22, 2024

Yes

@christofmuc
Copy link
Owner

@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
Copy link
Owner

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

@christofmuc christofmuc closed this Nov 2, 2024
@Andy2No
Copy link

Andy2No commented Nov 3, 2024

... seems to link to #360

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.

3 participants