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
Add kb and user level keyboard initialization functions #3113
Changes from all commits
e92b2b5
cfe9c6c
f2ec9cb
2adde43
9bbce97
c733ca8
20491f0
8efd254
205b419
7ded7da
1b25ffb
1d78ec8
8b519fe
4ceb0b7
570067c
65ffa3e
fb85430
5c4a3e6
1ad1cf2
8c8d53a
dad3525
6e5d2aa
dea7413
b563e5e
ae36d65
22e49c6
f91f579
1470579
90407a3
5018143
5c74fdd
98068cd
fedcd10
23d4068
af02782
58f6b2e
77565b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -139,13 +139,48 @@ __attribute__ ((weak)) | |
void matrix_setup(void) { | ||
} | ||
|
||
/** \brief keyboard_pre_init_user | ||
* | ||
* FIXME: needs doc | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume you'll fix these before merging? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe? Nothing else is actually documented. And I'm not sure of the formatting for this still. |
||
*/ | ||
__attribute__ ((weak)) | ||
void keyboard_pre_init_user(void) { } | ||
|
||
/** \brief keyboard_pre_init_kb | ||
* | ||
* FIXME: needs doc | ||
*/ | ||
__attribute__ ((weak)) | ||
void keyboard_pre_init_kb(void) { | ||
keyboard_pre_init_user(); | ||
} | ||
|
||
/** \brief keyboard_post_init_user | ||
* | ||
* FIXME: needs doc | ||
*/ | ||
|
||
__attribute__ ((weak)) | ||
void keyboard_post_init_user() {} | ||
|
||
/** \brief keyboard_post_init_kb | ||
* | ||
* FIXME: needs doc | ||
*/ | ||
|
||
__attribute__ ((weak)) | ||
void keyboard_post_init_kb(void) { | ||
keyboard_post_init_user(); | ||
} | ||
|
||
/** \brief keyboard_setup | ||
* | ||
* FIXME: needs doc | ||
*/ | ||
void keyboard_setup(void) { | ||
disable_jtag(); | ||
matrix_setup(); | ||
keyboard_pre_init_kb(); | ||
} | ||
|
||
/** \brief is_keyboard_master | ||
|
@@ -199,6 +234,7 @@ void keyboard_init(void) { | |
#if defined(NKRO_ENABLE) && defined(FORCE_NKRO) | ||
keymap_config.nkro = 1; | ||
#endif | ||
keyboard_post_init_kb(); /* Always keep this last */ | ||
} | ||
|
||
/** \brief Keyboard task: Do keyboard routine jobs | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,6 +119,8 @@ int main(void) { | |
// TESTING | ||
// chThdCreateStatic(waThread1, sizeof(waThread1), NORMALPRIO, Thread1, NULL); | ||
|
||
keyboard_setup(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment for the other main.c There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what you mean here. But |
||
|
||
/* Init USB */ | ||
init_usb_driver(&USB_DRIVER); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I like the word "Post" here. That implies that initialization should be complete by this point. I don't have any immediate suggestions for better names, but to me the naming of this means that I have no idea what the purpose of this function is. It's post initialization, so nothing should be initialized here, but it's also not called in a loop or anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps
user_code_init
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The biggest issue with the name is that there are 3 inits. Which ... is far from ideal.
Though, the post init is the last function run as part of the keyboard setup, and should always be the last thing called.
The problem with
user_code_init
is... the functionality levels.user_code_init_kb
, anduser_code_init_user
?init_cleanup? init_finished?
I'm not great with naming, and this was close in name to existing stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, one question: does there need to be a
_kb
and_user
version of this function? What would the keyboard need to do that it doesn't already have a callback for?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, for one, it would be a better place to start the setup for SSD1306 screens. Or additional keyboard specific hardware, when matrix_init wouldn't necessarily be appropriate.