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

AL1 Unable to Compile on Configurator #3339

Merged
merged 3 commits into from
Jul 9, 2018

Conversation

mechmerlin
Copy link
Contributor

On the Configurator, you get the following

Linking: .build/al1_layout_mine.elf                                                                 �
 | 
 | .build/obj_al1_layout_mine/keyboards/al1/al1.o: In function `matrix_init_kb':
 | /qmk_compiler_worker/qmk_firmware/keyboards/al1/al1.c:19: undefined reference to `matrix_init_user'
 | .build/obj_al1_layout_mine/keyboards/al1/al1.o: In function `matrix_scan_kb':
 | /qmk_compiler_worker/qmk_firmware/keyboards/al1/al1.c:23: undefined reference to `matrix_scan_user'
 | collect2: error: ld returned 1 exit status
 | 

However you don't hit this when making any of the two default keymaps currently in AL1 directory.

The two matrix routines need matrix.h included. I figured it would be better to just define them in matrix. c which already has matrix.h included.

Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you check the normal matrix.c, the reason this works normally but not here, is that they're defined weakly in the matrix:
https://github.com/qmk/qmk_firmware/blob/master/quantum/matrix.c#L97-L113

@@ -15,14 +15,6 @@
*/
#include "al1.h"

void matrix_init_kb(void) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, these should remain.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -28,6 +28,13 @@ inline uint8_t matrix_cols(void) {
return MATRIX_COLS;
}

void matrix_init_kb(void) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, these should be set as "weak".

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@drashna
Copy link
Member

drashna commented Jul 9, 2018

Well, I meant having the weak defines in the matrix.c file (as the main matrix.c file does that)

However, this should be fine, as well.

@drashna drashna merged commit 5ef5025 into qmk:master Jul 9, 2018
alexey-danilov pushed a commit to alexey-danilov/qmk_firmware that referenced this pull request Jul 27, 2018
* use QMK_KEYBOARD_H

* init_kb and scan_kb need to be in matrix.c to make use of the matrix.h include

* Make the routines weak as suggested by Drashna
@mechmerlin mechmerlin deleted the feature/al1_fixes branch August 3, 2018 03:59
ChrissiQ pushed a commit to ChrissiQ/qmk_firmware that referenced this pull request Sep 25, 2018
* use QMK_KEYBOARD_H

* init_kb and scan_kb need to be in matrix.c to make use of the matrix.h include

* Make the routines weak as suggested by Drashna
yamad pushed a commit to yamad/qmk_firmware that referenced this pull request Apr 10, 2019
* use QMK_KEYBOARD_H

* init_kb and scan_kb need to be in matrix.c to make use of the matrix.h include

* Make the routines weak as suggested by Drashna
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants