Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] combos introspection fails to compile when in user space #21137

Closed
2 tasks
EdenEast opened this issue Jun 6, 2023 · 5 comments
Closed
2 tasks

[Bug] combos introspection fails to compile when in user space #21137

EdenEast opened this issue Jun 6, 2023 · 5 comments

Comments

@EdenEast
Copy link

EdenEast commented Jun 6, 2023

Describe the Bug

After the introduction of PR #19670 qmk fails to compile when defining key_combos in userspace.

Compiling: quantum/keymap_introspection.c            quantum/keymap_introspection.c: In function ‘combo_count_raw’:
quantum/keymap_introspection.c:80:19: error: ‘key_combos’ undeclared (first use in this function); did you mean ‘keymaps’?
     return sizeof(key_combos) / sizeof(combo_t);
                   ^~~~~~~~~~
                   keymaps
quantum/keymap_introspection.c:80:19: note: each undeclared identifier is reported only once for each function it appears in
quantum/keymap_introspection.c: In function ‘combo_get_raw’:
quantum/keymap_introspection.c:87:13: error: ‘key_combos’ undeclared (first use in this function); did you mean ‘keymaps’?
     return &key_combos[combo_idx];
             ^~~~~~~~~~
             keymaps
quantum/keymap_introspection.c: In function ‘combo_count_raw’:
quantum/keymap_introspection.c:81:1: error: control reaches end of non-void function [-Werror=return-type]
 }
 ^
quantum/keymap_introspection.c: In function ‘combo_get_raw’:
quantum/keymap_introspection.c:88:1: error: control reaches end of non-void function [-Werror=return-type]
 }
 ^
cc1: all warnings being treated as errors
 [ERRORS]

Some examples of keyboard builds that fail because of this:

make handwired/dactyl_manuform/4x5:ibnuda
make crkbd:ninjonas
make splitkb/kyria:zigotica

This does not happen when moving the code to the keymap.c file.

Keyboard Used

handwired/dactyl_manuform/4x5:ibnuda

Link to product page (if applicable)

No response

Operating System

linux

qmk doctor Output

Ψ QMK Doctor is checking your environment.
Ψ CLI version: 1.1.2
Ψ QMK home: /home/eden/dev/keyboard/qmk/firmware
Ψ Detected Linux (WSL, Ubuntu 20.04.6 LTS).
⚠ QMK home does not appear to be a Git repository! (no .git folder)
Ψ All dependencies are installed.
Ψ Found arm-none-eabi-gcc version 12.2.1
Ψ Found avr-gcc version 8.5.0
Ψ Found avrdude version 7.1
Ψ Found dfu-programmer version 0.7.2
Ψ Found dfu-util version 0.11
Ψ Submodules are up to date.
Ψ Submodule status:
Ψ - lib/chibios: 2023-04-15 13:48:04 +0000 --  (11edb1610)
Ψ - lib/chibios-contrib: 2023-01-11 16:42:27 +0100 --  (a224be15)
Ψ - lib/googletest: 2021-06-11 06:37:43 -0700 --  (e2239ee6)
Ψ - lib/lufa: 2022-08-26 12:09:55 +1000 --  (549b97320)
Ψ - lib/vusb: 2022-06-13 09:18:17 +1000 --  (819dbc1)
Ψ - lib/printf: 2022-06-29 23:59:58 +0300 --  (c2e3b4e)
Ψ - lib/pico-sdk: 2023-02-12 20:19:37 +0100 --  (a3398d8)
Ψ - lib/lvgl: 2022-04-11 04:44:53 -0600 --  (e19410f8)
Ψ QMK is ready to go, but minor problems were found

Is AutoHotKey / Karabiner installed

  • AutoHotKey (Windows)
  • Karabiner (macOS)

Other keyboard-related software installed

No response

Additional Context

No response

@tzarc
Copy link
Member

tzarc commented Jun 6, 2023

Maintenance of userspace keymaps is not always QMK collaborator's concern -- in the past people have been abusive when things have changed without their input. We'll generally try to handle things like keycode renames, but anything more complex is left to the user to fix.

That said, in the case of combos (or any other introspection), there's a directive usable within the keymap's rules.mk file:

INTROSPECTION_KEYMAP_C = <my_combos_file>.c

This will auto-include the specified file in the introspection preamble so that it makes them available to the introspection functions.
If you're already specifying it in a SRC += <my_combos_file>.c elsewhere, then it'll need to be removed from the SRC line.

Generally people specify combos in keymap.c, so 99% of cases are already catered for.

@EdenEast
Copy link
Author

EdenEast commented Jun 6, 2023

Thanks for the response. I used that those user space keymaps as they were examples of the same compile failures that I was having while upgrading my qmk_firmware submodule.

Thanks I was not aware of INTROSPECTION_KEYMAP_C. Using this solved my compile failures. Searching the docs, there is no mention of INTROSPECTION_KEYMAP_C. This might be good to add to the docs.

@tzarc
Copy link
Member

tzarc commented Jun 13, 2023

Under normal circumstances I'd agree with you, but the docs explicitly state that combos should be placed in keymap.c.
For 99% of usecases people abide by that, and introspection works fine. INTROSPECTION_KEYMAP_C is a "get out of jail" necessity and not something that should be promoted.

@tzarc tzarc closed this as completed Jun 13, 2023
pbrehm added a commit to pbrehm/qmk_firmware that referenced this issue Mar 25, 2024
@uqs
Copy link

uqs commented Jun 9, 2024

I just ran into this problem myself, and I'm not sure how having them in keymap.c is all that useful. I have a DacMan, a Kyria, a Preonic and my custom DacMan and I obviously (?) want the same combos on all of them.

How would you suggest that I organize this in code, other than my common uqs.c file that I use for all keymaps?

@tzarc
Copy link
Member

tzarc commented Jun 9, 2024

How would you suggest that I organize this in code, other than my common uqs.c file that I use for all keymaps?

Move the list of combos to a different file, and use INTROSPECTION_KEYMAP_C as described above.

Locking this issue as there's no point necro'ing it.

@qmk qmk locked as resolved and limited conversation to collaborators Jun 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants