refactor(OpenGloves): decouple from Arduino SDK #100
Conversation
Apply Sweep Rules to your PR?
This is an automated message generated by Sweep AI. |
|
@CodiumAI-Agent /review |
PR Review
Code feedback:
✨ Review tool usage guide:Overview: With a configuration file, use the following template:
See the review usage page for a comprehensive guide on using this tool. |
PR Analysis
PR Feedback💡 General suggestions: The PR is well-structured and the changes are logically grouped. However, it would be beneficial to add some comments explaining the purpose and usage of the newly introduced macros and methods. This would make the code easier to understand for other developers. Additionally, it would be good to add tests to verify the functionality of the new code. 🤖 Code feedback:
✨ Usage guide:Overview: With a configuration file, use the following template:
See the review usage page for a comprehensive guide on using this tool. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #100 +/- ##
===========================================
+ Coverage 43.61% 44.75% +1.13%
===========================================
Files 42 42
Lines 1229 1200 -29
===========================================
+ Hits 536 537 +1
+ Misses 693 663 -30
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
@CodiumAI-Agent /describe --pr_description.add_original_user_description=true --pr_description.keep_original_user_title=true |
|
@CodiumAI-Agent /review -i --pr_reviewer.remove_previous_review_comment=true |
|
@CodiumAI-Agent /improve |
PR Code Suggestions
✨ Improve tool usage guide:Overview: With a configuration file, use the following template:
See the improve usage page for a more comprehensive guide on using this tool. |
PR Code Suggestions💡 [maintainability] Consider using a macro to reduce the repetition of the button enabled checks. File: lib/opengloves/senseshift/opengloves/autoconfig.hpp (147-151) Example code:Existing code: #ifdef PIN_BUTTON_A
#define BUTTON_A_ENABLED (PIN_BUTTON_A != -1)
#else
#define BUTTON_A_ENABLED false
#endifImproved code: #define CHECK_BUTTON_ENABLED(BUTTON) \
#ifdef PIN_##BUTTON \
#define BUTTON_##BUTTON##_ENABLED (PIN_##BUTTON != -1) \
#else \
#define BUTTON_##BUTTON##_ENABLED false \
#endif
CHECK_BUTTON_ENABLED(BUTTON_A)
CHECK_BUTTON_ENABLED(BUTTON_B)
CHECK_BUTTON_ENABLED(BUTTON_MENU)
CHECK_BUTTON_ENABLED(BUTTON_JOYSTICK)
CHECK_BUTTON_ENABLED(BUTTON_CALIBRATE)
CHECK_BUTTON_ENABLED(BUTTON_TRIGGER)
CHECK_BUTTON_ENABLED(BUTTON_GRAB)
CHECK_BUTTON_ENABLED(BUTTON_PINCH)💡 [best practice] Consider removing commented out code if it is not needed. File: lib/opengloves/senseshift/opengloves/opengloves_component.hpp (18-19) Example code:Existing code: // Plotter raw_plotter_ = Plotter(&Serial, "Raw");
// Plotter calibrated_plotter_ = Plotter(&Serial, "Cal");Improved code: 💡 [best practice] Consider using a for-each loop to iterate over File: lib/opengloves/senseshift/opengloves/opengloves_component.hpp (17-26) Example code:Existing code: const auto& curls = this->curl.fingers;
const auto& splays = this->splay.fingers;
for (auto i = 0; i < curls.size(); i++) {
const auto& finger_curl = curls[i].curl;
for (auto j = 0; j < finger_curl.size(); j++) {
auto* joint_sensor = finger_curl[j];
if (joint_sensor != nullptr) {
data.curl.fingers[i].curl[j] = joint_sensor->FN();
}
}Improved code: const auto& curls = this->curl.fingers;
const auto& splays = this->splay.fingers;
for (const auto& finger_curl : curls) {
for (auto* joint_sensor : finger_curl.curl) {
if (joint_sensor != nullptr) {
data.curl.fingers[i].curl[j] = joint_sensor->FN();
}
}💡 [best practice] Consider making File: lib/opengloves_arduino/senseshift/opengloves/opengloves_plotter.hpp (34-35) Example code:Existing code: Print* output_;
char* prefix_;Improved code: Print* const output_;
char* const prefix_; |
5432c49 to
2040226
Compare
No description provided.