-
Notifications
You must be signed in to change notification settings - Fork 0
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
added: radians cmd #17
Conversation
WalkthroughThis update introduces a new constant for monitoring delay, enhances angle handling by supporting both radians and degrees, and fine-tunes motor operations. It also revises setup procedures for essential components and modifies serial command processing for target settings, incorporating a delay mechanism within the main loop for improved monitoring. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/main.cpp (6 hunks)
Additional comments: 6
src/main.cpp (6)
- 19-21: The introduction of a
MONITOR_DELAY_MS
constant and amonitor_delay
variable is a good practice for making the monitoring delay configurable. However, it's important to ensure that the initial value ofmonitor_delay
is set appropriately in the setup to avoid any unintended delays before the first monitoring occurs.- 27-28: The implementation of
doDegrees
function correctly converts degrees to radians before setting the target. This is a crucial step for maintaining consistency in how angles are processed internally. It's a good practice to encapsulate such conversions within the function to keep the rest of the codebase simple and focused on logic rather than unit conversions.- 93-95: Adjusting motor limits (
voltage_limit
,velocity_limit
, andcurrent_limit
) is crucial for ensuring the motor operates within safe and efficient parameters. These settings should be carefully chosen based on the motor's specifications and the application's requirements. It's also important to document the rationale behind these values for future reference and adjustments.- 99-99: The use of
monitor_downsample
to control the frequency of monitoring outputs is a good practice for balancing the need for monitoring with the desire to minimize performance impacts. It's important to choose a value that provides sufficient visibility into the motor's operation without overly burdening the system.- 117-118: The addition of
doRadians
anddoDegrees
commands to the commander is a significant enhancement, allowing users to set the target angle in their preferred unit of measurement. This improves the software's usability and flexibility. It's important to ensure that the help text ("target [rad]" and "target [deg]") clearly communicates the expected input format to the users.- 139-144: The implementation of monitoring delay logic in the loop is a good practice for controlling the frequency of potentially performance-impacting operations like monitoring. This approach allows for real-time monitoring without significantly slowing down the execution. It's important to ensure that this logic does not inadvertently introduce any timing issues or delays in critical operations.
@@ -51,7 +55,8 @@ void setup() { | |||
// initialize the driver | |||
driver.voltage_power_supply = 24.0f; | |||
rc = driver.init(); | |||
checkRC(rc); SIMPLEFOC_DEBUG("Driver ready."); | |||
checkRC(rc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The call to checkRC(rc)
after initializing the driver is a good practice for error handling. It ensures that any issues with the driver initialization are caught early. However, it would be beneficial to add comments explaining the expected range of return codes from driver.init()
and what conditions might lead to a failure, for better maintainability and clarity for future developers.
Consider adding comments detailing the expected return codes from driver.init()
and the conditions that might lead to initialization failure.
@@ -60,7 +65,8 @@ | |||
// initialize the current sensor | |||
current_sensor.skip_align = true; | |||
rc = current_sensor.init(); | |||
checkRC(rc); SIMPLEFOC_DEBUG("Current sensor ready."); | |||
checkRC(rc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the previous comment on driver.init()
, the use of checkRC(rc)
after initializing the current sensor is a good error handling practice. Including comments about the expected return codes from current_sensor.init()
would enhance code readability and maintainability.
It would be helpful to include comments explaining the expected return codes from current_sensor.init()
and under what conditions initialization might fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/main.cpp (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/main.cpp
Summary by CodeRabbit