From 8fa5259ae568e693f4301255dfbf7b1b2262bb02 Mon Sep 17 00:00:00 2001 From: xs5871 <60395129+xs5871@users.noreply.github.com> Date: Tue, 19 Nov 2024 22:39:19 +0000 Subject: [PATCH] Cleanup debug (#1048) * Replace print() with guarded debug() * Remove the legacy `keyboard.debug_enabled` flag and document the debug utility proper * Remove KC.DEBUG from keys.py --------- Co-authored-by: regicidal.plutophage <36969337+regicidalplutophage@users.noreply.github.com> --- boards/atreus62/main.py | 1 - boards/crowboard/main.py | 2 - boards/fingerpunch/ffkb/nice_nano/main.py | 1 - .../fingerpunch/ffkb/other_pro_micro/main.py | 1 - boards/keebio/iris/main.py | 1 - boards/kyria/main.py | 1 - boards/splitkb/aurora/lily58/main.py | 1 - docs/en/config_and_keymap.md | 4 -- docs/en/debugging.md | 24 +++++++++- docs/en/encoder.md | 1 - docs/en/extension_stringy_keymaps.md | 6 +-- docs/en/keycodes.md | 2 +- kmk/extensions/led.py | 7 ++- kmk/extensions/peg_rgb_matrix.py | 12 +++-- kmk/extensions/statusled.py | 6 ++- kmk/handlers/stock.py | 8 ++-- kmk/keys.py | 1 - kmk/kmk_keyboard.py | 8 ---- kmk/modules/adns9800.py | 19 ++++---- kmk/modules/encoder.py | 13 +++-- kmk/modules/midi.py | 7 ++- kmk/modules/split.py | 48 ++++++++++--------- kmk/utils.py | 1 + tests/keyboard_test.py | 6 ++- 24 files changed, 103 insertions(+), 78 deletions(-) diff --git a/boards/atreus62/main.py b/boards/atreus62/main.py index 0820dc30e..b2ccb63ca 100644 --- a/boards/atreus62/main.py +++ b/boards/atreus62/main.py @@ -32,7 +32,6 @@ keyboard.modules = [layers, encoder, macros] keyboard.tap_time = 250 -keyboard.debug_enabled = False # custom keys diff --git a/boards/crowboard/main.py b/boards/crowboard/main.py index e83b13b28..3dc28ef48 100644 --- a/boards/crowboard/main.py +++ b/boards/crowboard/main.py @@ -85,7 +85,5 @@ # ((KC.VOLD, KC.VOLU),(KC.VOLD, KC.VOLU),), # Layer 4 # ] -# keyboard.debug_enabled = True - if __name__ == '__main__': keyboard.go() diff --git a/boards/fingerpunch/ffkb/nice_nano/main.py b/boards/fingerpunch/ffkb/nice_nano/main.py index 28cb778ee..d2c0831ce 100644 --- a/boards/fingerpunch/ffkb/nice_nano/main.py +++ b/boards/fingerpunch/ffkb/nice_nano/main.py @@ -18,7 +18,6 @@ keyboard = kb.KMKKeyboard() keyboard.modules = [combos, dyn_seq, layers, sticky_keys] -keyboard.debug_enabled = False # Convenience variables for the Keymap _______ = KC.TRNS diff --git a/boards/fingerpunch/ffkb/other_pro_micro/main.py b/boards/fingerpunch/ffkb/other_pro_micro/main.py index fb406cdd6..2aa97ab03 100644 --- a/boards/fingerpunch/ffkb/other_pro_micro/main.py +++ b/boards/fingerpunch/ffkb/other_pro_micro/main.py @@ -12,7 +12,6 @@ keyboard = KMKKeyboard() keyboard.tap_time = 150 -keyboard.debug_enabled = False # Cleaner key names _______ = KC.TRNS diff --git a/boards/keebio/iris/main.py b/boards/keebio/iris/main.py index c94bcf42d..4a043293d 100644 --- a/boards/keebio/iris/main.py +++ b/boards/keebio/iris/main.py @@ -7,7 +7,6 @@ keyboard = KMKKeyboard() -keyboard.debug_enabled = False keyboard.tap_time = 750 _______ = KC.TRNS diff --git a/boards/kyria/main.py b/boards/kyria/main.py index 4175f96fb..7c36c1d30 100644 --- a/boards/kyria/main.py +++ b/boards/kyria/main.py @@ -9,7 +9,6 @@ from kmk.modules.split import Split, SplitType keyboard = KMKKeyboard() -keyboard.debug_enabled = True keyboard.modules.append(Layers()) keyboard.modules.append(HoldTap()) diff --git a/boards/splitkb/aurora/lily58/main.py b/boards/splitkb/aurora/lily58/main.py index c2b10cde5..5e6de20de 100644 --- a/boards/splitkb/aurora/lily58/main.py +++ b/boards/splitkb/aurora/lily58/main.py @@ -10,7 +10,6 @@ from kmk.modules.split import Split keyboard = KMKKeyboard() -# keyboard.debug_enabled = True # Adding modules # Using drive names (LILY58L, LILY58R) to recognize sides; use split_side arg if you're not doing it diff --git a/docs/en/config_and_keymap.md b/docs/en/config_and_keymap.md index e9a85f418..664d637e8 100644 --- a/docs/en/config_and_keymap.md +++ b/docs/en/config_and_keymap.md @@ -82,10 +82,6 @@ keyboard.keymap = [[KC.A, KC.B]] You can further define a bunch of other stuff: -- `keyboard.debug_enabled` which will spew a ton of debugging information to the serial - console. This is very rarely needed, but can provide very valuable information - if you need to open an issue. - - `keyboard.tap_time` which defines how long `KC.TT` and `KC.LT` will wait before considering a key "held" (see `layers.md`). diff --git a/docs/en/debugging.md b/docs/en/debugging.md index b026f9790..e33429ae4 100644 --- a/docs/en/debugging.md +++ b/docs/en/debugging.md @@ -1,13 +1,33 @@ # Debugging + KMK's debug output is written to CircuitPython's serial console -- the one that's used for the REPL -- and is automatically enabled if it detects a connection to that console. It can also be enabled manually, though that shouldn't be necessary in -general: +general. + +## KMK's Debug Utility + +KMK has a convenient debug utility that adds a timestamp in milliseconds since boot and a message origin to distinguish subsystems to debug statements. + ```python -keyboard.debug_enabled = True +from kmk.utils import Debug + +# Create a debug source with the current file as message origin +debug = Debug(__name__) + +# For completeness: Force enable/disable debug output. This is handled +# automatically -- you will most likely never have to use this: +# debug.enabled = True/False + +# KMK idiomatic debug with guard clause +var = 'concatenate' +if debug.enabled: + debug('Arguments ', var, '!') ``` +## Connecting to the Serial Console + Follow for example Adafruit's beginners guide on [how to connect to the serial console](https://learn.adafruit.com/welcome-to-circuitpython/kattni-connecting-to-the-serial-console). For Linux users, we recommend [picocom](https://github.com/npat-efault/picocom) or [screen](https://www.gnu.org/software/screen/manual/screen.html) diff --git a/docs/en/encoder.md b/docs/en/encoder.md index 45eec014f..0378d0f91 100644 --- a/docs/en/encoder.md +++ b/docs/en/encoder.md @@ -148,7 +148,6 @@ keyboard.diode_orientation = DiodeOrientation.COLUMNS encoder_handler.pins = ((board.GP17, board.GP15, board.GP14, False),) keyboard.tap_time = 250 -keyboard.debug_enabled = False # Filler keys diff --git a/docs/en/extension_stringy_keymaps.md b/docs/en/extension_stringy_keymaps.md index 7d8777f71..f20880c7f 100644 --- a/docs/en/extension_stringy_keymaps.md +++ b/docs/en/extension_stringy_keymaps.md @@ -22,14 +22,10 @@ keyboard.keymap = [[ 'A' , 'B', 'RESET' ]] stringyKeymaps = StringyKeymaps() -# Enabling debug will show each replacement or failure. -# This is recommended during the initial development of a keyboard. -# stringyKeymaps.debug_enable = True - keyboard.extensions.append(stringyKeymaps) ``` It should be noted that these are **not** ASCII. The string is **not** what will be sent to the computer. The examples above have no functional difference. -When utilizing argumented keys, such as `KC.MO(layer)`, it's not possible to use a string like `'MO(layer)'` instead employ the standard notation of e.g. `KC.MO(1)` in your keymap. \ No newline at end of file +When utilizing argumented keys, such as `KC.MO(layer)`, it's not possible to use a string like `'MO(layer)'` instead employ the standard notation of e.g. `KC.MO(1)` in your keymap. diff --git a/docs/en/keycodes.md b/docs/en/keycodes.md index d242090cc..92a9d5fee 100644 --- a/docs/en/keycodes.md +++ b/docs/en/keycodes.md @@ -162,7 +162,7 @@ |-------------------------|------------------------------------------------------------------------| | `KC.RESET` | Restarts the keyboard | | `KC.RELOAD`, `KC.RLD` | Reloads the keyboard software, preserving any serial connections | -| `KC.DEBUG` | Toggle `debug_enabled`, which enables log spew to serial console | +| `KC.DEBUG` | Toggle `debug.enabled`, which enables log spew to serial console | | `KC.ANY` | Any key between `A` and `/` | | `KC.GESC` | Escape when tapped, ` when pressed with Shift or GUI | | `KC.BKDL` | Backspace when tapped, Delete when pressed with GUI | diff --git a/kmk/extensions/led.py b/kmk/extensions/led.py index 1cdcbc508..cd155efb7 100644 --- a/kmk/extensions/led.py +++ b/kmk/extensions/led.py @@ -3,7 +3,9 @@ from kmk.extensions import Extension, InvalidExtensionEnvironment from kmk.keys import Key, make_argumented_key, make_key -from kmk.utils import clamp +from kmk.utils import Debug, clamp + +debug = Debug(__name__) class LEDKey(Key): @@ -46,7 +48,8 @@ def __init__( try: self._leds = [pwmio.PWMOut(pin) for pin in pins_iter] except Exception as e: - print(e) + if debug.enabled: + debug(e) raise InvalidExtensionEnvironment( 'Unable to create pwmio.PWMOut() instance with provided led_pin' ) diff --git a/kmk/extensions/peg_rgb_matrix.py b/kmk/extensions/peg_rgb_matrix.py index 1e81d635e..816dc5783 100644 --- a/kmk/extensions/peg_rgb_matrix.py +++ b/kmk/extensions/peg_rgb_matrix.py @@ -4,6 +4,9 @@ from kmk.extensions import Extension from kmk.keys import make_key +from kmk.utils import Debug + +debug = Debug(__name__) class Color: @@ -26,10 +29,12 @@ class Color: class Rgb_matrix_data: def __init__(self, keys=[], underglow=[]): if len(keys) == 0: - print('No colors passed for your keys') + if debug.enabled: + debug('No colors passed for your keys') return if len(underglow) == 0: - print('No colors passed for your underglow') + if debug.enabled: + debug('No colors passed for your underglow') return self.data = keys + underglow @@ -39,7 +44,8 @@ def generate_led_map( ): keys = [key_color] * number_of_keys underglow = [underglow_color] * number_of_underglow - print(f'Rgb_matrix_data(keys={keys},\nunderglow={underglow})') + if debug.enabled: + debug('Rgb_matrix_data(keys=', keys, ', nunderglow=', underglow, ')') class Rgb_matrix(Extension): diff --git a/kmk/extensions/statusled.py b/kmk/extensions/statusled.py index 3e56f5aef..7610ca121 100644 --- a/kmk/extensions/statusled.py +++ b/kmk/extensions/statusled.py @@ -5,6 +5,9 @@ from kmk.extensions import Extension, InvalidExtensionEnvironment from kmk.keys import make_key +from kmk.utils import Debug + +debug = Debug(__name__) class statusLED(Extension): @@ -20,7 +23,8 @@ def __init__( try: self._leds.append(pwmio.PWMOut(led)) except Exception as e: - print(e) + if debug.enabled: + debug(e) raise InvalidExtensionEnvironment( 'Unable to create pulseio.PWMOut() instance with provided led_pin' ) diff --git a/kmk/handlers/stock.py b/kmk/handlers/stock.py index 6921908c1..5ac5d0193 100644 --- a/kmk/handlers/stock.py +++ b/kmk/handlers/stock.py @@ -25,12 +25,10 @@ def bootloader(*args, **kwargs): def debug_pressed(key, keyboard, KC, *args, **kwargs): - if keyboard.debug_enabled: - print('DebugDisable()') - else: - print('DebugEnable()') + from kmk.utils import Debug - keyboard.debug_enabled = not keyboard.debug_enabled + debug = Debug() + debug.enabled = not debug.enabled return keyboard diff --git a/kmk/keys.py b/kmk/keys.py index ce0d51385..e34486392 100644 --- a/kmk/keys.py +++ b/kmk/keys.py @@ -290,7 +290,6 @@ def maybe_make_firmware_key(candidate: str) -> Optional[Key]: ((('BLE_REFRESH',), handlers.ble_refresh)), ((('BLE_DISCONNECT',), handlers.ble_disconnect)), ((('BOOTLOADER',), handlers.bootloader)), - ((('DEBUG', 'DBG'), handlers.debug_pressed)), ((('HID_SWITCH', 'HID'), handlers.hid_switch)), ((('RELOAD', 'RLD'), handlers.reload)), ((('RESET',), handlers.reset)), diff --git a/kmk/kmk_keyboard.py b/kmk/kmk_keyboard.py index dd9316845..96d69059f 100644 --- a/kmk/kmk_keyboard.py +++ b/kmk/kmk_keyboard.py @@ -185,14 +185,6 @@ def _process_resume_buffer(self): self._resume_buffer_x = buffer - @property - def debug_enabled(self) -> bool: - return debug.enabled - - @debug_enabled.setter - def debug_enabled(self, enabled: bool): - debug.enabled = enabled - def pre_process_key( self, key: Key, diff --git a/kmk/modules/adns9800.py b/kmk/modules/adns9800.py index 108b1dd52..6b0302332 100644 --- a/kmk/modules/adns9800.py +++ b/kmk/modules/adns9800.py @@ -7,6 +7,9 @@ from kmk.keys import AX from kmk.modules import Module from kmk.modules.adns9800_firmware import firmware +from kmk.utils import Debug + +debug = Debug(__name__) class REG: @@ -177,17 +180,17 @@ def during_bootup(self, keyboard): self.adns_write(REG.Configuration_I, 0x10) microcontroller.delay_us(self.tsww) - if keyboard.debug_enabled: - print('ADNS: Product ID ', hex(self.adns_read(REG.Product_ID))) + if debug.enabled: + debug('ADNS: Product ID ', hex(self.adns_read(REG.Product_ID))) microcontroller.delay_us(self.tsrr) - print('ADNS: Revision ID ', hex(self.adns_read(REG.Revision_ID))) + debug('ADNS: Revision ID ', hex(self.adns_read(REG.Revision_ID))) microcontroller.delay_us(self.tsrr) - print('ADNS: SROM ID ', hex(self.adns_read(REG.SROM_ID))) + debug('ADNS: SROM ID ', hex(self.adns_read(REG.SROM_ID))) microcontroller.delay_us(self.tsrr) if self.adns_read(REG.Observation) & 0x20: - print('ADNS: Sensor is running SROM') + debug('ADNS: Sensor is running SROM') else: - print('ADNS: Error! Sensor is not running SROM!') + debug('ADNS: Error! Sensor is not running SROM!') return @@ -208,8 +211,8 @@ def before_matrix_scan(self, keyboard): if delta_y: AX.Y.move(keyboard, delta_y) - if keyboard.debug_enabled: - print('Delta: ', delta_x, ' ', delta_y) + if debug.enabled: + debug('Delta: ', delta_x, ' ', delta_y) def after_matrix_scan(self, keyboard): return diff --git a/kmk/modules/encoder.py b/kmk/modules/encoder.py index 43c18b338..ca63610a2 100644 --- a/kmk/modules/encoder.py +++ b/kmk/modules/encoder.py @@ -5,6 +5,9 @@ from supervisor import ticks_ms from kmk.modules import Module +from kmk.utils import Debug + +debug = Debug(__name__) # NB : not using rotaryio as it requires the pins to be consecutive @@ -107,7 +110,6 @@ def button_event(self): # return knob velocity as milliseconds between position changes (detents) # for backwards compatibility def vel_report(self): - # print(self._velocity) return self._velocity @@ -178,7 +180,8 @@ def __init__(self, i2c, address, is_inverted=False): try: from adafruit_seesaw import digitalio, neopixel, rotaryio, seesaw except ImportError: - print('seesaw missing') + if debug.enabled: + debug('seesaw missing') return super().__init__(is_inverted) @@ -189,7 +192,8 @@ def __init__(self, i2c, address, is_inverted=False): seesaw_product = (self.seesaw.get_version() >> 16) & 0xFFFF if seesaw_product != 4991: - print('Wrong firmware loaded? Expected 4991') + if debug.enabled: + debug('Wrong firmware loaded? Expected 4991') self.encoder = rotaryio.IncrementalEncoder(self.seesaw) self.seesaw.pin_mode(24, self.seesaw.INPUT_PULLUP) @@ -281,7 +285,8 @@ def during_bootup(self, keyboard): ) self.encoders.append(new_encoder) except Exception as e: - print(e) + if debug.enabled: + debug(e) return def on_move_do(self, keyboard, encoder_id, state): diff --git a/kmk/modules/midi.py b/kmk/modules/midi.py index 2cee603c0..ca339f983 100644 --- a/kmk/modules/midi.py +++ b/kmk/modules/midi.py @@ -10,6 +10,9 @@ from kmk.keys import Key, make_argumented_key from kmk.modules import Module +from kmk.utils import Debug + +debug = Debug(__name__) class MidiKey(Key): @@ -73,8 +76,8 @@ def __init__(self): self.midi = adafruit_midi.MIDI(midi_out=usb_midi.ports[1], out_channel=0) except IndexError: self.midi = None - # if debug_enabled: - print('No midi device found.') + if debug.enabled: + debug('No midi device found.') def during_bootup(self, keyboard): return None diff --git a/kmk/modules/split.py b/kmk/modules/split.py index bbf9cfc67..a54b3e93f 100644 --- a/kmk/modules/split.py +++ b/kmk/modules/split.py @@ -10,6 +10,9 @@ from kmk.hid import HIDModes from kmk.kmktime import check_deadline from kmk.modules import Module +from kmk.utils import Debug + +debug = Debug(__name__) class SplitSide: @@ -38,7 +41,6 @@ def __init__( data_pin2=None, uart_flip=True, use_pio=False, - debug_enabled=False, ): self._is_target = True self._uart_buffer = [] @@ -53,7 +55,6 @@ def __init__( self._use_pio = use_pio self._uart = None self._uart_interval = uart_interval - self._debug_enabled = debug_enabled self.uart_header = bytearray([0xB2]) # Any non-zero byte should work if self.split_type == SplitType.BLE: @@ -68,7 +69,8 @@ def __init__( self.ProvideServicesAdvertisement = ProvideServicesAdvertisement self.UARTService = UARTService except ImportError: - print('BLE Import error') + if debug.enabled: + debug('BLE Import error') return # BLE isn't supported on this platform self._ble_last_scan = ticks_ms() - 5000 self._connection_count = 0 @@ -158,8 +160,9 @@ def during_bootup(self, keyboard): cm.append(cols_to_calc * (rows_to_calc + ridx) + cidx) keyboard.coord_mapping = tuple(cm) - else: - print('Error: please provide coord_mapping for custom scanner') + + if not keyboard.coord_mapping and debug.enabled: + debug('Error: please provide coord_mapping for custom scanner') if self.split_side == SplitSide.RIGHT: offset = self.split_offset @@ -190,7 +193,8 @@ def after_matrix_scan(self, keyboard): elif self.split_type == SplitType.ONEWIRE: pass # Protocol needs written else: - print('Unexpected case in after_matrix_scan') + if debug.enabled: + debug('Unexpected case in after_matrix_scan') return @@ -264,21 +268,21 @@ def _initiator_scan(self): break if not self._uart: - if self._debug_enabled: - print('Scanning') + if debug.enabled: + debug('Scanning') self._ble.stop_scan() for adv in self._ble.start_scan( self.ProvideServicesAdvertisement, timeout=20 ): - if self._debug_enabled: - print('Scanning') + if debug.enabled: + debug('Scanning') if self.UARTService in adv.services and adv.rssi > -70: self._uart_connection = self._ble.connect(adv) self._uart_connection.connection_interval = 11.25 self._uart = self._uart_connection[self.UARTService] self._ble.stop_scan() - if self._debug_enabled: - print('Scan complete') + if debug.enabled: + debug('Scan complete') break self._ble.stop_scan() @@ -287,8 +291,8 @@ def _target_advertise(self): # Give previous advertising some time to complete if self._advertising: if self._check_if_split_connected(): - if self._debug_enabled: - print('Advertising complete') + if debug.enabled: + debug('Advertising complete') self._ble.stop_advertising() self._advertising = False return @@ -296,12 +300,12 @@ def _target_advertise(self): if not self.ble_rescan_timer(): return - if self._debug_enabled: - print('Advertising not answered') + if debug.enabled: + debug('Advertising not answered') self._ble.stop_advertising() - if self._debug_enabled: - print('Advertising') + if debug.enabled: + debug('Advertising') # Uart must not change on this connection if reconnecting if not self._uart: self._uart = self.UARTService() @@ -337,11 +341,11 @@ def _send_ble(self, update): try: self._uart.disconnect() except: # noqa: E722 - if self._debug_enabled: - print('UART disconnect failed') + if debug.enabled: + debug('UART disconnect failed') - if self._debug_enabled: - print('Connection error') + if debug.enabled: + debug('Connection error') self._uart_connection = None self._uart = None diff --git a/kmk/utils.py b/kmk/utils.py index f6e6494ea..e0830d9d2 100644 --- a/kmk/utils.py +++ b/kmk/utils.py @@ -46,3 +46,4 @@ def enabled(self) -> bool: def enabled(self, enabled: bool): global _debug_enabled _debug_enabled = enabled + self('debug.enabled=', enabled) diff --git a/tests/keyboard_test.py b/tests/keyboard_test.py index ca5e3421b..2d44bdca8 100644 --- a/tests/keyboard_test.py +++ b/tests/keyboard_test.py @@ -9,6 +9,9 @@ from kmk.kmk_keyboard import KMKKeyboard from kmk.scanners import DiodeOrientation from kmk.scanners.digitalio import MatrixScanner +from kmk.utils import Debug + +debug = Debug(__name__) class DigitalInOut(Mock): @@ -39,7 +42,8 @@ def __init__( self.debug_enabled = debug_enabled self.keyboard = KMKKeyboard() - self.keyboard.debug_enabled = keyboard_debug_enabled + if keyboard_debug_enabled: + debug.enabled = True self.keyboard.modules = modules self.keyboard.extensions = extensions