search

clubcapra / takin

:goat: Capra-Takin is a ROS-based solution for managing and operating Club Capra's rescue robot. :sheep:
GNU General Public License v3.0
6 stars 3 forks source link

Takin motors #51

Closed lvanasse closed 5 years ago

lvanasse commented 5 years ago

Travail fait : Refactoring du package takin_motors et ajout du publishing de la température des moteurs

Avez-vous bien documenté vos changements ? Oui, une documentation a été ajouté au package

Est-ce que ça été testé et comment? : Aucun test n'a pu être exécuter à cause que le robot n'est pas en état d'intéragir avec les drives. Seulement une compilation local a été fait et réussi.

lvanasse commented 5 years ago

Aussi j'ai fait le test de merger localement avant de créer la PR

island212 commented 5 years ago
  1. remote_controller le nom pourrait être mieux.
  2. Question quiz est-ce que publishTemperature utilise a) les variable membres ou b) les variables local ? en plus d'etre ambigu, je pense que tu pourrais simplement enlever left_track et right_track dans ta signature de methode.
  3. Tu as oublié d'enlever le dynamic_reconfigure dans le main du cpp.
  4. Tu as oublié d'enlever le configCallback dans le cpp
  5. Tu peux enlever ros::spin() completement
  6. publishTemperature ne publish pas, il fait juste ajout dans la liste. nom ambigu
  7. As-tu tester la temperature ? parce que tu gardes toujours le même takin_msgs::temp donc les vector vont garder tous les valeurs de temperatures depuis le debut.
  8. takin_msgs::temp est un nom bof, tu devrais écrire Temperature.

Sinon je viens de voire que Temperature existe. Tu pourrais l'utiliser à la place

Sinon bonne documentation

lvanasse commented 5 years ago
  1. Ça utilise des variables membres.

3 & 4. J'ai pas oublié, je voulais le faire dans un autre PR, mais pas grave, je vais l'ajouter à celui-ci .

  1. Non, pcq le robot à un problème de connectivité avec les drives. Je peux juste le clear avant de l'utiliser soit dans la méthode ou dans la boucle while.

Aussi je vais juste enlever le msg et prendre Temperature.