[Bug] Getting multiple key presses from second half.

superami-code commented 3 years ago

Describe the Bug

I have as split keyboard with I²C communications. Both halves work great when connected via USB directly. However, when one half is connected via interlink cable to the other, then that half produces double key presses, not always but frequently. It doesn't matter which half either. Like I said its double odd since they both work on their own just fine.

I've tried playing with the de-bouncing, but it doesn't seem to resolve things, and again they both work fine when connected directly via USB.

System Information

Keyboard:
Custom ErgoDactyl using AmoebaKing PCB (1 per key, an AmoebaRoyale variant)
- Elite-C v3.1
- PIN method for recognizing half
- I²C interconnect with 10k pull-ups on each side.
- RGB with individually addressable SK6812 MINI E
- TRRS cable interconnect, 1.2m (shortest I could find on short notice.)
Operating system:
- Manjaro for build and development,
- Windows for Gaming ( Behavior is the same in both OSes )
AVR GCC version: 11.1.0
QMK Firmware version: 0.13.5

Rules.mk

MCU = atmega32u4

BOOTLOADER = caterina

BOOTMAGIC_ENABLE = no      # Virtual DIP switch configuration(+1000)
MOUSEKEY_ENABLE = yes       # Mouse keys(+4700)
EXTRAKEY_ENABLE = yes       # Audio control and System control(+450)
CONSOLE_ENABLE = no        # Console for debug(+400)
COMMAND_ENABLE = no        # Commands for debug and configuration

SLEEP_LED_ENABLE = no       # Breathing sleep LED during USB suspend

NKRO_ENABLE = yes            # USB Nkey Rollover
BACKLIGHT_ENABLE = no       # Enable keyboard backlight functionality on B7 by default
RGBLIGHT_ENABLE = yes       # Enable keyboard backlight functionality on B7 by default
UNICODE_ENABLE = no         # Unicode
MIDI_ENABLE = no            # MIDI controls
BLUETOOTH_ENABLE = no       # Enable Bluetooth with the Adafruit EZ-Key HID
AUDIO_ENABLE = no           # Audio output on port C6

SPLIT_KEYBOARD = yes

LTO_ENABLE = yes

LAYOUTS = ergodox

Config.h

#define VENDOR_ID       0xBB80
#define PRODUCT_ID      0x0504
#define DEVICE_VER      0x0001
#define MANUFACTURER    King
#define PRODUCT         ErgoDox
#define DESCRIPTION     Dactyl

#define MATRIX_ROWS 10
#define MATRIX_COLS 9

#define MATRIX_COL_PINS_RIGHT   { B6, B2, B3, B1, F7, F6, F5, F4, B0 }
#define MATRIX_COL_PINS         { B0, F4, F5, F6, F7, B1, B3, B2, B6 }
#define MATRIX_ROW_PINS_RIGHT   { C6, D7, E6, B4, B5 }
#define MATRIX_ROW_PINS         { C6, D7, E6, B4, B5 }
#define SPLIT_HAND_PIN D2
#define UNUSED_PINS
#define USE_I2C

/* COL2ROW or ROW2COL */
#define DIODE_DIRECTION ROW2COL
#define MATRIX_EXPANDER_COL_PINS { 5, 4, 3, 2, 1, 0 }
#define MATRIX_EXPANDER_ROW_PINS { 0, 1, 2, 3, 4, 5, 6 }

#define MOUSEKEY_INTERVAL       20
#define MOUSEKEY_DELAY          0
#define MOUSEKEY_TIME_TO_MAX    60
#define MOUSEKEY_MAX_SPEED      7
#define MOUSEKEY_WHEEL_DELAY 0

/* Debounce reduces chatter (unintended double-presses) - set 0 if debouncing is not needed */
#define DEBOUNCE 5

#define RGBLIGHT_LAYERS
#define RGBLIGHT_ANIMATIONS
#define RGBLEG_SPLIT { 38, 38 }
#define RGB_DI_PIN D4
#define RGBLED_NUM 76
#define RGBLIGHT_HUE_STEP 8
#define RGBLIGHT_SAT_STEP 8
//#define RGBLIGHT_LED_MAP { 1, 2, 3, 12, 13, 14, 15, 0, 7, 6, 5, 4, 11, 10, 9, 8 }

#define BACKLIGHT_LEVELS 3

#ifndef LED_BRIGHTNESS_LO
#define LED_BRIGHTNESS_LO       15
#endif
#ifndef LED_BRIGHTNESS_HI
#define LED_BRIGHTNESS_HI       255
#endif
#define LED_BRIGHTNESS_DEFAULT (128)

#define FORCE_NKRO

daskygit commented 3 years ago

The 10K pull ups might be a little high but you can try increasing this value and see if it improves. I'd probably at least double it. On develop branch you could just define it in your config.h.

https://github.com/qmk/qmk_firmware/blob/c232882fda0669e28312446feac3cadb254962e8/quantum/split_common/matrix.c#L27

superami-code commented 3 years ago

Thank you for the quick response.

Increasing ERROR_DISCONNECT_COUNT to 20, seems to have helped , but not solved the issue. I suspect that its also my interconnect cable. I didn't pay enough attention when I made the purchase and its 1.2m (coiled). I've ordered a 0.3m cable which should arrive next week and hopefully also help with the I2C communications.

On the I2C I wanted to keep the build mirrored as much as possible so I used a 10k pull-up on each side, which I figured would make the resistance to VCC about 5k. What would be better values for these?

daskygit commented 3 years ago

Ahh that's a pretty long cable, it may not even be a problem with the shorter one.

superami-code commented 3 years ago

So the shorter error did reduce errors, but there are still issue with double key presses and RGB matrix synchronization. Is it possible to slow down the I²C or would a lower pull-up on each end be better? Is there a guide to how the pick the optimum pull-up?

tzarc commented 3 years ago

I'd aim for something like a 2.2k resistance overall on both lines. A 4.7k on both sides (i.e. a parallel resistor network) should be fine.

tzarc commented 3 years ago

Part of the problem I'd imagine is that I2C is non-responsive while sending out the RGB data to the SK6812's -- 38 LEDs per side would mean about 1.4ms of complete unresponsiveness from the slave due to the RGB requiring the disabling of interrupts.

You've also typo'd RGBLEG_SPLIT -- should be RGBLED_SPLIT.

superami-code commented 3 years ago

Part of the problem I'd imagine is that I2C is non-responsive while sending out the RGB data to the SK6812's -- 38 LEDs per side would mean about 1.4ms of complete unresponsiveness from the slave due to the RGB requiring the disabling of interrupts.

Disabling the interrupts for the inter RGB time is painful. I will need to look into a better solution here. As the whole train of data needs to go out, I doubt it will be a trivial fix. Maybe I can split the RGB's onto multiple pins. Where is the code for the RGB's located? Specifically the code that would disable the RGB's

You've also typo'd RGBLEG_SPLIT -- should be RGBLED_SPLIT.

Thanks for the tip, I'd already found that one.

sigprof commented 3 years ago

You could just try compiling with RGBLIGHT_ENABLE = no (maybe you would need to fix some custom code that calls RGB functions), and check whether that affects the communication errors. Alternatively, you could just set RGB to a static mode without any animations (and disable any code which uses RGB LEDs as indicators, if you have it).

Splitting RGB LEDs onto multiple pins is not supported (the problem is that on AVR the addresses of the corresponding port registers need to be compiled into the assembly code, therefore RGB_DI_PIN must be a compile time constant).

However, disabling interrupts should not be a major problem for I2C — the slave controller should just use clock stretching to pause the transfer until the interrupt is serviced, and the timeout used by the master side seems to be 100 ms, so there should not be any communication errors caused just by this. But 1.4ms with interrupts disabled could still cause some timer ticks to be lost (this may cause some RGB sync issues, although there is the “sync timer” code which should fix them).

superami-code commented 3 years ago

You could just try compiling with RGBLIGHT_ENABLE = no (maybe you would need to fix some custom code that calls RGB functions), and check whether that affects the communication errors. Alternatively, you could just set RGB to a static mode without any animations (and disable any code which uses RGB LEDs as indicators, if you have it).

Splitting RGB LEDs onto multiple pins is not supported (the problem is that on AVR the addresses of the corresponding port registers need to be compiled into the assembly code, therefore RGB_DI_PIN must be a compile time constant).

However, disabling interrupts should not be a major problem for I2C — the slave controller should just use clock stretching to pause the transfer until the interrupt is serviced, and the timeout used by the master side seems to be 100 ms, so there should not be any communication errors caused just by this. But 1.4ms with interrupts disabled could still cause some timer ticks to be lost (this may cause some RGB sync issues, although there is the “sync timer” code which should fix them).

Turning off the RGB animation with the RGB_TOG button seems to fix key stroke issues. I will look into other solutions deeper in the code when I have more time. It would be great to have both split and full RGB capabilities. Another oddity I noticed, was that when the halves were both connected via USB, Linux often had real lag between fast typing and the letters actually appearing in an editor or browser.

superami-code commented 3 years ago

I couldn't figure out how to disable the I2C interrupt and tried switching to serial communications, but the issues are also present there. For now, I have no issues when using slow or static animations. This seems to confirm that the issue is not a hardware, but rather a software issue. I still need to figure out if the issue is with the time it takes to run the animation or the time it takes to transfer the animations between halves.

qmk / qmk_firmware