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

WIP:Lily58 glow edition firmware #8890

Closed
wants to merge 13 commits into from
Closed

WIP:Lily58 glow edition firmware #8890

wants to merge 13 commits into from

Conversation

luckenbach
Copy link

There was no firmware that 'properly' supported the Lily58+glow

Description

Created a 'glow' board inside of the Lily58 keyboard and defined sane default values that would ensure an end-user could have a desirable experience. I also included my keymap with this as I used it to author the initial changes and verify the properness of the 'glow' board.

Types of Changes

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

Issues Fixed or Closed by This PR

  • N/A

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).

luckenbach and others added 8 commits March 30, 2020 01:42
* Moved code over to quantum instead of promicro
* corrected LED number
* renamed rev2 to glow to make room for author to make a rev2
* updated lily58.h to support this

* added mayo keymap
* added luckenbach keymap
@zvecr
Copy link
Member

zvecr commented Apr 24, 2020

Duplicate of #7495, while this one is named nicer, there are a lot of unnecessary extra files in this PR.

@zvecr zvecr mentioned this pull request Apr 24, 2020
13 tasks
@luckenbach
Copy link
Author

I can clean up the files to some degree but I am not 100% what is required and what is now as far as a 'board' is concerned. Furthermore the PR you referenced is incorrect in the number of LEDs this board has and while yes it will work it is not proper by any stretch and is NOT the Rev2 of the board at all.

@ShadowProgr
Copy link
Contributor

ShadowProgr commented Apr 24, 2020

I'm using a lily58-glow board so I thought I'd try out your PR. But I can't seem to compile your keymaps (both luckenbach and mayo) on your fork. It seems to still use lily58/rev1 instead of lily58/glow

Making lily58/rev1 with keymap luckenbach

make[1]: Entering directory '/home/shadowprogr/Repos/temp/qmk_firmware'
avr-gcc (GCC) 8.3.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Compiling: keyboards/lily58/i2c.c                                                                   [OK]
Compiling: keyboards/lily58/serial.c                                                                [OK]
Compiling: keyboards/lily58/ssd1306.c                                                               [OK]
Compiling: keyboards/lily58/rev1/matrix.c                                                          keyboards/lily58/rev1/matrix.c:33:10: fatal error: pro_micro.h: No such file or directory
 #include "pro_micro.h"
          ^~~~~~~~~~~~~
compilation terminated.
 [ERRORS]
 | 
 | 
 | 
make[1]: *** [tmk_core/rules.mk:386: .build/obj_lily58_rev1_luckenbach/rev1/matrix.o] Error 1
make[1]: Leaving directory '/home/shadowprogr/Repos/temp/qmk_firmware'
Make finished with errors
make: *** [Makefile:579: lily58:luckenbach] Error 1

@luckenbach
Copy link
Author

you would need to do make lily58/glow:luckenbach

@luckenbach
Copy link
Author

you also showed me that my older branch (which my bad and i didn't rebase) had pro_micro as rev1 and was swapped out in #8374 for quantum.h

@luckenbach
Copy link
Author

oh my... rev1 is very broke by me now...

@luckenbach luckenbach changed the title Lily58 glow edition firmware WIP:Lily58 glow edition firmware Apr 24, 2020
@ShadowProgr
Copy link
Contributor

I've been able to build your keymap now, but yeah rev1 is broken right now.

Another thing I noticed with this board is that these numbers don't actually work as intended

#define RGBLED_NUM 70
#define RGBLED_SPLIT {35, 35} // lily58 doesn't seem to make use of this line

currently the RGBLED_NUM should be set to 35 for animations to work flawlessly. You can compare the snake animation yourself with both values 70 and 35 to see what I mean.

if the RGBLED_SPLIT option was working correctly the snake animation would have started on one half and went over to end on the other half, which is currently not the case. I strongly suspect this will be fixed by #6260, but I'm not really sure, I hope someone more knowledgeable can confirm

@luckenbach
Copy link
Author

luckenbach commented Apr 25, 2020

rev1 and glow now both pass the build!

I made the glow just ref the rev1's matrix.c and split_util/scomm, I am not sure what to do about the .h files as removing them from the glow dir breaks the build.

@ShadowProgr I built the firmware setting RGBLED_NUM to 35 and flashed both controllers and did the same for 70 and I don't see a difference ¯\_(ツ)_/¯
video of each

@ShadowProgr
Copy link
Contributor

@luckenbach check my video out. It's more clear when compared side by side. (sorry for potato quality)

Notice how the right half restarts the animation as soon as it ends. While the left side have idle time before restarting, it's almost as if it has to traverse 35 more "invisible" LEDs before restarting the animation, taking twice as long to finish one loop compared to the right side.

@luckenbach
Copy link
Author

Ahh I see what you mean! I have (in a different branch) added your matrix code, but unsure how to properly use it. Ill incorporate the adjustment till the split stuff is fixed in #6260

@luckenbach
Copy link
Author

Closing in favor of #8900

@luckenbach luckenbach closed this Apr 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants