Switch Ergodox Infinity over to split_common #13481

firetech · 2021-07-07T08:53:42Z

Description

Serial Link is cool and all, but since the Infinity is the only keyboard using it, it gets easily forgotten when new split sync features are added.

I had some weird behaviour in that calling the suspend handler for LED Matrix on the slave killed the serial communication, so I added is_keyboard_master() to its if statement (which fixed that issue).

Types of Changes

Issues Fixed or Closed by This PR

Somewhat related to Fancy lightning on Infinity Ergodox? #12450

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

Serial Link is cool and all, but since the Infinity is the only keyboard using it, it gets easily forgotten when new split sync features are added.

firetech · 2021-07-07T08:54:15Z

Note that the visualizer (not used by default since #13165 was just merged) isn't synced between halves after this. I have some code to fix this, that could be added to this PR if desired. Since the visualizer isn't enabled by default, I left it out for now.

keyboards/ergodox_infinity/rules.mk

drashna · 2021-07-07T17:03:11Z

keyboards/ergodox_infinity/config.h

@@ -54,7 +54,20 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 /* key matrix size */
 #define MATRIX_ROWS 18
 #define MATRIX_COLS 5
-#define LOCAL_MATRIX_ROWS 9
+
+// For some reason, the rows are colums in the schematic, and vice versa


This is probably because it's based on the ergodox (OG), which has it's matrix rotated 90 degrees. So it's probably the same here.

That said... if that's the case, it may be worth using the eager per row debounce algo for the board, if each row can only be used by one finger at a time.

What I referred to here was the fact that the rows (as taken from the old custom matrix.c) are named columns in the schematic, and vice versa. I left it in this state to avoid having to rewrite all the LAYOUT defines.

I've actually tried using the eager_pr debounce algo in the past (I did some work last year to move over the Infinity to the common debounce handling, but ended up not submitting it), mostly for parity with the EZ I use at work, and actually got quite a lot of bouncing keys with that, so I went back to defer_g (which is the main reason I never submitted a PR, it didn't really add any value).

I'm currently using the defer_pk algo on my Infinity, for no particular reason other than having lots of RAM to spare. 😎

fauxpark · 2021-07-08T03:03:21Z

quantum/led_matrix/led_matrix.c

@@ -459,7 +459,7 @@ void led_matrix_init(void) {

 void led_matrix_set_suspend_state(bool state) {
 #ifdef LED_DISABLE_WHEN_USB_SUSPENDED
-    if (state) {
+    if (state && is_keyboard_master()) {


Hm, why does LED Matrix need this but RGB Matrix not? Could it be something in the ISSI driver itself?

Not sure. Looking at the code of led_matrix_set_value_all, it seems it shouldn't make any difference, but it does. 😕

I've done some more testing, and this is really weird.

If I skip the special handling of LED_MATRIX_SPLIT in either led_matrix_set_value_all or led_matrix_set_value, everything works fine, for Ergodox Infinity at least (since its LED pinouts are the same on each half, just mirrored positions). The weird part of this is that the first and second halves of g_is31_leds are identical, so the only real difference should be that it's writing the led values twice. 😵

Note that the serial comms die after suspend regardless of which side is the master (without any change).

firetech · 2021-07-08T12:08:38Z

Forgot to mention that this change makes it "impossible" to use the keyboard halves separately (something that works fine with Serial Link), due to several serial read timeouts each scan cycle eating up the matrix scan rate.

It's possible to solve this by having the split_common system detect that the slave half is missing and holding off on connection attempts for a while (I'm running a hacky version of this on my keyboard without issues), but I feel that is a bit out of scope for this PR.

Edit: Made a PR for this suggested change: #13523

firetech · 2021-07-12T13:29:10Z

@drashna This PR on top of develop doesn't really boot properly on my Ergodox Infinity anymore, since #13330 was merged. It hangs during boot and Windows pops up a notification about not being able to identify the last connected USB device. Reverting the #13330 commit makes it work. Any idea on what could be wrong/missing?

Edit: Figured it out myself, see #13525

Two occurrences of `MATRIX_ROWS` weren't properly changed to `ROWS_PER_HAND` in qmk#13330, causing a crash during boot on at least my Ergodox Infinity (including qmk#13481).

Two occurrences of `MATRIX_ROWS` weren't properly changed to `ROWS_PER_HAND` in #13330, causing a crash during boot on at least my Ergodox Infinity (including #13481).

drashna · 2021-07-20T01:54:24Z

Thanks!

Two occurrences of `MATRIX_ROWS` weren't properly changed to `ROWS_PER_HAND` in qmk#13330, causing a crash during boot on at least my Ergodox Infinity (including qmk#13481).

roygbyte · 2022-02-16T20:16:51Z

Hi. Relative newbie here. I use an Ergodox Infinity and switched from kiibohd to QMK in May 2021. At the time, I used QMK firmware 0.12.50 to build/flash my keyboard.

The other day I had the bright idea of making some updates/improvements to my configuration and decided to use the new 0.15 firmware to flash. As the story goes, I have noticed a significant reduction in performance and reliability, including:

Slave half stops receiving input after an unpredictable amount of time
Random keystrokes seem to be skipped/ignored/etc.

I would like to debug this issue and have figured this PR might be the best place to drop a note, since the switch to split_common occurred soon after (i.e.: July 2021) my initial flash in May 2021. I wonder if this is the cause of the issue I am facing. I also wonder if other folks have noticed any performance differences since the switch.

From what I can tell so far, the issue is not with the keyboard configuration itself. I have tested @firetech's lovely configuration using QMK firmware 0.12.50 and 0.15. It works fine with 0.12.50, but experiences the aforementioned issues on 0.15.

I have not dug too deeply into the code changes brought about since 0.12.50, but wouldn't be terrified to do so. I just want to get a sense of what contributors/maintainers think about this issue before investing that time.

(Also: I just noticed that the little eared cat [?] creature is animated. So cute! I didn't see this on firmware 0.15, but now that I'm back on 0.12.50 the creature is hamming it up. Love that.)

firetech · 2022-02-16T21:16:37Z

@roygbyte As you may have noticed in my repository, I haven't updated my build in quite a long while, and my keyboard still runs the exact code in my repository (based on the develop branch back in August)... Life has simply gotten in the way.

I have no idea what has been changed in the recent months, and I'm not sure if the maintainers will see your comment here since this PR has been merged and closed. I recommend checking out the (official) QMK Discord, the people there are usually very helpful. :)

roygbyte · 2022-02-16T21:26:35Z

Thanks for the speedy reply! Yes, I did notice your repo was a bit behind... that could be a good thing, though, if it works as you seem to imply. Perhaps I'll start there and see how that version of QMK performs on my keyboard. (Funny enough, I had been saying to myself the other day that I wanted to work with C code more... low-and-behold, it seems like my wish has been granted xD ) ... or I'll just settle for what works (which seems to be my own out-dated repo...).

firetech · 2022-02-16T23:12:37Z

@roygbyte I just happened to be browsing GitHub for entirely unrelated reasons and stumbled upon the notification about your post before I noticed the email about it. xD

Actually, since I had some time over just now, I tried rebasing my branch onto current master, and I don't have any issues (except for two core bugfix PRs that I just posted). Everything seems to work fine and dandy here*. My main branch has been updated accordingly (including said PRs).

*) I've been using a build based on master since shortly after my last post. In other words, for about two hours, including hunting bugs and fixing them. In that time, I haven't noticed any of the problems you described.

(Edit: if you want my old version for reference I pushed it here)

roygbyte · 2022-02-16T23:44:34Z

Such happenstance to have you stumbled upon my error :)

After a bit of poking around with versions, it seems that between 0.13.38 and 0.14.0 is where my performance degrades. I tested the same layout on both, and while it runs really snappy on the former, it's quite terrible on the later. I should mention that I flashed both using the older instructions for flashing, where the master side is explicitly indicated. I.e.: make ergodox_infinity:layout MASTER=left.

Is it possible there's some other variable causing the issue? I am using USB 3.0 cables, best I could buy. I have the halves connected together... Not sure what else could be causing different results for me vs. you!

Anyways, I don't mean to take up too much of your time. I have made a bit of discovery, which is good. Thanks for pushing your old version. I'll look around... perhaps there are more clues to reveal...

firetech · 2022-02-17T09:26:48Z

After a bit of poking around with versions, it seems that between 0.13.38 and 0.14.0 is where my performance degrades.

git bisect might help you narrow down the exact commit when performance degrades (it does a binary search between two given revisions). :)

roygbyte · 2022-02-17T16:05:54Z

Thanks for the suggestion. I would not have thought to use that command! If I find anything interesting you might hear from me again 😅

roygbyte · 2022-02-18T13:56:37Z

Which side do you used as the master keyboard? I don't know if/why it would matter, but I seem to get fewer issues when the right side is used as the master.

I flashed the keyboards with their respective dfu commands:
make ergodox_infinity:firetech:dfu-util-split-right for the right side, make ergodox_infinity:firetech:dfu-util-split-left for the left side. After this, with the left side as master, the right side will eventually (after a minute or so) stop receiving/transmitting keystrokes. If I switch to the right side as master, I don't get this problem. But: some keys will "lag" and not transmit a keystroke until pressed numerous times.

firetech · 2022-02-18T14:06:52Z

Which side do you used as the master keyboard? I don't know if/why it would matter, but I seem to get fewer issues when the right side is used as the master.

I mainly use the right side as the master. However, while implementing this PR (and some others), I was switching the master role back and forth, as it made the flashing process easier (flash the master, connect it to the slave, then connect the slave as new master and flash it), so I doubt the direction should matter. Have you tried switching your cables around to see if that makes any difference?

I flashed the keyboards with their respective dfu commands: make ergodox_infinity:firetech:dfu-util-split-right for the right side, make ergodox_infinity:firetech:dfu-util-split-left for the left side. After this, with the left side as master, the right side will eventually (after a minute or so) stop receiving/transmitting keystrokes. If I switch to the right side as master, I don't get this problem. But: some keys will "lag" and not transmit a keystroke until pressed numerous times.

I don't have any such issues, weird. Either the hardware, cables or compiler is to blame, I guess. 😛

roygbyte · 2022-02-18T17:39:29Z

The break point for me is here: 6fe3943. Why that is so I do not know. And, quite frankly, I'm beginning not to care xD

I don't have any such issues, weird. Either the hardware, cables or compiler is to blame, I guess.

I'm just waiting for something to reveal to me that I am to blame 😂.

For my own sanity... Could you confirm for me the revision number of your infinity ergodox? Mine is 1.2b, from 2015 I believe.

Two occurrences of `MATRIX_ROWS` weren't properly changed to `ROWS_PER_HAND` in qmk#13330, causing a crash during boot on at least my Ergodox Infinity (including qmk#13481).

firetech added 2 commits July 7, 2021 10:15

Switch Ergodox Infinity over to split_common.

b52e2a7

Serial Link is cool and all, but since the Infinity is the only keyboard using it, it gets easily forgotten when new split sync features are added.

Fix memory corruption(?) in LED Matrix when suspending slave half.

a0d8574

github-actions bot added core keyboard labels Jul 7, 2021

firetech mentioned this pull request Jul 7, 2021

Fancy lightning on Infinity Ergodox? #12450

Closed

fauxpark reviewed Jul 7, 2021

View reviewed changes

keyboards/ergodox_infinity/rules.mk Outdated Show resolved Hide resolved

fauxpark requested a review from a team July 7, 2021 09:03

Remove mostly blank led.c from Ergodox Infinity.

35061e9

drashna approved these changes Jul 7, 2021

View reviewed changes

drashna reviewed Jul 7, 2021

View reviewed changes

fauxpark reviewed Jul 8, 2021

View reviewed changes

fauxpark approved these changes Jul 8, 2021

View reviewed changes

firetech mentioned this pull request Jul 12, 2021

Make solo half of split keyboards (more) usable. #13523

Merged

14 tasks

firetech mentioned this pull request Jul 12, 2021

Fix two out of bounds accesses from #13330. #13525

Merged

14 tasks

drashna merged commit 1414e97 into qmk:develop Jul 20, 2021

nhongooi pushed a commit to nhongooi/qmk_firmware that referenced this pull request Dec 5, 2021

Switch Ergodox Infinity over to split_common (qmk#13481)

e06fde6

firetech deleted the ergodox_infinity_split_common branch May 8, 2022 13:08

BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024

Switch Ergodox Infinity over to split_common (qmk#13481)

6837930

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch Ergodox Infinity over to split_common #13481

Switch Ergodox Infinity over to split_common #13481

firetech commented Jul 7, 2021

firetech commented Jul 7, 2021

drashna Jul 7, 2021

firetech Jul 7, 2021

fauxpark Jul 8, 2021

firetech Jul 8, 2021

firetech Jul 8, 2021

firetech commented Jul 8, 2021 •

edited

Loading

firetech commented Jul 12, 2021 •

edited

Loading

drashna commented Jul 20, 2021

roygbyte commented Feb 16, 2022 •

edited

Loading

firetech commented Feb 16, 2022

roygbyte commented Feb 16, 2022 •

edited

Loading

firetech commented Feb 16, 2022 •

edited

Loading

roygbyte commented Feb 16, 2022

firetech commented Feb 17, 2022

roygbyte commented Feb 17, 2022

roygbyte commented Feb 18, 2022

firetech commented Feb 18, 2022

roygbyte commented Feb 18, 2022 •

edited

Loading

Switch Ergodox Infinity over to split_common #13481

Switch Ergodox Infinity over to split_common #13481

Conversation

firetech commented Jul 7, 2021

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

firetech commented Jul 7, 2021

drashna Jul 7, 2021

Choose a reason for hiding this comment

firetech Jul 7, 2021

Choose a reason for hiding this comment

fauxpark Jul 8, 2021

Choose a reason for hiding this comment

firetech Jul 8, 2021

Choose a reason for hiding this comment

firetech Jul 8, 2021

Choose a reason for hiding this comment

firetech commented Jul 8, 2021 • edited Loading

firetech commented Jul 12, 2021 • edited Loading

drashna commented Jul 20, 2021

roygbyte commented Feb 16, 2022 • edited Loading

firetech commented Feb 16, 2022

roygbyte commented Feb 16, 2022 • edited Loading

firetech commented Feb 16, 2022 • edited Loading

roygbyte commented Feb 16, 2022

firetech commented Feb 17, 2022

roygbyte commented Feb 17, 2022

roygbyte commented Feb 18, 2022

firetech commented Feb 18, 2022

roygbyte commented Feb 18, 2022 • edited Loading

firetech commented Jul 8, 2021 •

edited

Loading

firetech commented Jul 12, 2021 •

edited

Loading

roygbyte commented Feb 16, 2022 •

edited

Loading

roygbyte commented Feb 16, 2022 •

edited

Loading

firetech commented Feb 16, 2022 •

edited

Loading

roygbyte commented Feb 18, 2022 •

edited

Loading