Refactor Lily58 to use split_common

[yyc]

Now that split_common is pretty mature, it might be a good idea to get rid of all the duplicated functionality and just use the common libraries so we can piggyback on those updates.

Description

This removes the duplicated code (matrix.c, split_util.c, etc.), and updates the build files to automatically include the split_common files.

Then, the OLED code is refactored to use the core OLED library, so we can remove ssd1306.c as well. All the widgets are also updated for compatibility with the new core OLED library. The default keymap has also been updated to work with the core and OLED libraries. For a guide on how to update your own keymap for compatibility, take a look at the default keymap refactor.

This is tested to work with two halves communicating over serial, with i2c communication on both OLEDs. (sidenote: does i2c over the two halves even work? I couldn't get it to work even with the original repo)

My build doesn't have ws2812 underlighting, so I'm unable to test whether that's working. Help on that would be appreciated.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[yyc]

@kata0510 would appreciate you taking a look at this :)

@drashna addresses some of your concerns with the Helix code that was originally used

[MysticMixles]

I'll test encoder functionality, backlighting, and anything else I can think of, and report back with any issues I find.

[drashna]

To be blunt, it may be much better to move the OLED code all into the keymap.c file, so that it's accessible and editable there, rather than relying on the "lib" stuff.

This is especially true of the layer code.

Also, why not 270 degree rotation? :)

[drashna]

This is tested to work with two halves communicating over serial, with i2c communication on both OLEDs. (sidenote: does i2c over the two halves even work? I couldn't get it to work even with the original repo)

If this is like the helix, I think you have to cut a trace (between two pads), and add 2x resistors on one of the sides for it to work properly.

Additionally, you can't have OLED's on both sides in this case, since they use the same i2c address, which is a big no-no.

[MysticMixles]

• OLEDs appear to be completely non-functional

• Encoder doesn't appear to work on either side of the keyboard using the same changes as work on the main branch, which include enabling encoders, defining the pads, defining the resolution, and adding in the function to monitor the encoder.

• RGB doesn't appear to work - triggering RGB_MOD doesn't do anything, and adding custom rgb code in the keymap.c file as I do on the main branch results in implicit declaration errors of rgblight_enable and rgblight_sethsv

[yyc]

@MysticMixles

• Do the OLEDs not work even with the default keymap? Or if you're using your own keymap, make sure to add OLED_DRIVER_ENABLE= yes to your rules.mk
• can I see your code for enabling the encoder? I have an encoder lying around somewhere and I can test with that. (sidenote: I'm interested to see how your encoder is integrated into your keyboard, if you won't mind sharing 😃 )
• For RGB, can you try directly calling rgblight_enable_noeeprom() in your keymap's matrix_init_user function? I don't think RGB_MOD will turn on the LEDs if they're off.

[MysticMixles]

@ycc

• Do the OLEDs not work even with the default keymap? Or if you're using your own keymap, make sure to add OLED_DRIVER_ENABLE= yes to your rules.mk

I just added OLED_DRIVER_ENABLE= yes to my rules.mk, but both OLEDs are still turned off.

• can I see your code for enabling the encoder? I have an encoder lying around somewhere and I can test with that. (sidenote: I'm interested to see how your encoder is integrated into your keyboard, if you won't mind sharing 😃 )

void encoder_update_user(uint8_t index, bool clockwise) { if (index == 0) { if (clockwise) { tap_code(KC_VOLU); } else { tap_code(KC_VOLD); } } }

This is the code I'm using and it works, assuming the encoder is on the master side - the main branch's custom split code doesn't let an encoder work on the slave side at all, as far as I can tell.

• For RGB, can you try directly calling rgblight_enable_noeeprom() in your keymap's matrix_init_user function? I don't think RGB_MOD will turn on the LEDs if they're off.

I didn't realize that rgblight wasn't enabled by default, I just turned it on and it works. However, if I call rgblight_sethsv, I still get errors that it hasn't been defined.

[MysticMixles]

Also, the RGB only works on the master side of the keyboard - whatever side I plug the usb cable into lights up, and the other side is dark.

[MysticMixles]

Looking at the code, I'm not super familiar with QMK, but I actually don't see where the OLED gets turned on - in the main branch there is a #define SSD1306OLED, and in the keymap code there is some OLED code, and in this branch there isn't any - I don't know if this is causing the issue, or just because of the split common changes, but it is the first thing I jumped to.

[MysticMixles]

Oh, and for installing the encoder, I used these, and bent all of the pins to be flat with the pcb, other than the mounting legs, which I put into the holes that a PCB mount switch would use. This holds pretty tight, and probably isn't great for the keyboard if you reinstall several times, but works fine for me.

[drashna]

@MysticMixles I believe that for the SSD1306 type code, it gets turned on by keypress, and statup.

[MysticMixles]

@MysticMixles I believe that for the SSD1306 type code, it gets turned on by keypress, and statup.

Then it is broken - when I flashed the main branch the screens worked as expected

[drashna]

Well, for the OLED Driver feature, it runs it from keyboard_task, and is triggered by a matrix change. That's why it doesn't work right on the non-split common stuff.

[yyc]

Looking at the code, I'm not super familiar with QMK, but I actually don't see where the OLED gets turned on - in the main branch there is a #define SSD1306OLED, and in the keymap code there is some OLED code, and in this branch there isn't any - I don't know if this is causing the issue, or just because of the split common changes, but it is the first thing I jumped to.

As long as OLED_DRIVER_ENABLE is set and oled_task_user(void) is defined, the OLEDs should be set up automatically. It's in the oled driver documentation. Can you try using the default keymap in this branch and see if that works for you?

Also, the RGB only works on the master side of the keyboard - whatever side I plug the usb cable into lights up, and the other side is dark.

Typically you would wire the RGBs the other way round, and have the LED data pin be wired to the unused TRRS connector, so that the master would do all the work of controlling the RGBs. In this case, since you've wired the data pin on both sides to the pro micro, you would probably need to enable RGB_SPLIT documented here to get it working.

Disclaimer: my lily58 isn't wired with RGBs so I have no way of testing this, but I have gotten the same setup working with an iris split keyboard.

re: the encoders, still trying to get encoders working with my keyboard. I'll keep you updated.

[yyc]

void encoder_update_user(uint8_t index, bool clockwise) { if (index == 0) { if (clockwise) { tap_code(KC_VOLU); } else { tap_code(KC_VOLD); } } }

Actually, according to this thread, if your encoder is on the slave side you should change the index to 1 (or if you have just one encoder, get rid of the index-checking altogether and just have:

void encoder_update_user(uint8_t index, bool clockwise) {
  if (clockwise) { tap_code(KC_VOLU); }
  else { tap_code(KC_VOLD); }
}

[MysticMixles]

@yyc The encoder doesn't work on either side of the keyboard, using the default keymap with that bit of code added and encoders enabled. I haven't tried the no index method, but it doesn't work on the master side with an index of either 1 or 0.

I'll play with RGB_SPLIT in a few hours to see if that works.

Regarding the OLEDs, using the default keymap they don't appear to work.

[MysticMixles]

I still can't call rgblight_sethsv - it is undefined, even after adding RGB_SPLIT.

Also, using the index-less encoder code, it doesn't work either.

And I'm unclear as to what's going on with the OLEDs - are they supposed to work, or do I need to change things? They don't turn on using the default keymap.

[yyc]

@MysticMixles

I still can't call rgblight_sethsv - it is undefined, even after adding RGB_SPLIT.

Can you upload a copy of your repo?

Also, using the index-less encoder code, it doesn't work either.

I've tested the split encoder code individually and it seems to work but not when I plug everything in so I'm not sure whether there's something wrong with my testing setup or whether there's some low-level issue that I'm not thinking of :/

And I'm unclear as to what's going on with the OLEDs - are they supposed to work, or do I need to change things? They don't turn on using the default keymap.

OLEDs should work immediately when you flash with the default keymap.
You said your OLEDs work on the master branch default keymap without any changes?

[yanfali]

If I have some time later today, I'll give this a try, I have the lily58 pro with oleds and there's an rgb strip but it's not wired up at the moment.

[fauxpark]

https://travis-ci.org/github/qmk/qmk_firmware/builds/682472639#L633-L637
Seems like it.

[noroadsleft]

Have never known #pragma once to be used in a .c file 😕

[yyc]

¯\(ツ)/¯
was requested by drashna in #6260 (comment)

[rhruiz]

well this is used in an #include by the OLED driver, should not be a problem to rename it to .h

[noroadsleft]

@drashna 😕

[noroadsleft]

Flashed the default keymap from this branch to some Pro Micros. Don't have the actual PCB or the hardware to emulate this board, so __attribute__ ((weak)) ✔️ because the only thing I can test is that it works as a keyboard.

[noroadsleft]

Looks good to me.

@yyc Fantastic work on the changelog entry. ❤️

[noroadsleft]

Thanks!