search

clubcapra / markhor

🐐 Capra-Markhor is a ROS-based solution for managing and operating Club Capra's second rescue robot. 🐐
9 stars 1 forks source link

Tuning PID nouvelles gearbox #36

Closed benmalenfant closed 2 years ago

benmalenfant commented 2 years ago

Changement du prescaler de l'encodeur pour accomoder les nouveaux gearbox

lvanasse commented 2 years ago

Can those coefficients be member variables instead of magic numbers? Like we do for the other variables like tracks_kp and such?

benmalenfant commented 2 years ago

Yeah, will do, should integrate them as parameters form the launch file to allow easier tuning in the future

McCaroon commented 2 years ago

Coeffs. worked on the robot. Approved.

benmalenfant commented 2 years ago

Changes done, those are now parameters, needs to be tested, @lvanasse can you review?

lvanasse commented 2 years ago

Can you merge master into your branch? You seem to have commits from @saxtot that I assume aren't related to your PR, right?

benmalenfant commented 2 years ago

I rebased on master, I guess that's what caused the issue here

benmalenfant commented 2 years ago

Yeah it's the right version, it should not affect previous changes when we merge

benmalenfant commented 2 years ago

should all be good now, ready for testing

saxtot commented 2 years ago

Can you merge master into your branch? You seem to have commits from @saxtot that I assume aren't related to your PR, right?

The branch protection rules I added require the branch to be up to date with master before merging, hence my commits.

benmalenfant commented 2 years ago

So all good code wise, just waiting for testing?

lvanasse commented 2 years ago

So all good code wise, just waiting for testing?

For me I think that's right :)