Lotus58 glow #20386

TweetyDaBird · 2023-04-09T14:02:16Z

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

My code follows the code style of this project: C, Python
I have read the PR Checklist document and have made the appropriate changes.
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).

atmel-dfu to atmeldfu

Cleaned up the default keymap Made versions for bootloaders

keyboards/tweetydabird/lotus58/atmeldfu/keymaps/default/info.json

keyboards/tweetydabird/lotus58/config.h

keyboards/tweetydabird/lotus58/info.json

keyboards/tweetydabird/lotus58/lotus58.c

keyboards/tweetydabird/lotus58/nanoboot/keymaps/default/info.json

Co-authored-by: jack <[email protected]>

zvecr

Keymap folder per revision seems a bit unnecessary given its the same PCB in all cases.

keyboards/tweetydabird/lotus58/keymaps/default/keymap.c
keyboards/tweetydabird/lotus58/atmeldfu/rules.mk
keyboards/tweetydabird/lotus58/caterina/rules.mk
keyboards/tweetydabird/lotus58/nanoboot/rules.mk

Using some existing terminology could also help with consistency;

caterina -> promicro
atmeldfu -> elite_c

As a decent proportion of user will have no idea when to use caterina vs atmeldfu.

keyboards/tweetydabird/lotus58/nanoboot/keymaps/default/info.json

keyboards/tweetydabird/lotus58/info.json

TweetyDaBird · 2023-04-11T06:13:50Z

Keymap folder per revision seems a bit unnecessary given its the same PCB in all cases.
keyboards/tweetydabird/lotus58/keymaps/default/keymap.c
keyboards/tweetydabird/lotus58/atmeldfu/rules.mk
keyboards/tweetydabird/lotus58/caterina/rules.mk
keyboards/tweetydabird/lotus58/nanoboot/rules.mk
Using some existing terminology could also help with consistency;

caterina -> promicro

atmeldfu -> elite_c

As a decent proportion of user will have no idea when to use caterina vs atmeldfu.

Well, since it is in fact categorized by boot loader, and the list of controllers it applies to was included in the readme. But sure I can rename them.

Edit: to clarify, since Atmega32u4 are routinely running out of memory for advanced features on split keyboards especially, I try to steer builders to use nanoBoot as the bootloader, and for whatever reason, making commented out lines in a single firmware ended up causing confusion. Separate folders/versions seems to solve/reduce that.

Co-authored-by: jack <[email protected]>

Co-authored-by: Joel Challis <[email protected]>

TweetyDaBird · 2023-04-13T05:23:25Z

I believe all requested changes (manual edits) should be fixed and tested now. Let me know if there is more that needs fixing.

Again, in all respect, no rush, just confirming that I have tested things.

zvecr · 2023-04-13T10:10:42Z

Keymap folder per revision seems a bit unnecessary given its the same PCB in all cases.

keyboards/tweetydabird/lotus58/keymaps/default/keymap.c
keyboards/tweetydabird/lotus58/atmeldfu/rules.mk
keyboards/tweetydabird/lotus58/caterina/rules.mk
keyboards/tweetydabird/lotus58/nanoboot/rules.mk

This suggestion has not been completed.

TweetyDaBird · 2023-04-13T11:28:33Z

Ah, right. Then I misunderstood and/or didn't read carefully enough. Now, I'll have to think on that. This sort of creates an issue going to Vial.

Again trying to have a discussion, not arguing/being a PITA more than necessary. I agree that this isn't your issue to solve, I'm just trying to find the middle ground where it's acceptable here, and doesn't totally shoot me in the foot there, creating more work as a whole. Feel free to just ignore me if you don't want to have the discussion.

Moving the key-maps to a central folder doesn't work in Vial as the key-maps there have to be dependent on the boot-loader (lack of space) and the standard for the Vial key-map is a single folder with the name Vial.

Do you have any good suggestions for a workaround?

Can the boot-loader assignment move to key-map level? It actually can can't it? (Although I'm not sure if that the correct thing to do).

zvecr · 2023-04-13T11:54:48Z

where it's acceptable here, and doesn't totally shoot me in the foot there

The only thing that is of concern here if its acceptable to QMK. Optimising, even if its only in part, for Vial is zero on the scale.

However it should be noted that having a common folder for keymaps does not stop at a later date the introduction of

keyboards/tweetydabird/lotus58/atmeldfu/keymaps/something/keymap.c
keyboards/tweetydabird/lotus58/caterina/keymaps/something/keymap.c

As long as "something" doesnt also exist within keyboards/tweetydabird/lotus58/keymaps.

You can also use have conditional blocks within the compilation units, with #ifdef BOOTLOADER_CATERINA and things like post_rules.mk to disable features completely.

TweetyDaBird · 2023-04-13T13:07:01Z

The only thing that is of concern here if its acceptable to QMK. Optimising, even if its only in part, for Vial is zero on the scale.

I fully understand the position. However, had I just swallowed the first suggestion and not questioned, it would probably resulted in another PR with an update fairly soon, and then another, and another... Making us both batty...

The reason I asked however is this:

You can also use have conditional blocks within the compilation units, with #ifdef BOOTLOADER_CATERINA and things like post_rules.mk to disable features completely.

That should have been obvious to me, having used the '#ifdef OLED' etc... It wasn't for some reason. So, from the bottom of my heart, thank you for making me feel damned stupid... That was going beyond...

It might perhaps take me a while to iron it out to my satisfaction (and yours), but I think you just gave me the key...

…mware into Lotus58Glow

drashna · 2023-04-14T00:31:17Z

This is the fifth PR for this board: #20281, #18554, #15867, #12594.

TweetyDaBird · 2023-04-14T08:01:48Z

This is the fifth PR for this board: #20281, #18554, #15867, #12594.

Yeah. I don't do good with GitHub time limits and such. ADHD. Either things happen fast, and I say focused, or I loose focus and hyper focus on something else, and end up procrastinating. Was sort of why I managed to annoy Joel before(justified from his side Btw). I was more trying to keep myself on task than anything else.

And git isn't my strong suit to start with. Is there a way to re-open a PR from my side? I would have preferred that.

TweetyDaBird · 2023-04-14T14:04:06Z

Well, a work days worth of testing. It work as is, and armed with the above tip, I feel I can take this and make Vial work without shooting myself in the foot once it's in the main repo. I'm happy with it. Let me know if there is anything else that needs fixed.

keyboards/tweetydabird/lotus58/elite_c/rules.mk

keyboards/tweetydabird/lotus58/info.json

keyboards/tweetydabird/lotus58/keymaps/default/keymap.c

keyboards/tweetydabird/lotus58/nanoboot/rules.mk

keyboards/tweetydabird/lotus58/promicro/rules.mk

Co-authored-by: Pablo Martínez <[email protected]>

…mware into Lotus58Glow

zvecr

Ive fixed up things after the bad info you were given, as well as a few minor tidy ups. All good to go from my front now.

* upstream/master: (73 commits) [Keyboard] Add Kalakos Bahrnob65 (qmk#20424) Tidy up stray RGB_DISABLE_TIMEOUT references (qmk#20460) [Keyboard] Add zoom75 wired (qmk#20396) [Keyboard] Add dymium65 (qmk#20257) Lotus58 glow (qmk#20386) ADPenrose Obi Layout Macro Conversion and Addition (qmk#20445) Add hardware information momokai keyboards (qmk#20434) 4pplet/eagle_viper_rep/rev_a Layout Macro Conversion and Additions (qmk#20414) [Keymap] Add paulomp90 lily58 keymap (qmk#20327) [Keymap] Add personal keymap for Lily58 (qmk#18735) [Keyboard] Fix h87 g2 VID conflict (qmk#20388) [Keymap] PHSC138 Keymap for Atom47 (qmk#18768) [Keyboard] add kb2040 flavor of gherkin (qmk#18360) [Keyboard] ymdk/id75 (qmk#19967) fixing bug that caused KC_DEL and KC_MUTE (encoder press) to be swapped (qmk#20420) 4pplet/bootleg/rev_a Layout Macro Conversion and Addition (qmk#20400) 4pplet/aekiso60 Layout Macro Conversion and Additions (qmk#20399) Reject info.json at keymap level (qmk#20408) Bump anothrNick/github-tag-action from 1.61.0 to 1.62.0 (qmk#20407) [Keyboard] Update angle65 VID/PID (qmk#20401) ...

TweetyDaBird · 2023-04-17T12:21:44Z

Ive fixed up things after the bad info you were given, as well as a few minor tidy ups. All good to go from my front now.

Thanks! And a lot of thanks for the pointers!

Co-authored-by: jack <[email protected]> Co-authored-by: Joel Challis <[email protected]> Co-authored-by: Pablo Martínez <[email protected]>

* master: NK Plus (qmk#20392) [Docs] Fix suggested code pattern when a specific mod-mask is required. (qmk#20512) [Docs] Remove combo count from array (qmk#20511) Add QuadrumLabs Delta (qmk#20409) Adds Docs option for ArduinoIDE's example `ArduinoISP` (qmk#20486) GMMK 2 volume up/down Fn keys are backwards in default mapping (qmk#20476) Fix typo in `feature_wpm.md` title (qmk#20464) [Keyboard] Add Kalakos Bahrnob65 (qmk#20424) Tidy up stray RGB_DISABLE_TIMEOUT references (qmk#20460) [Keyboard] Add zoom75 wired (qmk#20396) [Keyboard] Add dymium65 (qmk#20257) Lotus58 glow (qmk#20386) ADPenrose Obi Layout Macro Conversion and Addition (qmk#20445)

TweetyDaBird added 5 commits March 28, 2023 20:06

Added Keyboard Lotus58Glow

7276bb7

renamed folder

ed31c5c

atmel-dfu to atmeldfu

Merge branch 'qmk:master' into Lotus58Glow

26148a7

Cleanup

3207e95

Cleaned up the default keymap Made versions for bootloaders

Merge branch 'qmk:master' into Lotus58Glow

b82a25d

github-actions bot added keyboard keymap labels Apr 9, 2023

waffle87 requested changes Apr 9, 2023

View reviewed changes

TweetyDaBird and others added 4 commits April 10, 2023 20:38

Update keyboards/tweetydabird/lotus58/info.json

a902172

Co-authored-by: jack <[email protected]>

Update keyboards/tweetydabird/lotus58/lotus58.c

2d21362

Co-authored-by: jack <[email protected]>

Update keyboards/tweetydabird/lotus58/lotus58.c

3483506

Co-authored-by: jack <[email protected]>

removed whitespace

d1fe51a

zvecr reviewed Apr 11, 2023

View reviewed changes

zvecr mentioned this pull request Apr 11, 2023

Reject info.json at keymap level #20408

Merged

14 tasks

TweetyDaBird and others added 5 commits April 11, 2023 08:16

Update keyboards/tweetydabird/lotus58/config.h

cb8455f

Co-authored-by: jack <[email protected]>

Update keyboards/tweetydabird/lotus58/info.json

6fdb9ad

Co-authored-by: Joel Challis <[email protected]>

Renamed folders, removed keymap info.json, bootmagic

73c0b2b

Update keyboards/tweetydabird/lotus58/info.json

0a90ac0

Co-authored-by: Joel Challis <[email protected]>

Update keyboards/tweetydabird/lotus58/info.json

a2a4544

Co-authored-by: Joel Challis <[email protected]>

TweetyDaBird requested a review from zvecr April 13, 2023 05:23

TweetyDaBird added 2 commits April 13, 2023 15:17

Single keymap folder

a00f3a0

Merge branch 'Lotus58Glow' of https://github.com/TweetyDaBird/qmk_fir…

48146c6

…mware into Lotus58Glow

elpekenin suggested changes Apr 14, 2023

View reviewed changes

TweetyDaBird and others added 4 commits April 15, 2023 09:54

Update keyboards/tweetydabird/lotus58/info.json

06d5ed1

Co-authored-by: Pablo Martínez <[email protected]>

Update keyboards/tweetydabird/lotus58/keymaps/default/keymap.c

13eb733

Co-authored-by: Pablo Martínez <[email protected]>

rules.mk -> info.json

1085faa

Merge branch 'Lotus58Glow' of https://github.com/TweetyDaBird/qmk_fir…

b76c808

…mware into Lotus58Glow

drashna approved these changes Apr 15, 2023

View reviewed changes

Review comments

eea7770

zvecr approved these changes Apr 15, 2023

View reviewed changes

keyboard-magpie approved these changes Apr 15, 2023

View reviewed changes

keyboard-magpie merged commit 28c11ed into qmk:master Apr 15, 2023

rodrigob pushed a commit to rodrigob/qmk_firmware that referenced this pull request Apr 18, 2023

Lotus58 glow (qmk#20386)

59f79c7

Co-authored-by: jack <[email protected]> Co-authored-by: Joel Challis <[email protected]> Co-authored-by: Pablo Martínez <[email protected]>

quangd42 pushed a commit to quangd42/qmk_firmware that referenced this pull request Apr 20, 2023

Lotus58 glow (qmk#20386)

efcde6c

Co-authored-by: jack <[email protected]> Co-authored-by: Joel Challis <[email protected]> Co-authored-by: Pablo Martínez <[email protected]>

queyenth pushed a commit to queyenth/qmk_firmware that referenced this pull request Apr 21, 2023

Lotus58 glow (qmk#20386)

ea3e22a

Co-authored-by: jack <[email protected]> Co-authored-by: Joel Challis <[email protected]> Co-authored-by: Pablo Martínez <[email protected]>

stratosgear pushed a commit to stratosgear/qmk_firmware that referenced this pull request Apr 26, 2023

Lotus58 glow (qmk#20386)

a4389af

Co-authored-by: jack <[email protected]> Co-authored-by: Joel Challis <[email protected]> Co-authored-by: Pablo Martínez <[email protected]>

TweetyDaBird deleted the Lotus58Glow branch April 29, 2023 16:21

struckmb pushed a commit to struckmb/qmk_firmware that referenced this pull request May 1, 2023

Lotus58 glow (qmk#20386)

fa7fab9

Co-authored-by: jack <[email protected]> Co-authored-by: Joel Challis <[email protected]> Co-authored-by: Pablo Martínez <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lotus58 glow #20386

Lotus58 glow #20386

TweetyDaBird commented Apr 9, 2023

zvecr left a comment •

edited

Loading

TweetyDaBird commented Apr 11, 2023 •

edited

Loading

TweetyDaBird commented Apr 13, 2023 •

edited

Loading

zvecr commented Apr 13, 2023

TweetyDaBird commented Apr 13, 2023 •

edited

Loading

zvecr commented Apr 13, 2023

TweetyDaBird commented Apr 13, 2023

drashna commented Apr 14, 2023

TweetyDaBird commented Apr 14, 2023

TweetyDaBird commented Apr 14, 2023 •

edited

Loading

zvecr left a comment •

edited

Loading

TweetyDaBird commented Apr 17, 2023

Lotus58 glow #20386

Lotus58 glow #20386

Conversation

TweetyDaBird commented Apr 9, 2023

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

zvecr left a comment • edited Loading

Choose a reason for hiding this comment

TweetyDaBird commented Apr 11, 2023 • edited Loading

TweetyDaBird commented Apr 13, 2023 • edited Loading

zvecr commented Apr 13, 2023

TweetyDaBird commented Apr 13, 2023 • edited Loading

zvecr commented Apr 13, 2023

TweetyDaBird commented Apr 13, 2023

drashna commented Apr 14, 2023

TweetyDaBird commented Apr 14, 2023

TweetyDaBird commented Apr 14, 2023 • edited Loading

zvecr left a comment • edited Loading

Choose a reason for hiding this comment

TweetyDaBird commented Apr 17, 2023

zvecr left a comment •

edited

Loading

TweetyDaBird commented Apr 11, 2023 •

edited

Loading

TweetyDaBird commented Apr 13, 2023 •

edited

Loading

TweetyDaBird commented Apr 13, 2023 •

edited

Loading

TweetyDaBird commented Apr 14, 2023 •

edited

Loading

zvecr left a comment •

edited

Loading