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

New track velocity control #23

Closed benmalenfant closed 2 years ago

benmalenfant commented 2 years ago

Improvements to come :

benmalenfant commented 2 years ago

J'ai review pas mal de comments, je devrais essayer de preparer un code demain avec quelques fix (probablement pas tous) je devrais ĂȘtre bon pour le tester dans la semaine

benmalenfant commented 2 years ago

Also je pensais à ça, techniquement on serrait pas mieux de juste avoir un gros module hw_interface générique pour les talon srx qui serait paramétré avec un gros yaml file, apres on aurrait juste a l'instancier pour chacune des drives, apres t'a un controller qui les drives en rad ou en rad/s

Techniquement sa eliminerais bcp de code redondant mais sa en créerait bcp pour read les parametres faudrait check ce qui serrait le plus efficace pis c'est vraiment juste un nice to have

lvanasse commented 2 years ago

Also je pensais à ça, techniquement on serrait pas mieux de juste avoir un gros module hw_interface générique pour les talon srx qui serait paramétré avec un gros yaml file, apres on aurrait juste a l'instancier pour chacune des drives, apres t'a un controller qui les drives en rad ou en rad/s

Techniquement sa eliminerais bcp de code redondant mais sa en créerait bcp pour read les parametres faudrait check ce qui serrait le plus efficace pis c'est vraiment juste un nice to have

J'ai de la misÚre à imaginer ce que tu entends par «paramétré avec un gros yaml file», je suis plus ou moins fan d'avoir un fichier de configuration qui dicte comment les drives sont configuré par expérience ça créer de l'indirection et ça rend la compréhension du code beaucoup plus difficile (un bon exemple de cela ce sont les xacro files qui existe dans markhor_description, on sait p-e 100 lignes de code qui se répÚte un peu, mais c'est plus complexe à comprendre.). Dans notre cas, je pense qu'on peut rendre cela mieux et garder cela dans le code.

En fait, ce qui serait bien, ça serait d'avoir une classe parente qui contient les drives CTRE, un genre de Meta CTRE drive classe. Qui contient déjà le code boilerplate et qui facilite là configuration en utiliser genre une struct comme structure de paramÚtre.

benmalenfant commented 2 years ago

on est en train d'Ă©crire un roman, je dis on se fait un call pour en discuter

saxtot commented 2 years ago

Est-ce qu'on est good pour le merge? Les nouvelles modifications m'ont l'air bien

benmalenfant commented 2 years ago

J'aimerais juste qu'on test la branche ainsi que le nouveau ui sur markhor et l'operator station, un coup testé, je dis on est good to go.

Dans le futur faudrait rajouter le parameter file pour les gains du pid et les paramĂštres de l'antiwindup

saxtot commented 2 years ago

La vitesse/acceleration vont etre sujet a ajustements au fil d'utilisation, mais tout le reste est fonctionnel presentement. J'ai ajuste les launch files pour permettre controle sans UI. Ca necessite teleop en local et markhor_command sur Markhor. Le merge est pret imo.