-
Notifications
You must be signed in to change notification settings - Fork 54
[spinal][control] support multiple type of thrust motors #726
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
base: master
Are you sure you want to change the base?
[spinal][control] support multiple type of thrust motors #726
Conversation
…est for spine system
… and motor_type array to identify type of motor when using multiple motors
usageMotorInfo.yaml example
|
… support multiple motors
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.
Pull Request Overview
This PR adds support for multiple types of thrust motors in the spinal control system. The change enables using different motor types for thrust rather than being limited to a single motor type.
- Adds new PwmInfos message containing motor type arrays and motor_infos field for different motor types
- Updates URDF files to use effort value from joint limits for motor-specific m_f_rate values
- Implements per-motor PWM conversion, saturation checking, and thrust limits in attitude control
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| aerial_robot_nerve/spinal/msg/PwmInfos.msg | New message type defining motor types array and PWM info array |
| aerial_robot_nerve/spinal/mcu_project/lib/Jsk_Lib/flight_control/attitude/attitude_control.h | Updated to handle multiple motor types with per-motor arrays |
| aerial_robot_nerve/spinal/mcu_project/lib/Jsk_Lib/flight_control/attitude/attitude_control.cpp | Implements per-motor PWM conversion and saturation logic |
| aerial_robot_control/include/aerial_robot_control/control/base/base.h | Updates control interface to support multiple motor types |
| aerial_robot_model/include/aerial_robot_model/model/aerial_robot_model.h | Changes thrust limits and m_f_rate to per-motor arrays |
| aerial_robot_model/src/model/base_model/robot_model.cpp | Implements per-motor property extraction from URDF |
| robots//urdf/.urdf.xacro | Sets effort limit to 0 to enable motor-specific m_f_rate from URDF |
| aerial_robot_simulation/include/aerial_robot_simulation/rotor_handle.h | Uses effort value from URDF for motor-specific m_f_rate |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| float yaw_decreasing_rate_i = residual_term / (fabs(yaw_term_[i]) / rotor_devider_); | ||
| if(yaw_decreasing_rate_i < yaw_decreasing_rate) yaw_decreasing_rate = yaw_decreasing_rate_i; |
Copilot
AI
Sep 23, 2025
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.
Potential division by zero when yaw_term_[i] is zero. This could occur when yaw control is not active or when the yaw term is exactly zero, leading to undefined behavior.
| float yaw_decreasing_rate_i = residual_term / (fabs(yaw_term_[i]) / rotor_devider_); | |
| if(yaw_decreasing_rate_i < yaw_decreasing_rate) yaw_decreasing_rate = yaw_decreasing_rate_i; | |
| if (fabs(yaw_term_[i]) > 1e-6) // prevent division by zero | |
| { | |
| float yaw_decreasing_rate_i = residual_term / (fabs(yaw_term_[i]) / rotor_devider_); | |
| if(yaw_decreasing_rate_i < yaw_decreasing_rate) yaw_decreasing_rate = yaw_decreasing_rate_i; | |
| } | |
| // else: skip update to yaw_decreasing_rate to avoid division by zero |
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
|
I found that generated message for nested array is incorrectly deserialized and this implementation did not work for real machine. |
This problem has been solved by sugikazu75/rosserial@eb8c359 |
What is this
enable to use different type motors for thrust.
Details
No change is needed if we use single type of motor.
TODO