Armdevel: Create PID Library #231

younesr1 · 2020-12-31T23:44:04Z

This lib was tested with test-pid. I apologise to anyone following the git workflow of this feature it has been rough. This feature is finally ready for review

wmmc88

should write a test for threadsafety.

also what are the accumulated errors and average error compared to the matlab?

apps/test-pid/CMakeLists.txt

apps/test-pid/include/control.h

apps/test-pid/include/feedback.h

lib/pid/include/PID.h

lib/pid/src/PID.cpp

lib/pid/include/PID.h

lib/pid/src/PID.cpp

Co-authored-by: Melvin Wang <[email protected]>

…er2020-firmware into younes/pidforarmdevel

apps/test-pid/CMakeLists.txt

Co-authored-by: Melvin Wang <[email protected]>

younesr1 · 2021-01-02T01:27:32Z

should write a test for threadsafety.

also what are the accumulated errors and average error compared to the matlab?

I'll rerun the app sometime this weekend and let you know

wmmc88 · 2021-01-02T17:43:27Z

should write a test for threadsafety.
also what are the accumulated errors and average error compared to the matlab?

I'll rerun the app sometime this weekend and let you know

maybe have the expected value saved in the test and have an assert that checks against it. almost like a unit test haha.

…er2020-firmware into younes/pidforarmdevel

younesr1 · 2021-01-03T20:15:13Z

I didnt add a test for threadability. the work needed to create any sort of meaningful test is too much to be worth it. i think we're safe skipping threadability test since it we lock a mutex with lock guard at the top of every function basically

wmmc88

couple comments from the old review weren't addressed and got auto dismissed because of changes

apps/test-pid/src/main.cpp

wmmc88 · 2021-01-04T00:57:58Z

lib/pid/src/PID.cpp


-  realOutput_ = 0.0;
+void PID::Pid::updateProportionalGain(uint32_t p) {
+  std::lock_guard<Mutex> lock(m_mutex);


wmmc88 · 2021-01-04T01:09:41Z

lib/pid/src/PID.cpp

+  float paths = computePPath(error);
+  paths += computeIPath(error, dt);
+  paths += m_antiKickback ? computeDPathOnPV(error, dt) : computeDPathOnError(processVariable, dt);
+  paths = std::clamp(paths, static_cast<float>(m_lowerBound), static_cast<float>(m_upperBound));


lib/pid/src/PID.cpp

wmmc88 · 2021-01-04T01:48:32Z

lib/pid/src/PID.cpp

-void PID::setDeadZoneError(float error) {
-  deadZoneError_ = error;
+void PID::PID::reset() {
+  std::lock_guard<Mutex> lock(m_mutex);


bump on the scoped lock comment

Look at my previous comment about lockguard vs scoped lock

I dont see any comment abt scoped lock (edit: nvm i found it)

cindyli-13 · 2021-01-04T03:48:36Z

Do we need feed-forward support or ability to accept tuning params on the fly? Or will those be future issues

lib/pid/include/PID.h

younesr1 · 2021-01-04T04:05:08Z

Do we need feed-forward support or ability to accept tuning params on the fly? Or will those be future issues

this is just a pid lib feed fwd is part of the control loop which is at a higher level. this includes on the fly tuning. see comment at the top of the header

wmmc88 · 2021-01-04T15:28:52Z

Do we need feed-forward support or ability to accept tuning params on the fly? Or will those be future issues

this is just a pid lib feed fwd is part of the control loop which is at a higher level. this includes on the fly tuning. see comment at the top of the header

something i just thought about re: feedforward is that by adding the feedforward outside of PID, we may run into issues with:

going over the saturation limit
pid error being wrong because its using a setpoint that isnt the true setpoint (that includes the feedforward term)

younesr1 · 2021-01-05T03:33:35Z

This PR is currently on hold. The average error is very large atm, there must be a bug in the code

younesr1 · 2021-01-12T01:06:02Z

This PR is no longer on hold

wmmc88

lgtm. pls update the other branch and tag it or smth

* Move Encoder class and create new sensor class * Move files to better folders * Modify Moisture sensor to use new Sensor Base * Edit cmake file for sensor folder * Update Moisture Sensor files and test app * Modify make files to build moved files * Changes to 'READ()' after PR * modify sensor read functions * Add actuator base class and derived classes (#171) * Add actuator base class and derived classes * Actuators lib: tweak actuator classes, delete old actuator libraries * Actuators lib: add copy constructor * Actuators lib: apply feedback comments * Actuators lib: apply feedback comments again * Actuators lib: Wrap classes in namespaces * Actuators lib: fix compile errors * Actuators lib: fix comments, rename variables, and other small tweaks * Actuators lib: fix comments and variable names again * Actuators lib: add std::abs guards * Armdevel: Create PID Library (#231) * created pid lib and fixed easily fixable errors * Update apps/test-pid/CMakeLists.txt Co-authored-by: Melvin Wang <[email protected]> * implemented melvin comments * Update apps/test-pid/CMakeLists.txt Co-authored-by: Melvin Wang <[email protected]> * implemented melvin comments * formatting * added mention of matlab code * use scoped lock. use better anti windup. * use new values * avg error is now 37 * fix formatting * debug commit (not final commit) * ready for review Co-authored-by: yreda1999 <[email protected]> Co-authored-by: Melvin Wang <[email protected]> * Younes: simplified interface, added namespaces, add config structs * Further updates after PR * Fixed and updated Moisture sensor and app * update relative encoder files * Update absolute encoder files * fix error with pwm in * initial commit * arm main compiles! * gimbtonomy also compiles * ready for reviews * fix silly mistake * change protected fields of final classes to private * forgot one file * added comments with instructions for felix and orson * implemented feedback and fixed failing test apps * changed pointers to refs where appropriate * fixed melvin anc cindy comments * remove current sensors from arm config headers since we wont have em ready for SAR * fix formatting * fix formatting angain * fixed test can app * Fw/younes actuator (#280) * Update science pins * Update pin names, update science apps * Updated for clang format * Revert "Updated for clang format" This reverts commit f3831a1. * Proper clang formatting * PR comments * Updating PR for changes on base * update servo range of motion * Missed a file for clang * Fixed code which was mistakenly changed Co-authored-by: Felix Wong <[email protected]> * fix formatting * fixed check not running * fixed comments * updated version of bridge Co-authored-by: Felix Wong <[email protected]> Co-authored-by: Cindy Li <[email protected]> Co-authored-by: yreda1999 <[email protected]> Co-authored-by: Melvin Wang <[email protected]> Co-authored-by: younes <[email protected]> Co-authored-by: FK3wong <[email protected]> Co-authored-by: Felix Wong <[email protected]>

created pid lib and fixed easily fixable errors

14dafcc

younesr1 requested review from wmmc88 and cindyli-13 December 31, 2020 23:45

wmmc88 requested changes Jan 1, 2021

View reviewed changes

younesr1 and others added 3 commits January 1, 2021 19:28

Update apps/test-pid/CMakeLists.txt

5df5f9f

Co-authored-by: Melvin Wang <[email protected]>

implemented melvin comments

0813d32

Merge branch 'younes/pidforarmdevel' of github.com:uwrobotics/MarsRov…

5366b11

…er2020-firmware into younes/pidforarmdevel

wmmc88 reviewed Jan 2, 2021

View reviewed changes

apps/test-pid/CMakeLists.txt Outdated Show resolved Hide resolved

Update apps/test-pid/CMakeLists.txt

c2dd626

Co-authored-by: Melvin Wang <[email protected]>

wmmc88 mentioned this pull request Jan 3, 2021

Implement Logger #141

Closed

younesr1 added 2 commits January 3, 2021 15:13

implemented melvin comments

e0a3838

Merge branch 'younes/pidforarmdevel' of github.com:uwrobotics/MarsRov…

3771900

…er2020-firmware into younes/pidforarmdevel

formatting

a869c7f

younesr1 requested a review from wmmc88 January 3, 2021 22:11

added mention of matlab code

615bdd0

wmmc88 requested changes Jan 4, 2021

View reviewed changes

cindyli-13 reviewed Jan 4, 2021

View reviewed changes

lib/pid/include/PID.h Outdated Show resolved Hide resolved

use scoped lock. use better anti windup.

6e4b7d7

younesr1 added 5 commits January 6, 2021 21:22

use new values

148b4f6

avg error is now 37

4659475

fix formatting

2fdd584

debug commit (not final commit)

0cdd8a0

ready for review

e401d0e

Merge branch 'armdevel' into younes/pidforarmdevel

585ae3e

wmmc88 approved these changes Jan 12, 2021

View reviewed changes

younesr1 merged commit 1425a5a into armdevel Jan 12, 2021

younesr1 deleted the younes/pidforarmdevel branch January 12, 2021 02:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Armdevel: Create PID Library #231

Armdevel: Create PID Library #231

younesr1 commented Dec 31, 2020

wmmc88 left a comment

younesr1 commented Jan 2, 2021

wmmc88 commented Jan 2, 2021

younesr1 commented Jan 3, 2021

wmmc88 left a comment

wmmc88 Jan 4, 2021

wmmc88 Jan 4, 2021

wmmc88 Jan 4, 2021

younesr1 Jan 4, 2021

wmmc88 Jan 4, 2021

younesr1 Jan 4, 2021 •

edited

Loading

cindyli-13 commented Jan 4, 2021

younesr1 commented Jan 4, 2021

wmmc88 commented Jan 4, 2021

younesr1 commented Jan 5, 2021

younesr1 commented Jan 12, 2021

wmmc88 left a comment

Armdevel: Create PID Library #231

Armdevel: Create PID Library #231

Conversation

younesr1 commented Dec 31, 2020

wmmc88 left a comment

Choose a reason for hiding this comment

younesr1 commented Jan 2, 2021

wmmc88 commented Jan 2, 2021

younesr1 commented Jan 3, 2021

wmmc88 left a comment

Choose a reason for hiding this comment

wmmc88 Jan 4, 2021

Choose a reason for hiding this comment

wmmc88 Jan 4, 2021

Choose a reason for hiding this comment

wmmc88 Jan 4, 2021

Choose a reason for hiding this comment

younesr1 Jan 4, 2021

Choose a reason for hiding this comment

wmmc88 Jan 4, 2021

Choose a reason for hiding this comment

younesr1 Jan 4, 2021 • edited Loading

Choose a reason for hiding this comment

cindyli-13 commented Jan 4, 2021

younesr1 commented Jan 4, 2021

wmmc88 commented Jan 4, 2021

younesr1 commented Jan 5, 2021

younesr1 commented Jan 12, 2021

wmmc88 left a comment

Choose a reason for hiding this comment

younesr1 Jan 4, 2021 •

edited

Loading