ned_ros
ned_ros copied to clipboard
NiryoRobotics
Bug fixes and improvements
Bien bonjour. Avant de commencer, je me permets de parler en français, car cela est plus simple pour moi et j'ai remarqué que vos développeurs ainsi que votre entreprise l'était. Si cela pose un problème, je peux essayer de communiquer en anglais, mais je ne garantis pas la certitude des informations.
Bonjour, je suis étudiant en informatique en BTS, et pour mon projet de fin d'année, je dois utiliser votre robot, le Niryo Ned 2, dans un cycle de production industriel à échelle réduite. Je dois le faire communiquer en ModBus/TCP, mais je suis déçu de voir que la partie modbus de votre projet n'est pas fini, et n'a pas subi assez de tests, car pas grand-chose ne fonctionne correctement. J'ai donc plongé dans votre code source pour le modifier et ainsi compléter et corriger le système du niryo (je suis un développeur expérimenté, donc ça n'a pas été très difficile).
J'ai donc corrigé les bugs de la partie modbus, ainsi qu'ajouté des fonctionnalités qui me semblait nécessaire pour une bonne utilisation. J'ai remarqué qu'il y avait aussi quelques oublies dans la classe principale de l'api, NiryoRosWrapper, je l'ai donc aussi modifié pour ajouter ce qu'il manquait. J'ai aussi modifié la partie gérant le bouton sur le socle du robot, car il manquait la fonction pour relancer le programme d'autorun.
Voici toutes les modifications que j'ai effectuées, j'ai essayé de les propager dans les fichiers utilisant les fonctions qui ont été modifiés pour éviter tout problème de test futur.
J'ai aussi modifié la documentation pour inclure les changements effectués, mais je ne sais pas si cela est bien fait, car je n'arrive pas à cloner le sous-module git sphinx_doc/front_end. J'espère que vous m'aiderai sur ce point pour ne rien oublier dans la documentation.
Je vous souhaite une bonne journée, et espère que vous prendrez en considération mes modifications. =)
Hello,
Effectivement comme tu l'as mentionné sur le discord il y a une nouvelle version de notre stack ROS. La dernière version est la v5.3.3, à ne pas confondre avec la v5.4.0 qui est la version de l'image système.
La dernière version embarque entre autres un refacto complet du package modbus qui devrait régler tous les problèmes rencontrés jusqu'à maintenant. (Voir la doc modbus)
Cependant, les autres changements que tu as faits ont l'air pertinents à 1ʳᵉ vue, je te laisse régler les éventuels conflits que tu pourras avoir avant de faire une review de la pull request.
mmm ok, github a fermé le pull request quand j'ai re-syncronisé la branche. je le rouvrirais quand je mettrais les changements
pour le reste, je suis pas sûr donc, je ferais des tests. Mais la semaine prochaine sur le robot, car je suis en épreuve de BTS toute cette semaine.
@jmottier-niryo Juste une petite question : dans une ancienne version la fonction get_saved_pose_list répurérais les valeurs a partir d'un service nommé /niryo_robot_poses_handlers/get_pose_list,.
Tandis que dans les nouvelles versions, cette meme fonction fait appel au publisher /niryo_robot_poses_handlers/pose_list. Mais il faut faire une étape de décodage qui n'était pas nécessaire dans les anciennes version.
Pourquoi avoir changé, le service n'était pas bien ? il est toujours présent dans le code source pourtant.
autre chose que j'ai remarqué : vous avez implémenté une fonction get_current_tool_state dans le ros_wrapper, mais je ne vois documenté nul par les états qu'il doit renvoyé. Le les ais trouvés dans le code source, mais pas très pratique.
Et d'ailleur, en regardant cela, je voit qu'il manque l'état pour l'électro magnet
En re-regardant le fonctionnement du nouveau serveur modbus, j'ai remarqué qu'il était serte plus jolie, plus objet et plus esthétique, mais surtout beaucoup beaucoup moins optimisé. Pour une simple tâche de récupérer la position des axes, il doit faire autant de demande que de valeur voulue, pour juste en prendre une seule. Donc, il doit demander 6x la position des axes pour récupérer qu'une seule valeur à chaque fois. Alors qu'il serait plus simple d'en faire la demande qu'une seule fois, mettre en cache, puis récupérer les valeurs dedans, puis on met à jour le cache si ce que l'on fait n'est pas de récupérer tous les axes, sa évitera d'avoir de fausses valeurs.
Quel est votre avis sur cela ? Serait-il possible d'implémenter un système qui vérifie, si l'on récupère un groupe de donnés, de demander uniquement le groupe voulu, au lieu de relancer la fonction avec l'index d'incrémenté ?
ou alors plutot que ABCRegisterEntries.get revoie une liste et ABCRegisterEntry.get renvoie une seule valeur. Comme ça dans ABCRegisterEntries.read on vérifie le type de renvoi et si c'est une liste, on récupère uniquement ce qui nous intéresse.
EDIT : Non cette méthode ne sera pas possible vue la conception du modbus, il faudrait refaire CustomModbusSlaveContext pour cela.
Pourquoi avoir changé, le service n'était pas bien ? il est toujours présent dans le code source pourtant.
Nous avons créé un topic afin de remplacer le service. En effet, il est plus logique d'avoir un système de pub/sub pour avoir la liste de poses en temps réel plutôt que de devoir faire un appel au service régulièrement.
il faut faire une étape de décodage qui n'était pas nécessaire dans les anciennes version
Nous utilisons maintenant des messages BasicObject et BasicObjectArray qui vont à terme remplacer les listes de noms et de description, car nous estimons que c'est une manière plus fiable de gérer les données.
autre chose que j'ai remarqué : vous avez implémenté une fonction get_current_tool_state dans le ros_wrapper, mais je ne vois documenté nul par les états qu'il doit renvoyé.
Effectivement il pourrait être utile d'indiquer les états. Ils sont définis dans le message Tool du package tools_interface
Et d'ailleur, en regardant cela, je voit qu'il manque l'état pour l'électro magnet
l'electro-magnet est un tool à part car il est contrôlé via la DO4
Quel est votre avis sur cela ? Serait-il possible d'implémenter un système qui vérifie, si l'on récupère un groupe de donnés, de demander uniquement le groupe voulu, au lieu de relancer la fonction avec l'index d'incrémenté ?
Un système de cache augmenterait grandement la complexité du serveur modbus pour une amélioration minime des performances. De plus, l'intérêt est assez limité pour des valeurs qui fluctuent à une très haute fréquence, telles que la pose du robot.
De plus, les performances restent correctes même si elles ne sont pas optimales. Les seules opérations que nous faisons est la création d'un tableau et un get sur celui-ci. La pose du robot est stockée dans __pose_ntv, qui contient le subscriber au topic
Effectivement il pourrait être utile d'indiquer les états. Ils sont définis dans le message
Tooldu packagetools_interface
Est-ce qu'ensuite, vous pourriez m'aider pour modifier la documentation, et aussi ajouter les codes retournés de cette fonction, get_current_tool_state ?
Non il y en a encore quelques-uns. Exemple : https://github.com/NiryoRobotics/ned_ros/pull/19#discussion_r1599698799
d'accord, et pour clear_detected_collision(), c'est quoi qui doit émettre sur ce topic ? car c'est la seule fonction disponible pour retirer le trigger de collision.
Je me demandais aussi s'il est possible d'ajouter un registre modbus pour activer ou désactiver la gestion de la manette. J'ai tenté ce matin de connecter une manette (je n'ai pas encore fait la mise à jour du robot) et j'ai remarqué que la gestion de la manette n'est pas activée par défaut. Il faut lancer un service ROS, et lancer un script python qui va bouger le robot en fonction des boutons pressés.
d'accord, et pour
clear_detected_collision(), c'est quoi qui doit émettre sur ce topic ? car c'est la seule fonction disponible pour retirer le trigger de collision.
@jmottier-niryo
@jmottier-niryo pourquoi ne répondez-vous pas ?
pour clear_detected_collision(), c'est quoi qui doit émettre sur ce topic ? car c'est la seule fonction disponible pour retirer le trigger de collision.
On ne t'a pas oublié, on a juste beaucoup de choses à gérer.
Concernant clear_detected_collision, cette fonction n'a pas besoin d'émettre sur un topic. c'est un flag interne qui sert en interne au wrapper.
Le système de détection de collision est grandement améliorable mais il faudrait le revoir de zéro. Pour le moment il est préférable de le laisser tel quel pour éviter tout effet de bord.
On ne t'a pas oublié, on a juste beaucoup de choses à gérer.
Concernant
clear_detected_collision, cette fonction n'a pas besoin d'émettre sur un topic. c'est un flag interne qui sert en interne au wrapper.Le système de détection de collision est grandement améliorable mais il faudrait le revoir de zéro. Pour le moment il est préférable de le laisser tel quel pour éviter tout effet de bord.
Oui, mais cela pose un problème, qu'il soit interne, car il est donc propre à chaque instance de la classe. Alors que c'est justement ce que l'on veut éviter, car si on réinitialise le flag par modbus, par exemple, et bien il refusera de bouger, puisqu'il l'a été que du côté modbus, et non pas du reste. Et aussi en regardant le nouveau modbus, une instance de la classe est créée pour chaque type de registre, voir pour chaque valeur, donc ça n'a aucun intérêt.
Je trouve qu'il est bien mieux de trouver une solution un peu bancale qui fonctionne, plutôt que d'en attendre une autre qui sera mieux.
Il est ainsi préférable de le faire emmètre sur le topic et que cela ne soit pas très bien niveau conception logiciel, plutôt que d'attendre pour une refonte du système, mais qui prendra du temps. Cela peut être commenté pour dire que la solution n'est pas très bien mais qu'elle fait l'affaire pour le moment.
Bonjour @ZetaMap. Tout d'abord merci pour la proposition de contribution, c'est toujours intéressant d'avoir des retours de la part d'utilisateurs avec des propositions d'amélioration. Cependant, la stack robotique est entrain d'évoluer en interne et la plupart de vos propositions ont déjà été adressées d'une manière ou d'une autre dans une future version. Par manque de ressources nous ne pouvons investir trop de temps dans cette PR, qui intègre des changements de design que nous ne pouvons nous permettre d'avoir pour le moment. Par conséquent, nous allons fermer cette PR.
