Embedded software/controls pid refactor/stage#47
Merged
Shengw3n merged 22 commits intoMay 24, 2026
Merged
Conversation
DennisY888
force-pushed
the
embedded-software/controls-pid-refactor/stage
branch
from
b233788 to
6904ba8
Compare
…isibility from tests
…://github.com/UBC-Rocket/thrust_vectoring_consolidated into embedded-software/controls-pid-refactor/stage
lew1101
force-pushed
the
embedded-software/controls-pid-refactor/stage
branch
from
814fa8d to
49d9f57
Compare
lew1101
force-pushed
the
embedded-software/controls-pid-refactor/stage
branch
2 times, most recently
from
5641af8 to
9630d21
Compare
Co-authored-by: J1m1-P
lew1101
force-pushed
the
embedded-software/controls-pid-refactor/stage
branch
from
9630d21 to
2139eab
Compare
J1m1-P
force-pushed
the
embedded-software/controls-pid-refactor/stage
branch
from
c0b7fd4 to
ba3ab2d
Compare
Shengw3n
reviewed
May 24, 2026
| us_f = (float)servo->us_mid + t * (float)(servo->us_max - servo->us_mid); | ||
| } | ||
| // float d = (cfg->reversed ? -degree : degree) + cfg->deg_bias; | ||
| // float d_clamped = clamp_float(degree, cfg->deg_min, cfg->deg_max); |
Member
There was a problem hiding this comment.
reversed in servo_config_t is never applied
is it better to apply clamping to the mapping instead of the gimbal api?
Comment on lines
+72
to
+73
| * @param x_deg Desired X-axis (roll) gimbal angle in degrees, clamped to ±SERVO_GIMBAL_HALF_RANGE_DEG. | ||
| * @param y_deg Desired Y-axis (pitch) gimbal angle in degrees, clamped to ±SERVO_GIMBAL_HALF_RANGE_DEG. |
Member
There was a problem hiding this comment.
SERVO_GIMBAL_HALF_RANGE_DEG this does not exist
Comment on lines
+249
to
+250
| float calibrated_theta_x_deg = (1/sqrtf(2.0f)) * theta_x_deg + (1/sqrtf(2.0f)) * theta_y_deg; // Calibrated to match physical gimbal direction | ||
| float calibrated_theta_y_deg = -(1/sqrtf(2.0f)) * theta_x_deg + (1/sqrtf(2.0f)) * theta_y_deg; // Calibrated to match physical gimbal direction |
Member
There was a problem hiding this comment.
you are computing sqrtf(2) every 800 Hz here
Precompute const float k = 0.70710678f
Member
There was a problem hiding this comment.
osDelay in inside a 800 Hz controls task is questionable
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.