-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Conversation
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" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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/assets/main.css
Outdated
@@ -139,6 +139,7 @@ ul.btn-group { | |||
} | |||
.control.checkbox-container { | |||
flex-flow: unset; | |||
gap: 0.15rem 1rem; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
src/assets/main.css
Outdated
@@ -149,6 +150,10 @@ ul.btn-group { | |||
.control.radio-group span label { | |||
font-weight: unset; | |||
} | |||
.control.select-group { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, will do.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(index)
instead of [index]
.
src/assets/main.css
Outdated
@@ -143,12 +143,19 @@ ul.btn-group { | |||
.control.checkbox-container label { | |||
font-weight: normal; | |||
} | |||
.control.checkbox-group { | |||
flex-flow: unset; | |||
gap: 0.15rem 1rem; |
There was a problem hiding this comment.
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.
src/views/MidiView.vue
Outdated
</span> | ||
</div> | ||
<label>Settings for multichannel-to-equave mode</label> | ||
<div class="control dropdown-group"> |
There was a problem hiding this comment.
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.
The merge conflict needs to be resolved. Rebase on current |
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.