KMKfw / kmk_firmware

Clackety Keyboards Powered by Python
https://kmkfw.zulipchat.com
Other
1.45k stars 485 forks source link

[BUG] RapidFire unexpected keyword argument 'on_press' #1025

Closed tinyboatproductions closed 2 months ago

tinyboatproductions commented 2 months ago

Describe the bug When trying to setup RapidFire on a new install any RapidFire key presents the "unexpected keyword argument 'on_press' " error.

To Reproduce Steps to reproduce the behavior:

Expected behavior Expect it to behave as RapidFire keys used to. Rapid key presses of the provided key code

Debug output

main.py output:
main.py | 9.1.3Starting
Traceback (most recent call last):
  File "main.py", line 19, in <module>
  File "kmk/keys.py", line 576, in argumented_key
TypeError: unexpected keyword argument 'on_press'
576@kmk/keys.py TypeError | 9.1.3
Code done running.

Additional context

main.py:

print("Starting")

import board

from kmk.kmk_keyboard import KMKKeyboard
from kmk.keys import KC
from kmk.scanners import DiodeOrientation
from kmk.modules.rapidfire import RapidFire

keyboard = KMKKeyboard()

keyboard.col_pins = (board.GP0,)
keyboard.row_pins = (board.GP1,)
keyboard.diode_orientation = DiodeOrientation.COL2ROW

keyboard.modules.append(RapidFire())

test = KC.RF(KC.LSFT(KC.A), timeout=200, interval=100)

keyboard.keymap = [
    [test,]
]

if __name__ == '__main__':
    keyboard.go()
JamesToBoot commented 2 months ago

Confirmed: I got the same error running todays version of KMK on CircuitPython 8.2.7 (because thats what my production keyboard is currently running)

I reverted back to a KMK version from around the same date as the CircuitPython version (2023-10-19) and the code copiles without error.

JamesToBoot commented 2 months ago

wow @xs5871, how long did it take you to write up pull request 1026?

tinyboatproductions commented 2 months ago

Testing with updated rapidfire.py from pull 1026 works. Also that was fast af.

xs5871 commented 2 months ago

wow @xs5871, how long did it take you to write up pull request 1026?

Fixing the reported issue: maybe 20 seconds. A tiny and obvious mistake that only happend because there were zero unit tests. Adding unit tests, getting confused about weird inconsistent behavior (to the point were I was amazed how it wasn't reported as fundamentally broken long ago), fixing that + the rewrite because I got anoyed by all the jank: more like 4-5 hours.

JamesToBoot commented 2 months ago

Thanks for sharing. Good info and more for me to learn. Could you, by any chance, point out the obvious mistake?... for those of us curious enough to look thru the code and your PR for 30minutes and couldn't find it

xs5871 commented 2 months ago

**kwargs needs two asterisks to pack/unpack into a dictionary, i.e. catch additional named (=keyword) arguments like on_press which aren't explicitely defined in the RapidFireKey constructor.