Implement Keyboard Host via PIO #287
Conversation
deeebug
requested review from
Fortinbra,
mthiesen,
arntsonl,
FeralAI and
TheTrainGoes
and removed request for
Fortinbra
src/addons/keyboard_host.cpp
Outdated
|
|
||
| #include "pio_usb.h" | ||
|
|
||
| GamepadState _keyboard_host_state; |
There was a problem hiding this comment.
All the global variables should be static (all the _keyboard_host_xxx variables). in C++ static globals have "internal linkage", meaning they are only visible in the .cpp they are declared in.
Otherwise the program will fail to link if there are name clashes in the global scope. Admittedly highly unlikely in this case but this is the idiomatic thing to do in C++.
src/addons/keyboard_host.cpp
Outdated
| tuh_configure(1, TUH_CFGID_RPI_PIO_USB_CONFIGURATION, &pio_cfg); | ||
| tuh_init(BOARD_TUH_RHPORT); | ||
|
|
||
| _keyboard_host_mapDpadUp = new KeyboardButtonMapping(keyboardMapping.keyDpadUp , GAMEPAD_MASK_UP); |
There was a problem hiding this comment.
I don't see any reason to work with dynamic allocations here. Just put the structs in the global scope, not pointers. There is no need for this additional indirection.
src/addons/keyboard_host.cpp
Outdated
| } | ||
|
|
||
| void KeyboardHostAddon::setup() { | ||
| KeyboardMapping& keyboardMapping = Storage::getInstance().getAddonOptions().keyboardHostOptions.mapping; |
There was a problem hiding this comment.
This can be a const reference. You are not modifying the struct behind the reference.
src/addons/keyboard_host.cpp
Outdated
| uint8_t keycode = (i < 6) ? report->keycode[i] : getKeycodeFromModifier(report->modifier); | ||
| if ( keycode ) | ||
| { | ||
| GamepadOptions& gamepadOptions = Storage::getInstance().getGamepadOptions(); |
There was a problem hiding this comment.
This can be a const reference. You are not modifying the struct behind the reference.
|
@mthiesen thank you very much for the review! |
deeebug
force-pushed
the
keyboard_host_addon
branch
from
ff9215c
to
72882c8
Compare
deeebug
force-pushed
the
keyboard_host_addon
branch
from
72882c8
to
9e0a1ca
Compare
|
This PR looks so awesome! I don't have access to a rpi pico until one more month but I'm sure I will test this ASAP 💕💕 |
| @@ -175,7 +174,7 @@ app.get("/api/getPinMappings", (req, res) => { | |||
| }); | |||
|
|
|||
| app.get("/api/getKeyMappings", (req, res) => | |||
| res.send(mapValues(keyboardMapping)) | |||
| res.send(mapValues(DEFAULT_KEYBOARD_MAPPING)) | |||
There was a problem hiding this comment.
is this right? we only send default keyboard mapping?
There was a problem hiding this comment.
This is just the default data sent to the dev server, so it's good.
| setValidated(true); | ||
| }; | ||
|
|
||
| // const handleSubmit = async (e) => { |
There was a problem hiding this comment.
We can delete any big comment blocks :)
There was a problem hiding this comment.
I'll make another PR tomorrow.
The following changes integrates Pico-PIO-USB to implement a keyboard host interface. With this, a keyboard can be connected via GPIO pins to have its inputs translated via GP2040-CE.
Care must be taken when wiring as incorrectly mapping the GND could cause the keyboard to be borked (happened to one).
Currently, D+ and D- pins are set to GPIO0 and GPIO1 pins. RGB LED also seems to be incompatible with this addon.
This is only one implementation and needs to be expanded to support NKRO, which will be done soon. In the future, several other USB devices could be supported. I think the Pico-PIO-USB integration can be very useful for many use-cases, for instance, it could also be used as an adapter that reserves player slot, given that host support for other controllers are implemented.
Please review the changes and merge if all is good,
Thanks!