Am/m3/pid2 #46
Am/m3/pid2 #46
Conversation
There was a problem hiding this comment.
Looks like it needs a good rebase, and is waiting on some adjacent structs.
I had a few things that I may not have explained super clearly, but I tried my best in the comments. Let me know and just ping me if you're confused about what I said.
I also noticed some weird formatting. I'd suggest running a clang format using warg's template or your own preffered template. It'll get rid of some weird whitespace / newline shenanigans that seem to be popping up here and there.
Overall, this looks super awesome!!
| // _pids.pitch.execute(mappedOutputs.pitch, actual, actualRate); | ||
| // _pids.roll.execute(mappedOutputs.roll, actual, actualRate); | ||
| // _pids.yaw.execute(mappedOutputs.yaw, actual, actualRate); | ||
| // _pids.throttle.execute(mappedOutputs.throttle, actual, actualRate); |
| //Need to get actual and actualRate data from IMU filter | ||
| // _pids.pitch.execute(mappedOutputs.pitch, actual, actualRate); | ||
| // _pids.roll.execute(mappedOutputs.roll, actual, actualRate); | ||
| // _pids.yaw.execute(mappedOutputs.yaw, actual, actualRate); |
There was a problem hiding this comment.
yaw is unique, it operates on yawrate! So you'll need to give it your rate, and then acceleration!
| break; | ||
| case derivative: | ||
| _axis.setKd(desiredGain); | ||
| break; |
There was a problem hiding this comment.
Remember to have a default case which throws some sort of error/ warning!
Also remember to do bounds checking on your whichGain :)
AttitudeManager/Src/AM.cpp
Outdated
| @@ -19,6 +19,19 @@ AttitudeManagerInput AttitudeManager::control_inputs = { | |||
| .throttle = 0.0f | |||
| }; | |||
|
|
|||
| AxisPIDs axisPIDs = { | |||
There was a problem hiding this comment.
You should be able to declare these in the AM::fbwa class (same ideas as prev. comments)
You shouldn't have them in the main AM.cpp at all, since there will be more flight modes in the future which will also have varying different PID's.
AttitudeManager/Src/AM.cpp
Outdated
| @@ -49,7 +62,7 @@ void AttitudeManager::runControlLoopIteration() { | |||
| AttitudeManagerInput control_inputs = getControlInputs(); | |||
|
|
|||
| // Run Control Algorithms | |||
| AttitudeManagerInput motor_outputs = controlAlgorithm_->run(control_inputs); | |||
| AttitudeManagerInput motor_outputs = controlAlgorithm_->run(control_inputs, axisPIDs); | |||
There was a problem hiding this comment.
above comment(s) - PID owned by class and we shouldn't need to pass them.
TLDR: the function that calls <flightmode>::run doesn't know or care what PID's exist and how they need to be managed. It just expects the flightmode to update and give motor outputs when run is called.
AttitudeManager/Src/PID.cpp
Outdated
| } | ||
|
|
||
| float ret = constrain<float>((pid.kp * error) + (pid.ki * integral) - (pid.kd * derivative), | ||
| pid.max_output, pid.min_output); | ||
| float ret = constrain<float>((pid.kp * error) + (pid.ki * integral) - (derivative), |
| int main(int argc, char** argv) { | ||
| ::testing::InitGoogleTest(&argc, argv); | ||
| return RUN_ALL_TESTS(); | ||
| } |
There was a problem hiding this comment.
newline @ eof
| } | |
| } | |
AttitudeManager/Src/PID.cpp
Outdated
|
|
||
| float PIDController::map(float x, float in_min, float in_max, float out_min, float out_max) { | ||
| return (x - in_min) * (out_max - out_min) / (in_max - in_min) + out_min; | ||
| } |
There was a problem hiding this comment.
newline @ eof
| } | |
| } | |
csabahi
requested review from
twilkhoo,
Aidan-B,
Penguronik and
ayoungeun
and removed request for
antimonyB
Co-authored-by: Anthony Luo <[email protected]>
Description
What was completed, changed, or updated?
NOT ready to merge yet, waiting for IMU filters and driver to be completed
Designed PIDs for ZP3.5
Implemented it in Manual and FBWA flightmodes
Why was this done (if applicable)?
Because WARG need PID
Testing
What manual tests were used to validate the code?
What unit tests were used to validate the code?
Wrote unit tests in PIDTests.cpp
Documentation
Milestone number and name:
M3
Link to Asana task:
https://app.asana.com/0/1203458353737758/1206541298938863/f
Link to Confluence documentation:
https://uwarg-docs.atlassian.net/wiki/x/AQCtjw
Reminders
Add reviewers to the PR
Mention the PR in the appropriate discord channel