kvmd
kvmd copied to clipboard
pikvm
Build HID using STM32F103. WIP.
Please let me know if you could consider to merge it. What structure would you like to see? I would like to remove some if if/defs.
@mdevaev I added a few more commits. It is still WIP. From that point it should be easier to add new implementation. I added interface for usbmouse and keyboard. Instance is created by factory. If possible please take a look I would like to make sure that you agree with the direction.
A cursory review reveals violations of the protocol. You have incorrectly implemented keyboard and mouse type selection. The protocol allows them to be disabled, and in your case there is a fallback to the default type.
I don't like the approach where all changes are dumped as one giant patch that can no longer be reliably reviewed. As long as you did STM32 and changed the logic a little, it was still ok, but if you change the code structure and logic a lot, then these should be separate patches.
Plus, the approach you have chosen breaks the abstraction, when earlier all protocol logic was described only in main.cpp.
However, I understand the need for all these changes, so I suggest starting gradually. As a first step, I suggest not a factory, but several separate classes, and for the mouse win98fix should be implemented as a separate class. Absolute and relative mice can have both sendMove() and sendRelative() methods implemented, just one of them will be empty. This will greatly simplify the code in main.cpp and will allow you to move on to make STM32. That is, at the first stages it should only be refactoring without changing or adding logic.
A cursory review reveals violations of the protocol. You have incorrectly implemented keyboard and mouse type selection. The protocol allows them to be disabled, and in your case there is a fallback to the default type.
default means disabled. It that way code can be cleaner since I don't have to check it each time.
if (_kbd->getType()) { //<- by default it will be false
Plus, the approach you have chosen breaks the abstraction, when earlier all protocol logic was described only in main.cpp. I noticed that you wanted to separate driver from protocol. I just tried to make interface simpler do if someone adds new driver in the future they can easily understand what method does.
I also prefer approach with smaller changes. This is more like design so we both can understand how the final product will look like. I will break it and make a few separate changes.
I did more testing and sometimes it become unstable. There are 2 issues:
- USB does not enumerate correctly. It can be related with way how D+ is handle. Maybe bigger timeout?
- Sometimes USB gets stuck here https://github.com/arpruss/USBComposite_stm32f1/blob/3c58f97eb006ee9cd1fb4fd55ac4faeeaead0974/usb_hid.c#L376. Should transmitting be init with -1 ?
I did more testing and sometimes it become unstable. There are 2 issues:
1. USB does not enumerate correctly. It can be related with way how D+ is handle. Maybe bigger timeout? 2. Sometimes USB gets stuck here https://github.com/arpruss/USBComposite_stm32f1/blob/3c58f97eb006ee9cd1fb4fd55ac4faeeaead0974/usb_hid.c#L376. Should transmitting be init with -1 ?
Those looks to be HW issue. I changed the board and it works without any problem.
Hi everyone, thanks for your work on port to stm32 bluepill I am also working on porting to another platforms
- esp32
- rp2040
- blackpill
There aare platform-avr platform-stm32. I think it is possible to merge those into one file by carefully designing the envs. Are you open to this approach? If not I will just create another platform-esp32, etc..
We made separate inis because different firmware may have a different feature set and are too complex in terms of dependencies. I prefer not to mix one with the other.
The status is as follow https://github.com/pikvm/kvmd/blob/master/hid/lib/drivers-stm32/README.md. I may fix boot keyboard though.