Skip to content

Add multiple functions (velocity, mod, chord, chord memory, ...) to sidebar pads in keyboard views (#255) - #901

Merged
m-m-adams merged 27 commits into
SynthstromAudible:communityfrom
robmccoll:feature/isomorphic_kb_velocity
Feb 13, 2024
Merged

Add multiple functions (velocity, mod, chord, chord memory, ...) to sidebar pads in keyboard views (#255)#901
m-m-adams merged 27 commits into
SynthstromAudible:communityfrom
robmccoll:feature/isomorphic_kb_velocity

Conversation

@robmccoll

@robmccoll robmccoll commented Dec 30, 2023

Copy link
Copy Markdown
Contributor

Ready for review:

Inspired by the velocity drums keyboard and #255, this change adds various user-selectable functions to the mute and audition pad columns in the isometric and in-key keyboards. The functions include velocity (VEL), mod wheel (MOD), chord mode (CHRD), chord memory (CMEM), and scale mode (SMOD). There are commented out stubs for adding a beat repeat in the future. Hopefully it is simple to follow the pattern and add more functions over time.

  • For most modes, the current default selection or current selection is white.
  • VEL - red: low on bottom to high scaled linearly. The range can be adjusted by holding the top or bottom pad and scrolling vertically. Hold a pad to change the velocity of notes played while held. Short press a pad to set a new default velocity (within the keyboard - doesn't affect default instrument velocity).
  • MOD - blue: same as VEL, but for mod.
  • CHRD - green: Similar press and hold versus tap behavior to either temporarily set a chord or change the default chord. Default chord is none. You can get back to none by short pressing the current chord.
  • CMEM - cyan: hold down a chord and quickly press and release a pad to learn it to that pad. Shift + pad to clear the chord. Press the pad down to play the chord. Holding the pad down allows you to play notes over the chord without changing the learned chord.
  • SMOD - yellow: Short press to temporarily change scale mode. Long press to permanently change. Major on bottom to Locrian on top. Note that both changes affect the song scale mode either temporarily or permanently.

For follow up:

  • Add more functions.
  • Figure out what needs to be persisted and do that.
  • Share state between isometric, in-key, other children.
  • Possibly change to using a structure with function pointers or an interface class for functions for dispatch rather than a bunch of switch statements.

@m-m-adams

Copy link
Copy Markdown
Collaborator

Awesome thanks! We were thinking of using one bar for velocity and the other for mod wheel (y in the mod matrix), I haven't reviewed your code yet but would that be a feasible option?

@robmccoll

Copy link
Copy Markdown
Contributor Author

@m-m-adams That's much better! Can do - this commit changes to that format and makes the min and max values adjustable (with pads corresponding to a linear range between the current min and max).

ModelStack* modelStack = setupModelStackWithSong(modelStackMemory, currentSong);
ModelStackWithTimelineCounter* modelStackWithTimelineCounter =
modelStack->addTimelineCounter(currentSong->currentClip);
((MelodicInstrument*)currentInstrument())

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is this cast actually safe? I know isometric explicitly doesn't support Kit, but is there some way to verify that the current instrument is a melodic instrument?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's not great since that could change in the future - the overarching class for all clips is "output" - you can check the type by accessing the type field here

const InstrumentType type;

Another solution is just making an empty processParamFromInputMIDIChannel part of the instrument class, that would let it be implemented in future if mod wheel support is added to kits

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added processParamFromInputMIDIChannel to instrument and kit

Comment thread src/deluge/gui/ui/keyboard/layout/isomorphic.cpp Outdated
@PaulFreund

Copy link
Copy Markdown
Collaborator

Hey there :) Great initative! I'm sure you already saw #255. Regarding implementation it would be great if most stuff is higher up than in isomorphic (for example in the layout interface
/ keyboard_screen) because In-Key will benefit the same as Isomorphic and in keyboard_screen. If I remember correctly the default velocity is a system setting so it would be good to use that. From usability I could imagine it would be cool to have a short press to constantly set velocity and long press combined with note input it switches back to the default velocity but that is just an idea.


namespace deluge::gui::ui::keyboard::layout {

static inline void popupUint8(deluge::hid::Display* display, uint8_t val) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This function is useful enough that I think it should live in the display class for convenience

@soymonitus

soymonitus commented Jan 8, 2024

Copy link
Copy Markdown
Collaborator

Please, implement it as an "addon" (flexible, not hardcoded, so its functionality can be applied to any of the two columns) so we later can decide from a menu, which "action" we want for the left column of the keyboard view and which one for the right column. I would love this to be tweakable from the main menu so when more things are implemented (pitch bend, mod wheel, beat repeat), I can, as a user, decide to put Velocity on the right column for example, and Beat Repeat on the left column, and then I have my keyboard views the way I like. Other users may decide that they prefer to have modweel and pitch bend on those two columns... Flexibility would be neat

@seangoodvibes seangoodvibes changed the title Add velocity on sidebar pads in isomorphic keyboard view [DRAFT] Add velocity on sidebar pads in isomorphic keyboard view Jan 9, 2024
@seangoodvibes
seangoodvibes marked this pull request as draft January 9, 2024 00:15
@m-m-adams

Copy link
Copy Markdown
Collaborator

That's easy enough to do in a follow on PR. I think once this is done on the KeyBoardLayout class instead of specifically isomorphic it's good to merge

…opup(uint8_t), handle midi on instrument (noop for kit)
@robmccoll

robmccoll commented Jan 9, 2024

Copy link
Copy Markdown
Contributor Author
  • initialize to instrument default velocity
  • short press to change, long press => return to default on release
  • support for alternative control types
  • fix differing mod shift

@robmccoll

Copy link
Copy Markdown
Contributor Author

Hey there :) Great initative! I'm sure you already saw #255. Regarding implementation it would be great if most stuff is higher up than in isomorphic (for example in the layout interface / keyboard_screen) because In-Key will benefit the same as Isomorphic and in keyboard_screen. If I remember correctly the default velocity is a system setting so it would be good to use that. From usability I could imagine it would be cool to have a short press to constantly set velocity and long press combined with note input it switches back to the default velocity but that is just an idea.

I actually missed #255 somehow 🤦 but I'll try to bring this more in line with the ideas there. I moved the code to an intermediate class ColumnControlsKeyboard instead of the KeyboardLayout (assuming other layouts may be developed that want to use the audition pads for something else) and added support to the In-Key keyboard. Modified the pad press logic to track held keys, but short press / long press is still TODO

@robmccoll

robmccoll commented Jan 14, 2024

Copy link
Copy Markdown
Contributor Author
  • Fixed some fuzzy math problems, un/signed clamping problems
  • Use instrument default velocity
  • Long presses will change a value temporarily and return to the set value on release
  • Short presses change the set value
  • Pressing and holding the top pad in a column lets you use the horizontal scroll to change the function of that column
  • Functions now include velocity (VEL) mod (MOD) chord mode (CHRD) and beat repeat - unfinished (BEAT)
  • Chord mode allows you to toggle back to no chord by short pressing the set value. Chords include 5th, Sus2, Minor, Major, Sus4, Minor7, Dom7, Major7.
  • Beat repeat will need some thought to make sure the timing is correct and that the implementation isn't crazy inefficient. If anyone has suggestions, I'd appreciate hearing them. I looked at the arpeggiator and I'm not sure this can / should be implemented the same way.
  • Still an opportunity to add a chord-saving mode like in Sidebar function in Instrument keyboard layouts #255 along with other functions. The implementation should be fairly easy to add new column functions to.

@robmccoll robmccoll changed the title [DRAFT] Add velocity on sidebar pads in isomorphic keyboard view [DRAFT] Add multiple functions (velocity, mod, chord, chord memory, ...) to sidebar pads in keyboard views (#255) Jan 14, 2024
@robmccoll
robmccoll marked this pull request as ready for review January 14, 2024 19:25
@robmccoll robmccoll changed the title [DRAFT] Add multiple functions (velocity, mod, chord, chord memory, ...) to sidebar pads in keyboard views (#255) Add multiple functions (velocity, mod, chord, chord memory, ...) to sidebar pads in keyboard views (#255) Jan 14, 2024
@robmccoll
robmccoll requested a review from m-m-adams January 14, 2024 21:23
@soymonitus

soymonitus commented Jan 29, 2024

Copy link
Copy Markdown
Collaborator

GENERAL IMPROVEMENTS: As an OLED user I would like to see the full name of the mode instead of the 4-letters abbreviation meant for 7seg users:
CHRD -> Chords
CMEM -> Chord memory
VEL -> Velocity
MODW -> Modwheel
SCAL -> Scales

And that's it. All my comments up to this one are what I found as bugs/considerations/suggestions from my testing.

@robmccoll
robmccoll marked this pull request as draft January 29, 2024 16:14
@robmccoll

Copy link
Copy Markdown
Contributor Author

@m-m-adams Don't you think that modwheel and velocity should set the value to whatever you press without mattering if it is short or long press? It is super weird and non usable that the values jump back to previous value depending on the amount of time you press the pad. Why? It would make this much more usable if no matter the type of click, just in the button press (not release) you set the new value but it doesn't jump back when released.

I can see the use for having things be temporary or the new normal, but I agree that the short-press / long-press seems a little unintuitive and the difference between short and long feels arbitrary. I'm fine with changing or scrapping it depending on what seems reasonable to everyone. I also thought it might be interesting to add a variable slew so that you could get smooth transitions between mod values.

Regarding default velocity, I imagine this has been programmed so that the one set in settings would be the one used when entering new notes in the sequencer manually, but with the keyboard open, it should take the value set in the slidebar.

Yes - this is the intended behavior. The keyboard's velocity is initialized to the instrument's default velocity, but from that point onward, the keyboard's velocity (both the immediate value and the set-by-short-press value that it falls back to) do not affect the default velocity used by the sequencer and are not affected by it.

@robmccoll

Copy link
Copy Markdown
Contributor Author

@soymonitus - thanks so much for all of this! I'm sure it's been frustrating and I'm sorry I didn't catch more of it myself, but I'm glad that you're willing to put in the effort. Keeping the quality of the community firmware up is massively important.

@m-m-adams

m-m-adams commented Jan 29, 2024

Copy link
Copy Markdown
Collaborator

I like the tap to latch/press to be momentary behaviour and it's consistent with performance mode. Found the mod wheel issue, it's the mpe y CC and not the mod wheel CC

case MOD:
if (pad.active) {
modDisplay = modMin + pad.y * modStep;
getCurrentInstrument()->processParamFromInputMIDIChannel(CC_NUMBER_Y_AXIS, modDisplay,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should be CC1 (mod wheel), this records it as MPE y to the currently sounding notes instead

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Actually looking again I think this is right as is, incoming CC1 is converted to this value. This should be fine

@soymonitus

soymonitus commented Jan 29, 2024

Copy link
Copy Markdown
Collaborator

I like the tap to latch/press to be momentary behaviour and it's consistent with performance mode. Found the mod wheel issue, it's the mpe y CC and not the mod wheel CC

but then you cannot operate the modwheel for example as a real touchstrip modwheel, which is what I would expect from it, I mean, sliding my finger over the column to set a value, and don't want the value to jump back arbitrarily based on how fast or slow I did the slide (because some pad detected a short press vs other that detected a long press).

Besides, if @robmccoll adds a slew then we would be in modwheel heaven, like a real touchstrip

This two controls has nothing to do with performance view, they are different purposes

@seangoodvibes seangoodvibes changed the title Add multiple functions (velocity, mod, chord, chord memory, ...) to sidebar pads in keyboard views (#255) [DRAFT] Add multiple functions (velocity, mod, chord, chord memory, ...) to sidebar pads in keyboard views (#255) Feb 10, 2024
- Fix missing check for dead pad presses
- Adjustable SMOD pads to any scale
- Change CMEM behavior to force clearing pads so you can play over it
- Display::popup with long and short text for OLD/7SEG
@robmccoll

robmccoll commented Feb 13, 2024

Copy link
Copy Markdown
Contributor Author

Ok - this is probably ready for review again.

  • Almost all of the major bugs were related to not checking and filtering out key presses in the dead state that I added 🤦 . Any press on another column pad or the keyboard could register a previous release a second time any time in the future.
  • Chord memory is changed to capture on press only when a pad was empty to make playing easier. You must shift+pad to clear. This also fixed the stuck note bug.
  • Scale modes now support all modes. The pads default to the first 7, but press and hold any pad and use the vertical encoder to change the mode associated with that pad.
  • Popup labels have full length strings for OLED (I don't have an OLED, so please test that I didn't mess this up).
  • Added text to the community features markdown about this.

Hopefully the formatting is ok now - I'm having problems with .so resolution using the clang-format in the prepackaged toolchain so I used my own.

Note: This is still using the short tap / long press. Also slew is not implemented. Where did things land on short press / tap? Maybe that could be turned on and off? Are there other UI options?

@robmccoll
robmccoll requested a review from m-m-adams February 13, 2024 01:08
@m-m-adams

Copy link
Copy Markdown
Collaborator

Awesome I'll take another look!

Regarding short/long press time this matches the rest of Deluge (performance view/sticky shift), so while there's some usability issues they should be addressed as a whole and it's not a problem specific to this PR

@m-m-adams
m-m-adams marked this pull request as ready for review February 13, 2024 03:55
@m-m-adams

Copy link
Copy Markdown
Collaborator

You’ve got a conflict, can you merge in community? I believe you just have to keep both

I think it will make the checks run too since it looks like this pr has an old version of the CI config in it

@robmccoll robmccoll changed the title [DRAFT] Add multiple functions (velocity, mod, chord, chord memory, ...) to sidebar pads in keyboard views (#255) Add multiple functions (velocity, mod, chord, chord memory, ...) to sidebar pads in keyboard views (#255) Feb 13, 2024

@m-m-adams m-m-adams left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Awesome work, this is a ton of fun and the performance issues seem resolved. Thanks for the contribution!!

@m-m-adams
m-m-adams added this pull request to the merge queue Feb 13, 2024
Merged via the queue into SynthstromAudible:community with commit 1608cf4 Feb 13, 2024
@soymonitus

soymonitus commented Feb 13, 2024

Copy link
Copy Markdown
Collaborator

This works much better than when i first tested. However, i still think that velocity and modwherl should work differently, by not having a different behavior for long and short press, only short press, so you can slide your finger to change values like a touch strip

@entzmingerc entzmingerc mentioned this pull request Feb 20, 2024
7 tasks
@lobit12

lobit12 commented Feb 28, 2024

Copy link
Copy Markdown

Is it possible to record external midi input as a chord memory?

@sichtbeton

Copy link
Copy Markdown
Contributor

The normal mute pads allow you to exit out of menus immediately, even a couple of layers deep. could this functionality be added to the selectable sidebar functions or could the mute buttons be one of the selectable items?

tastycode pushed a commit to tastycode/DelugeFirmware that referenced this pull request Jul 3, 2024
…idebar pads in keyboard views (SynthstromAudible#255) (SynthstromAudible#901)

* Add velocity on sidebar pads in isomorphic keyboard view

* Change to velocity and mod in sidebar for isometric KB

* Velocity and mod to column controls, add to in_key, Display::displayPopup(uint8_t), handle midi on instrument (noop for kit)

* gitignore vim swap files

* Start at instrument default velocity

* Fix keyboard screen padPressHeld, fix clamping un/signed rollover, short press change, long press return to default

* Chord mode working, switching column functions, more fixes

* Add chord memory CMEM - bugged

* Adding scale mode column control - working

* Add dead pad state to keyboard, fix chord memory, comment out beat repeat

* Change chord memory to save on short press to allow playing over chord

* update for rgb

* format

* show current velocity/mod as white instead of default

* Keyboard column control fixes

- Fix missing check for dead pad presses
- Adjustable SMOD pads to any scale
- Change CMEM behavior to force clearing pads so you can play over it
- Display::popup with long and short text for OLD/7SEG

* Add keyboard column functions to community_features.md

* Formatting

* formatting

---------

Co-authored-by: Rob McColl <rmccoll@seek-now.com>
Co-authored-by: m-m-adams <mark.adams@queensu.ca>
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.

7 participants