feat(motor): using open_loop_velocity example #6
Conversation
WalkthroughThe recent code overhaul transitions from utilizing a hall sensor for motor control to implementing an open-loop control system for a BLDC motor. This shift entails removing the hall sensor configurations and interrupt routines, and instead, focusing on setting up the BLDC motor and its driver, establishing voltage and current limits, and initializing open-loop control. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/main.cpp (3 hunks)
Additional comments: 3
src/main.cpp (3)
- 6-6: Ensure the pin definitions for
A_PHASE_UH,A_PHASE_UL,A_PHASE_VH,A_PHASE_VL,A_PHASE_WH, andA_PHASE_WLare defined elsewhere in the code or documentation, as they are critical for the motor driver initialization but not visible in the provided snippet.- 1-12: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [33-33]
Setting
driver.voltage_limitafterdriver.init()might not have the intended effect ifvoltage_limitis only read duringinit(). Verify that this limit is correctly applied post-initialization or consider setting it before callinginit().
- 45-45: The addition of commands for setting target velocity and voltage limit is a good practice for dynamic control. However, ensure that input validation is performed within
doTargetanddoLimitfunctions to prevent setting values that could potentially damage the motor or the driver.
| #include <SimpleFOC.h> | ||
|
|
||
| #define POLE_PAIRS 7 | ||
|
|
||
| // Motor instance | ||
| // BLDC motor & driver instance |
There was a problem hiding this comment.
The instantiation of BLDCMotor and BLDCDriver6PWM directly in the global scope can lead to issues with initialization order, especially when dealing with hardware components. Consider moving these into the setup() function or ensuring their initialization is safe at the point of construction.
| // Open loop motor control example | ||
| #include <SimpleFOC.h> | ||
|
|
||
| #define POLE_PAIRS 7 | ||
|
|
||
| // Motor instance | ||
| // BLDC motor & driver instance | ||
| BLDCMotor motor = BLDCMotor(POLE_PAIRS); | ||
| BLDCDriver6PWM driver = BLDCDriver6PWM(A_PHASE_UH, A_PHASE_UL, A_PHASE_VH, A_PHASE_VL, A_PHASE_WH, A_PHASE_WL); | ||
|
|
||
| // Hall sensor instance | ||
| // HallSensor(int hallA, int hallB , int hallC, int pp) | ||
| // - hallA, hallB, hallC - HallSensor A, B and C pins | ||
| // - pp - pole pairs | ||
| HallSensor sensor = HallSensor(A_HALL1, A_HALL2, A_HALL3, POLE_PAIRS); | ||
|
|
||
| // Interrupt routine initialisation | ||
| // channel A, B and C callbacks | ||
| void doA(){sensor.handleA();} | ||
| void doB(){sensor.handleB();} | ||
| void doC(){sensor.handleC();} | ||
|
|
||
| //target variable | ||
| float target_velocity = 0; | ||
|
|
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [25-25]
The delay of 2000 milliseconds in setup() might not be necessary or could be reduced, depending on the hardware initialization requirements. Evaluate if this delay can be shortened to speed up the startup process.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [55-55]
In the loop() function, there's an implicit assumption that target_velocity will be updated through the command interface. Ensure there's error handling or default behavior in case target_velocity is not set or receives invalid input.
| @@ -62,13 +35,14 @@ void setup() { | |||
| // start very low for high resistance motors | |||
| // current = voltage / resistance, so try to be well under 1Amp | |||
| motor.voltage_limit = 3; // [V] | |||
| motor.velocity_limit = 10; // [rad/s] | |||
|
|
|||
| // open loop control config | |||
There was a problem hiding this comment.
The comment about starting very low for high resistance motors and trying to be well under 1Amp is useful, but ensure there's a mechanism or recommendation for users to adjust motor.voltage_limit based on their specific motor's resistance. This could be in the form of documentation or a calculation example in the comments.
| motor.init(); | ||
| // use monitoring with serial | ||
| motor.useMonitoring(Serial); |
There was a problem hiding this comment.
Using motor.useMonitoring(Serial) enables monitoring over serial, which is great for debugging but consider the performance implications in a production environment. If the serial output is not needed in production, it might be beneficial to conditionally enable this feature.
Summary by CodeRabbit