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

Added Multichannel-to-equave feature to support Lumatone and other multichannel many-note controllers #642

Closed
wants to merge 0 commits into from

Conversation

000masa000
Copy link
Contributor

Added options in the MIDI tab to select multichannel mode, center channel (1-16), number of equaves (1-16), and equaves down (0-15). Added multichannel index offset calculation using mmod and user-selected parameters. If the index goes outside the 0-127 range, frequency is calculated directly using scale.getFrequency().
TODO: automatically select all channels when toggling multichannel mode.

@000masa000 000masa000 requested a review from frostburn April 11, 2024 20:47
package.json Outdated
@@ -28,7 +28,7 @@
"vue-router": "^4.3.0",
"webmidi": "^3.1.8",
"xen-dev-utils": "^0.2.9",
"xen-midi": "^0.1.3"
"xen-midi": "github:xenharmonic-devs/xen-midi"
Copy link
Member

Choose a reason for hiding this comment

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

Is xen-midi good enough now? I should publish current main then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is fine for multichannel use, please publish! I was thinking of adding another export MidiOutMTS which would give an alternative to multichannel PB using Midi Tuning Standard and sysex (opt in popup for browser users): The logic of retuning in Midi 2.0 is almost identical so the code will be re-useable when sw adds Midi 2.0 compatibility (roadmap 4.0???) Concept is to use the note numbers as slots in which the "actual" neareast MIDInote and needed PB are stored. Then one can simply cycle through the MIDI note slots and recover info from them for per-note stuff (NoteOff Velocity, Poly AT etc.)

Copy link
Member

Choose a reason for hiding this comment

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

Published. Please update to "xen-midi": "^0.2.0"

Please raise a dedicated issue in xen-midi about this MIDI 2.0 stuff, so your ideas won't get lost once this PR is closed.

src/App.vue Outdated Show resolved Hide resolved
@@ -139,6 +139,7 @@ ul.btn-group {
}
.control.checkbox-container {
flex-flow: unset;
gap: 0.15rem 1rem;
Copy link
Member

Choose a reason for hiding this comment

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

This affects all checkboxes in the application. Did you make sure we want this accross the board?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did this because otherwise they cram together without spacing when placed side-by-side. It seems to look OK, what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

It's too wide for my taste to be applied across the app.
A more conservative gap would be a nice addition, though. Some of the labels elements have extra spaces in them which looks bad in template code.

Beyond the scope of this PR anyway. Please include the changes you need under <style scoped> in MidiView.vue.

@@ -149,6 +150,10 @@ ul.btn-group {
.control.radio-group span label {
font-weight: unset;
}
.control.select-group {
Copy link
Member

Choose a reason for hiding this comment

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

"Select" is a particular HTML element. I don't see this being used with that tag. Try to find a synonym for this group type if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for now I replaced "select-group" with "dropdown-group" and also added "checkbox-group" to match the already existing "radio-group". Not sure but it seemed to make sense that a row of each of these HTML elements might need different spacing. Will move this to the new issue #644.

src/App.vue Outdated

// in multichannel-to-equave mode calculate an offset based on the incoming channel
if (midi.multichannelToEquave && channel !== undefined) {
let offset = mmod(channel - midi.multichannelCenter + midi.multichannelEquavesDown, midi.multichannelNumEquaves) - midi.multichannelEquavesDown
Copy link
Member

Choose a reason for hiding this comment

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

const offset; index += offset * scale.scale.size; Might be nicer (prefer const over let when possible).

src/App.vue Outdated
if (rawAttack === undefined) {
rawAttack = 80
}
let frequency = scale.frequencies[index]

let frequency = scale.getFrequency[index]
Copy link
Member

Choose a reason for hiding this comment

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

(index) instead of [index].

@@ -143,12 +143,19 @@ ul.btn-group {
.control.checkbox-container label {
font-weight: normal;
}
.control.checkbox-group {
flex-flow: unset;
gap: 0.15rem 1rem;
Copy link
Member

Choose a reason for hiding this comment

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

We already discussed this. Don't modify global styles in this PR.

</span>
</div>
<label>Settings for multichannel-to-equave mode</label>
<div class="control dropdown-group">
Copy link
Member

Choose a reason for hiding this comment

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

There's nothing dropdown about this group of elements. See btn-dropdown-group on what that means.

@frostburn
Copy link
Member

The merge conflict needs to be resolved. Rebase on current main.

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.

Mapping compatible with Surge XT
2 participants