[Keyboard] Add support for the Anne Pro keyboard by Obins (2nd PR)
Please see https://github.com/qmk/qmk_firmware/pull/9684 for original PR. I am resubmitting as I'm not sure whether @msvisser is still using or interested in maintaining this and I would like to get feedback on what additional changes are needed to bring this into qmk.
Description
This pull request adds support for the Anne Pro keyboard to QMK. This keyboard is produced by a Chinese company called Obins and was available for an affordable price. The original firmware supplied with this keyboard was okay at best, and often worked pretty poorly, therefore the community decided to reverse engineer the keyboard. As a result of this effort this QMK port was created.
These changes add complete support for all of the features of the keyboard, that includes both the LED backlight and the Bluetooth functionality. As mentioned in the README file for the keyboard there are currently two small known problems, which are currently considered won't fix.
On top of msvisser's work, I have brought this up to use the new info.json format, ~~fixed the broken EEPROM~~ (done in https://github.com/qmk/qmk_firmware/pull/24928), and moved from using our own custom board to using the L152 already available in ChibiOS.
~~In regards to the EEPROM driver changes, I'm interested if there is a better way to do this, and if there is a way for us to only use word-aligned writes for the lower-capability Cat.1 devices in the lineup while using the less invasive byte writes for all of the other devices. I don't see a current path to fix this as this code is already masquerading as a Cat.5 device.~~
Types of Changes
- [ ] Core
- [ ] Bugfix
- [ ] New feature
- [ ] Enhancement/optimization
- [x] Keyboard (addition or update)
- [x] Keymap/layout (addition or update)
- [ ] Documentation
Checklist
- [x] My code follows the code style of this project: C, Python
- [x] I have read the PR Checklist document and have made the appropriate changes.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have read the CONTRIBUTING document.
- [ ] I have added tests to cover my changes.
- [x] I have tested the changes and verified that they work and don't break anything (as well as I can manage).
Re: https://github.com/qmk/qmk_firmware/pull/9684#pullrequestreview-458814412:
I've added some comments specifically to do with minimising the number of required things with ChibiOS. These change things to pretend to be an L152RE -- as far as is my understanding L152 code runs perfectly fine on L151, as the only real difference is a lack of LCD controller. (I've already tested basic QMK functionality on an L151 masquerading as an L152, and it's been fine.)
Please don't blindly commit these changes -- test them first to make sure it's still functional.
I was able to integrate the changes with one small tweak needed to use a lower-end device in config.h.
I also have queries regarding the bootloader -- clearly there's an onboard bootloader present, as it's relocating QMK to 0x08004000. Do you know why this is the case? L151 already has an onboard USB DFU, so I'm intrigued as to what extra functionality the bootloader possesses, and whether or not it's even required if using USB DFU.
I may be wrong, but the specific board being used here (STM32L151C8) does not have DFU bootloader, see https://community.st.com/ysqtg83639/attachments/ysqtg83639/stm32-mcu-products-forum/28746/1/cd00167594-stm32-microcontroller-system-memory-boot-mode-stmicroelectronics.pdf Table 115.
There's likely going to be some significant restructuring required relating to the keyboard code after all of this low-level stuff settles. Re-implementing systems like animations are somewhat frowned upon, given that those subsystems exist in base QMK in general and we don't have ridiculous amounts of resources to throw at maintaining competing implementations. If there's a need to match existing functionality I believe the current stance is to do that within keymaps while keeping the default keyboard and keymap "pristine", with the bare minimum to get QMK up and running. Obviously, bluetooth support and the like are inherent in the keyboard itself, so they don't need to be punted into a keymap.
I think there's a misunderstanding of what the lighting code is for. It is not reimplementing custom lighting animations, it's just implementing the comms needed to talk to the onboard lighting chip with it's proprietary firmware and built-in animations.
I may be wrong, but the specific board being used here (STM32L151C8) does not have DFU bootloader, see community.st.com/ysqtg83639/attachments/ysqtg83639/stm32-mcu-products-forum/28746/1/cd00167594-stm32-microcontroller-system-memory-boot-mode-stmicroelectronics.pdf Table 115.
No, you're correct, AN2606 states that any of the L15x's with less than 256kB flash has no builtin USB DFU bootloader. I believe this was likely an oversight of mine in the original PR, given the references to the normal STM32 inbuilt DFU definitions in the original rules.mk.
I think there's a misunderstanding of what the lighting code is for. It is not reimplementing custom lighting animations, it's just implementing the comms needed to talk to the onboard lighting chip with it's proprietary firmware and built-in animations.
Yep. Given this doesn't have a separate lighting firmware available to support QMK's rgb_matrix (like the AP2 does) it will probably be fine. Will let this PR filter through the normal process.
Forgot to mention; the core bugfix for the eeprom should go to develop as a separate PR. We generally want functionality to be isolated merges.
I updated this PR to use transient eeprom instead of the vendor, pending the EEPROM fix arriving in master. So review can continue on this PR now.
Can the labels for this PR be updated? I have cleaned up the code and there is no longer any python code or core code in this PR.
Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.
This PR is still ready for review. No big changes that I foresee it needing but open to input.
Pushed a commit to enable EEPROM as driver fix is now in master.
Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.
Bump. Been daily-driving this for a while now.
Checking in, is there any changes needed here? Been using this for a while now. Noticed that there's a pr_checklist_pending label on this, is there something I missed?