qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
18.08k stars 38.88k forks source link

Need help implementing midi chord keys/functions in keymap #1301

Closed antelaurijssen closed 7 years ago

antelaurijssen commented 7 years ago

HI everyone. I've recently put together an S60X keyboard kit and am finding my way around QMK fairly well, even though I am not a programmer. However, I have hit a wall trying to implement custom midi chord keys. Here is what I want to do:

1) Create user defined mid chord keys. So far, I think I have done this correctly in my keymap (https://github.com/antelaurijssen/qmk_firmware/blob/master/keyboards/s60_x/keymaps/bluebear/keymap.c) 2) Create a function that would be called whenever one of these keys is pressed. I'm thinking the function would take two parameters, mode and root, and look something like this:

void midi_chord(uint8_t mode, uint16_t root) - where mode would be a number representing whether the chord is to be processed as a major or minor triad (0 = major and 1 = minor, with the possibility of adding more. I could create an enum to do that), and root would be the root note of the chord, using the already existing midi keycodes MI_C, MI_D, etc. as the value to pass. I have used code that I got from process_midi.c and process_midi.h in my function, thinking it would be the way to process this. But I'm not sure how to get this all to work.

So far I have tried using macros to try to send register_code16(MI_C), etc, but that didn't work. But I think having a function is a more elegant way of doing things, and would be better than writing a macro for each chord. I would really appreciate if someone could have a look and point me in the right direction. This is quite the learning curve!

All the code I have written so far is in my keymap.c that I have linked. Any help would be greatly appreciated!

Thanks!

fredizzimo commented 7 years ago

Looking at your code, it looks like you are on the right path in the commented code, using midi_compute_note and midi_send_noteon. You didn't tell exactly what problem you have though, so its hard for me to spot any bugs.

antelaurijssen commented 7 years ago
Hi Fredizzimo. Thanks for the reply. I've been playing around with the code a little, and have updated it a little and uncommented it so you can have a look. I cannot get it to compile, and get this error right now: Compiling: keyboards/s60_x/keymaps/bluebear/keymap.c keyboards/s60_x/keymaps/bluebear/keymap.c: In function ‘action_function’: keyboards/s60_x/keymaps/bluebear/keymap.c:652:3: error: implicit declaration of function ‘compute_velocity’ [-Werror=implicit-function-declaration] uint8_t velocity = compute_velocity(midi_config.velocity); ^ keyboards/s60_x/keymaps/bluebear/keymap.c:685:3: error: ‘root’ may be used uninitialized in this function [-Werror=maybe-uninitialized] midi_send_noteon(&midi_device, channel, root, velocity); ^ keyboards/s60_x/keymaps/bluebear/keymap.c:686:3: error: ‘major_third’ may be used uninitialized in this function [-Werror=maybe-uninitialized] midi_send_noteon(&midi_device, channel, major_third, velocity); ^ keyboards/s60_x/keymaps/bluebear/keymap.c:687:3: error: ‘fifth’ may be used uninitialized in this function [-Werror=maybe-uninitialized] midi_send_noteon(&midi_device, channel, fifth, velocity); ^ keyboards/s60_x/keymaps/bluebear/keymap.c:697:3: error: ‘minor_third’ may be used uninitialized in this function [-Werror=maybe-uninitialized] midi_send_noteon(&midi_device, channel, minor_third, velocity); ^ cc1: all warnings being treated as errors [ERRORS]

tmk_core/rules.mk:358: recipe for target '.build/obj_s60_x_rgb_bluebear/keyboards/s60_x/keymaps/bluebear/keymap.o' failed make[1]: [.build/obj_s60_x_rgb_bluebear/keyboards/s60_x/keymaps/bluebear/keymap.o] Error 1 ../../../Makefile:496: recipe for target 'bluebear' failed make: [bluebear] Error 1 Make finished with errors

If you could point me in the right direction, I would appreciate it. I'm guessing I'm doing something wrong with my variables... And I'm not sure what implicit declaration of function means either... ;-)

Thanks for your help!

fredizzimo commented 7 years ago

compute_velocity is an internal funciton so it can't be called. It could be exposed, but I looked a bit closer at the code and there's antother way. You can call process_midi manually.

void action_function(keyrecord_t *record, uint8_t id, uint8_t opt) {
    uint16_t root_note;
    switch (opt) {
    case 0: //Root note C
        root_note = MI_C;
        break;
    case 1: //Root note C#/Db
        root_note = MI_Cs;
        break;
    case 2: // Root note D
        root_note = MI_D;
        break;
    case 3: // Root note D#/Eb
        root_note = MI_Ds;
        break;
    case 4: // Root note E
        root_note = MI_E;
        break;
    case 5: // Root note F
        root_note = MI_F;
        break;
    case 6: // Root note F#/Gb
        root_note = MI_Fs;
        break;
    case 7: // Root note G
        root_note = MI_G;
        break;
    case 8: // Root note G#/Ab
        root_note = MI_Gs;
        break;
    case 9: // Root note A
        root_note = MI_A;
        break;
    case 10: // Root note A#/Bb
        root_note = MI_As;
        break;
    case 11: // Root note B
        root_note = MI_B;
        break;
    }
    uint8_t root = midi_compute_note(root_note);
    uint8_t major_third = root_note + 4;
    uint8_t minor_third = root_note + 3;
    uint8_t fifth = root_note + 7;
    switch (id) {
    case 0: //Major chord
        process_midi(root, record);
        process_midi(major_third, record);
        process_midi(fifth, record);
        break;
    case 1: //Minor chord
        process_midi(root, record);
        process_midi(minor_third, record);
        process_midi(fifth, record);
        break;
    }
}

I also added break after handling the switch case. Otherwise it will just fall through to the next case. I also moved out the declaration of variables from the switch, as I think that's what the second part of your errors were complaining about.

Also note that I'm passing the record futher to process_midi, since then it automatically handles the key down and key up events.

I haven't tried compiling the code, let alone testing it, but I hope this will let you process further.

antelaurijssen commented 7 years ago

Hi Fredizzimo. Thanks for the pointers. I tried your code, and still no luck but I feel maybe it's getting closer. At first I was still getting an error about root_note being used unitialized, so I gave it a starting value of MIDI_INVALID_NOTE, which let me at least compile without errors. The value should change depending on the opt value anyways. But still, I got nothing when I pressed those midi chord keys. I'm still confused at this point as to what the value of the midi note keys are that are passed to process_midi... those midi keys are defined under quantum_keycodes.h in the quantum_keycodes enum, and I don't know enough at this point about programming to know what their values are, which is not helping me understand all of this. MI_C = MIDI_TONE_MIN, but what is the value of MIDI_TONE_MIN? Anyways, I played with the code a little more, and tried register_keycode16(midi note keycode) instead of process_midi, but no luck either. I like your idea of calling process_midi directly, but there's something I'm missing to get this to work... Thanks again for your help! I appreciate you helping me out with this. It's a very interesting learning experience.

fredizzimo commented 7 years ago

Ah yes, I missed the potentially uninitialized root_note.

You don't need to care about MIDI_TONE_MIN, it's only used internally to calculate the right tone.

I'm still not sure what's wrong, but I assume that MIDI normally works, so you have everything that you need enabled. I don't think your current version with register_code16 works though, but the version I provided with process_midi should.

I think the easiest way to debug this would be to enable the debug prints. For that you will need to enable the console. CONSOLE_ENABLE=yes in your keymap makefile. Then run hid listen on your computer.

But that's not enough to enable the dprintf prints. For that you need to enable the debug prints using the magic key LSHFT+RSHFT+D, which requires COMMAND_ENABLE=yes in your makefile. But seeing that you don't have the shifts mapped normally, I don't think that works. So the alternative is to map the DEBUG key in some of your layers.

After the debug prints are enable you should get some output in the hid_listen console each time you attempt to play a midi note.

antelaurijssen commented 7 years ago

Let me try something. I just spotted a problem that could be the reason why this isn't working. My keys are defined as ACTION_FUNCTION_OPT(id, opt), but in the function code we used ACTION_FUNCTION... oops!

antelaurijssen commented 7 years ago

So, I put your code back, minus a couple of little things. I got rid of root = compute_midi_note(root_note), since process_midi takes root_note directly (since root note is the Midi keycode). compute_midi_note gives back a midi note number, which is not what I want. Still no luck. I also enabled all the options in my makefile to debug, and added a debug key, which does not seem to be changing anything to hid_listen. When I run hid_listen, it shows me whatever key I press, except with the midi note keys. That shows nothing. And pressing the DEBUG key changes nothing... I'm heading out until tomorrow so cannot try any changes until then. But I'll keep going at it! Let me know if you find anything in the meantime. I really appreciate your help!

fredizzimo commented 7 years ago

I don't have any more advice at the moment, but from what I can see in the code, when you play a normal midi note through a mapped key, it should output this

dprintf("midi noteon channel:%d note:%d velocity:%d\n", channel, note, velocity);

And the same thing when you call process_midi manually.

antelaurijssen commented 7 years ago

I got it to work this morning! the function itself seemed to be ok, but I wasn't using action_function correctly. I was also defining some variables as uint8_t when they should have been uint16_t. Have a look at my code and see how I did that. I also did get debug to work. You know how they say RTFM?... I did... ;-) Anyways, I'm not sure why it wasn't working when I was using the action_function directly without using F(n), but that's my next step, since I don't want to be limited to 32 chords. But the thing that I can't figure out is why certain chords are lagging. Some switch on intstantly, whereas others lag by about half a second on their first press... There was nothing different in debug when pressing these. Thanks for helping me with this one! It's lots of fun to learn something new.

antelaurijssen commented 7 years ago

And just FYI, the chords that are lagging are the ones with root notes Gs/Ab, A, As/Bb, and B. Not sure why.

fredizzimo commented 7 years ago

I'm not sure what the cause is, but those notes corresponds to the range where the major chord needs the third from the next octave. Does Gs/Ab work properly for minor chords? If they do then I'm pretty sure it's related, otherwise it could just be coincidence.

antelaurijssen commented 7 years ago

I thought that might be the case, but GMajor chord needs the D from the next octave, and that one does not lag. But those four chords with the Gs/Ab, A, As/Bb, B root notes lag in both Major and Minor. I'm really stumped as to why...

antelaurijssen commented 7 years ago

So I played around with the chords and added another layer of chords only, like on an accordion. Anyways, I still get those lagging chords for the same root notes on all four chord types and I just can't figure out why. If anyone has any ideas, let me know!

https://github.com/antelaurijssen/qmk_firmware/blob/master/keyboards/s60_x/keymaps/bluebear/keymap.c

fredizzimo commented 7 years ago

Does the same chords work propelry if you press the three keys manually?

fredizzimo commented 7 years ago

Another thing that you could try now, is to disable the console and debugging, because that could slow things down.

antelaurijssen commented 7 years ago

Hey Fredizzimo. Thanks for the reply. The console/debug are currently disabled, because my .hex file was too big if I kept those enabled. So I don't think that is the problem. And if I manually make the chords I do not get any lag at all. They only lag when done through my chord function. And only those with those 4 root notes.

fredizzimo commented 7 years ago

I don't have any idea what could be wrong, but here are a couple of more questions that could help with identifying the issue.

Does all notes of the chord lag, or just the first of them for example? And does it happen 100% each time you try to play it? Do you notice something similar when releasing it as well?

antelaurijssen commented 7 years ago

It is kind of odd... Here is what happens with chords with root notes Gs/Ab, A, As/Bb, B: 1) Pressing on the key does not sound the chord instantly like with the other root notes, but lags about a half second. Enough that it is very noticeable. 2) When key is released, chord stops instantly, so no issue there. 3) If I press the key a bunch of times in quick succession, it only lags the first time. But if I release the key and wait a little longer before pressing (like a half second or so), it will lag again. 4) I also thought it could be because two of the notes have to be computed from the next octave up, but that's not the case because G#minor also lags, and it has only one note in the higher octave.

fredizzimo commented 7 years ago

I thought more about it and checked the code, and I found the explanation in action_code.h

enum function_opts {
    FUNC_TAP = 0x8,     /* indciates function is tappable */
};
#define ACTION_FUNCTION(id)             ACTION(ACT_FUNCTION, (id))
#define ACTION_FUNCTION_TAP(id)         ACTION(ACT_FUNCTION, FUNC_TAP<<8 | (id))
#define ACTION_FUNCTION_OPT(id, opt)    ACTION(ACT_FUNCTION, (opt)<<8 | (id))

The fact that one bit is used for FUNC_TAP, leaves only 3 bits available for opt, which corresponds to a maximum value of 7 or G the last working one. Gs is equal to 8, and therefore identified as a TAP action., which I suppose has a timeout of around a half second.

In this case the fix is easy, just switch the opt and id parameters, since the id has 4 bits available, or a maximum value of 16

antelaurijssen commented 7 years ago

Fredizzimo, that is awesome, and very interesting indeed! I will give that a try when I get back home tonight. I'm wondering whether I can come up with something a little simpler through process_record_user, and not have that limitation on the number of opts, but that's for another day. At least I know one way to get my midi chord function to work! Thank you so much for your time with this. I'll report back once I have tried it, and probably you can mark the issue closed after that.

fredizzimo commented 7 years ago

Yes, you should be able to use process_record_user and SAFE_RANGE to get a bigger range. I think a little bit less than 1000 keycodes are available right now, but that should be enough.

I don't think there's any documentation right now, but you can take a look at the default planck keymap for an example.

antelaurijssen commented 7 years ago

I originally thought of doing it that way, but your are right, there isn't much documentation on how to do that. And I don't know if I could pass arguments to those keys... like passing root note and chord type. I really like the function that you helped me write, but it's all the stuff around action_function_opt that I find redundant. I just want a keypress that calls for example a function midi_chord(root_note, chord_type) without all the PROGMEM fn_actions[] stuff.

antelaurijssen commented 7 years ago

Well Fredizzimo, I have switched opt and id and it now works! Thank you so much for your help with this. I will try my hand at writing something using process_record_user once I learn a little more, but I have acheived what I wanted. Lots of learning in the process as well!