-
-
Notifications
You must be signed in to change notification settings - Fork 189
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
JS: Add badusb.altPrint()
and badusb.altPrintln()
to avoid layout issues on Windows targets
#33
Conversation
… and then inputting it using the ALT+numeric keypad sequence
…thout adding a newline at the end
…thout adding a newline at the end
Dear @Willy-JL , @Sil333033 , @HaxSam , and @MatthewKuKanich , I hope you're all doing well. I'm reaching out to kindly request your review on my pull request, #33: "Add numpad keybindings to improve non-US input methods". This contribution aims to enhance our beloved Momentum-Firmware project by introducing Numpad keybindings, a feature I believe many in our community have been looking forward to. Having followed this project for some time, I've grown to admire the robustness and versatility it offers. It's been an inspiration to contribute and I'm excited about the possibility of adding to its functionality. This update, I believe, could make a significant difference for users with non-US input methods, and I'm eager for it to be reviewed by minds as capable as yours. I've taken the liberty of addressing the initial feedback and ensuring that my contributions are in line with our project's standards and objectives. I've thoroughly tested the changes and I'm open to any further suggestions you might have to refine and improve this implementation. Your insights and feedback would be immensely valuable to me and to the project. I understand the demands on your time and truly appreciate any guidance you can provide. Thank you very much for your time and for the incredible work you've done with Momentum-Firmware. Looking forward to possibly hearing from you soon. Warm regards, |
so, a few thoughts while this may work, its not complete under a few aspects. it doesnt respect the same error checking that the exiting badusb.print functions have, like checking for badusb being setup and checking correctly the timeout value to be valid. rather, merging this new functionality into the existing print function (on the backend) would make more sense. this way, the error checking and looping is already done, just need to swap whether its pressing the keys normally, or doing numpad keys. this also does not check if numlock is enabled. altstring only works while numlock is enabled, the badusb app checks for numlock state and enables numlock if needed. please stick to a consistent namin convention, while comments usually are usually good, there are points where its a bit out of place... some things are self explanatory but have multiple redundant comments, while others that would highly benefit from a clarification have nothing. i dont mean to be rude, but this feels AI generated. thats not to say its a problem, but some human intervention and review is always good to have, even more so in languages like C where the AI probably wont be aware of complex memory management (memory wasnt an issue here, but just keep that in mind) there is also no example of this function being used, which would be beneficial since it would be hard to discover its existence without it i'm taking the liberty of addressing these couple points, and will gladly merge this afterwards. thank you! |
badusb.altPrint()
and badusb.altPrintln()
to avoid layout issues on Windows targets
Thanks, @Willy-JL ! I'm deeply thankful for your review and the subsequent merge of my pull request. I genuinely appreciate the effort you've put into enhancing the contribution to meet the high standards of Momentum-Firmware. Initially, I received some help from ChatGPT-4, but I encountered issues since the code didn't work as expected. This challenge led me to explore ducky_script.c for ducky_altchar and ducky_altstring methods, and I made an effort to adhere to the format used in js_badusb.c for consistency. My background in coding is almost entirely at a beginner level in Java and Python, and I've had no formal training in C. Thus, this experience has been a steep learning curve for me. Your insights on the limitations of AI-generated code, especially regarding memory management and the importance of code review, have been incredibly helpful. I'll keep these in mind as I move forward. I must express my admiration for you and your team's work on Momentum-Firmware. Transitioning from xfw to Momentum-Firmware was an easy decision, amazed by the stability and functionality your firmware offers. Your profile, showcasing your projects and achievements at such a young age, is truly inspiring. It's an honor to have my contribution considered and adopted by someone I hold in high regard. Thank you once again for the opportunity to contribute to such an impactful project. The acceptance of my suggestions and the welcoming approach to contributions from individuals with different levels of expertise is very encouraging. I look forward to continuing to learn from this experience and contributing to the future of Momentum-Firmware in any way I can. |
What's new
This PR implements the addition of Numpad keybindings in js_badusb.c, addressing the feature request (#30). The implementation allows users to use ALT+Numpad combinations for input, enhancing functionality for non-US input methods.
Implemented changes include:
For the reviewer
This update is in response to the previously discussed feature request #30. The modifications are intended to improve the firmware's usability for non-US layouts by allowing ALT+Numpad combinations for text input.
Reference to the original feature request for more context:
Please review the changes and consider merging this PR to enhance the project's functionality. Thank you for your consideration.