search

eirbot / eirbot2020-2A

GNU General Public License v3.0
2 stars 1 forks source link

API bas niveau #12

Open astralien3000 opened 4 years ago

astralien3000 commented 4 years ago

Suite au message de @polyedre sur #6, il est grand temps d'ouvrir la discussion sur les API (bas niveau pour cette issue).

Je me repose sur ce document pour mes propositions.

Il s'agit de l'API qui régira la "communication" entre le code MCU (nucleo/teensy/client vrep) et les capteurs/actionneurs.

Un petit point sur les besoins :

A savoir que la simulation ne gère pas les moteurs comme ce qu'on fait sur un MCU. En effet, sur MCU, on envoie des PWM aux moteurs et il faut une surcouche d'asserv pour avoir des moteurs asservis en vitesse alors que la simu ne fournit que des moteurs déjà asservis en vitesse. Cependant il est possible de faire une couche d'adaptation par dessus la simu pour fournir un comportement semblable à une PWM.

Encoder

class Encoder {
    int32_t getSpeed(void); // vitesse en ticks d'encodeur par secondes
    int32_t getDistance(void); // distance parcourue depuis le début en ticks d'encodeur
};

Encoder encoder_left;
Encoder encoder_right;

// possibilité de définir d'autres encodeurs si dispo

Motor

class Motor {
    void setPWM(int16_t pwm); // largeur d'impulsion (relative!) en ticks de timer PWM
    // OU
    void setRatio(int16_t ratio); // "pourcentage" de voltage à envoyer au moteur (toujours relatif)
    // OU
    void setVoltage(int volts); // tension en volts / millivolts
    // OU
    void setSpeed(int speed); // vitesse
    /// unité ? RPM ? radians par secondes ? ticks d'encodeur par secondes ? ...
};

Motor motor_left;
Motor motor_right;

Les 3 premières propositions sont équivalentes (il y a "juste" un peu de conversion à faire entre le voltage, la pwm et le ratio). Le cas de setSpeed demande une couche d'asserv.

Proposition annexe

Afin de faciliter le debug, nous avions "fixé" la notion de Device (input et output). Cela permet de traiter tout les capteurs et actionneurs de la même façon, mais ça peut rendre l'API moins claire si mal géré.

On peut avoir par exemple une API du genre : 

class Motor : Device::Output<int> {
    void put(int speed);
};

Ou :

class Motor {
    class Speed : Device::Output<int> {
        void put(int speed);
    };

    Speed speed;
};

Enfin voila, les idées sont lancées, j'attends les retours.

astralien3000 commented 4 years ago

Qu'on puisse statuer sur l'api pour le simulateur et les codes des robots :

// Encoders

/* Return value of encoder, the diff has to be handled by the code */
Encoder::get(); // -> short ?

/* Set the speed of a motor, unit : meter per sec ? */
Motor::SetSpeed(); // -> void
Motor::SetCouple(); // -> void

J'en ai sûrement oublié

Deux remarques :

polyedre commented 4 years ago

D'acc, je suis pour garder la convention de nommage setSpeed, et pour tout passer en anglais !

Encoders

Je reviens sur la discussion à propos de

int32_t getDistance(void); // distance parcourue depuis le début en ticks d'encodeur

La distance peut overflow ou pas ?

Motors

Pour les moteurs,

void setSpeed(int speed); // vitesse en radian par seconde

me semble être l'implémentation la plus naturelle.

Proposition annexe

Wow je trouve ça très propre, je voulais faire un truc similaire mais j'avais pas pensé à utiliser les classes template pour faire ça !

Pour garder une cohérence de vocabulaire et simplifier, on pourrait passer toutes les variables à observer en input et output et implémenter les fonctions get et set

astralien3000 commented 4 years ago

Pour les encodeurs, la distance peut effectivement faire un overflow, sauf que sur 32 bits, vas falloir y aller pour faire un overflow. Normalement, lors d'un match, ça devrait pas arriver. Sauf que si ton compteur interne est sur seulement 16 bits, il va falloir gérer les overflow sur 16 et transformer en 32. Au pire, on pourrai mettre une API de reset du compteur si besoin.

Par ailleurs, pour les unité, j'ai oublié de préciser qu'on peut aussi bien utiliser les (milli)mètres par secondes, que ce soit pour les enodeurs ou les moteurs. Au moins ça serait les mêmes unités pour l'encodeur et le moteur.

Après pour les variables simples (pas celles qui demandent des transformations en tout genre comme les pwm, encodeurs, etc), on a même pas besoin de faire les getters et setters, j'avais fais un proto de code qui faisait le get/set automatiquement, et qui le mettais à dispo d'un shell. D'ailleurs on peut y voir le système de mapping que j'utilisais ("device.variable") afin de les rendre accessibles au haut niveau (#13).

Elioty commented 4 years ago

Aversive++ sort de cette issue !! 🤣

Sinon, pour le motor, je serais plutôt pour setRatio, c'est ce qui semble être le plus indépendant de la plateforme matériel (aussi bien du timer utilisé que du moteur et de sa tension nominale ou la présence ou non d'un encodeur, ce que nécessite setSpeed qui crée d'ailleurs un couplage).

Et pour la distance des encodeurs, vu que ça peut overflow, il vaut mieux un type non signé du point de vue du langage C/C++ et du compilateur car l'overflow sur un signé est UB.

astralien3000 commented 4 years ago

Je suis d'accord sur le setRatio, c'est celui qui est le moins dépendant du reste. Bon, du coup vu que y a "besoin" de setSpeed pour la simu, et que setSpeed sera construite au dessus de setRatio, je propose deux couches d'API : 

class Motor {
    void setRatio(int16_t ratio); // "pourcentage" de voltage batterie à envoyer au moteur
    // peut être plus sur une base 1000 que 100, pour avoir plus de précision
};

Motor mot_l;
Motor mot_r;
class MotorController {
    void setSpeed(float speed); // vitesse (rad/s)
};

MotorController motc_l;
MotorController motc_r;

MotorController utilisera Motor. Comme ça, on peut "couper" là où on veut.

Elioty commented 4 years ago

Oui je préfère avec une couche Motor pour gérer un moteur seul puis MotorController qui couple un Motor et un Encoder (voir même plutôt un Input) pour faire la boucle d'asservissement en vitesse 😄

Concernant le ratio, je serai plutôt pour utiliser une puissance de 2 comme valeur max, donc dans l'exemple avec un int16_t : 2^15-1. Ça permet 1) au développeur utilisant l'interface de se posait moins de question, le max c'est le max du type du paramètre et 2) vu que 99,9% des implémentations de Motor seront basés sur une PWM d'un timer, ça permet de facilement transformer ce ratio en valeur à programmer dans le timer juste en faisant un shift vers la gauche (et en masquant le MSB si on prend un type signé en entrée, ou en prenant toutes valeurs négatives pour 0 car c'est peut-être que l'appelant fait des conneries).

polyedre commented 4 years ago

J'ai pas tout compris mais ok ;) Je vais bientôt commencer l'adaptation du code de l'année dernière, j'ai fork ce repo pour pouvoir expérimenter un peu avec les pull request.

polyedre commented 4 years ago

Un truc que je comprend pas au niveau de l'API, pour le code de la nucléo j'ai besoin de fournir au constructeur de la classe Motor le pin de sortie de la PWM. Si on utilise le simulateur, on va donner un PIN bidon ?

Elioty commented 4 years ago

Un truc que je comprend pas au niveau de l'API, pour le code de la nucléo j'ai besoin de fournir au constructeur de la classe Motor le pin de sortie de la PWM. Si on utilise le simulateur, on va donner un PIN bidon ?

Je pense que ça devrait être une référence vers un Output, T étant le type choisi pour le ratio donc a priori int16_t. En embarqué, la méthode put convertira le ratio en PWM comme je l'ai dit dans mon message précédent puis appellera ce qu'il faut sur le timer pour programmer la PWM. En simulé, la méthode put fera le boulot pour communiquer une accélération (si j'ai bien suivi) au logiciel de simulation.

astralien3000 commented 4 years ago

Erf, j'arrive pas à prendre du temps en ce moment. Bon, rapidement : A voir pour le ratio, mais ça ressemble plus à une PWM qu'un ratio dans ce cas (c'est juste une question de vocabulaire). Le petit avantage d'un ratio limité à 1000 par exemple est que c'est plus intuitif à lire pour un humain, et en plus on a plein de valeurs interdites qui permettent de vérifier qu'on a pas envoyé de la merde alors qu'un ratio sur 65536/2 est beaucoup plus difficile à lire et peut importe ce qu'on lui envoie, le moteur le fera. Bon vu qu'on est censé bien coder, ça devrait pas déconner mais bon ^^ on va pas cracher sur un peu plus de sécurité ? Et pour la précision, on en a pas besoin au point d'avoir 65536 pas normalement. (rappelle toi des 256 pas qu'on avait en 2014 ! XD)

Pour revenir sur la remarque sur les overflow encodeurs, il vaut mieux avoir un entier signé que non signé dans l'API. En effet, sur un non signée, l'overflow et l'underflow vont arriver tout le temps puisqu'on vas vraisemblablement bouger près du zero de l'encodeur. L'utilisateur a pas que ça a faire de gérer les overlow. Au pire on peut commencer à INT_MAX/2 pour éviter que ça arrive (trop vite). Je savais pas pour l'Undefined Behaviour des entiers signés. Sachant que le registre de Timer seront non signés de toute façon, la gestion d'overflow interne pourra être non signée pour éviter l'UB. Mais bon, en vrais, on a quand même une bonne idée du comportement de l'overflow des entier signés (au pire à tester pour vérifier).

Pour la construction des moteurs : déjà la simu s'arrêtera à l'API des MotorController, c'est pour ça que je l'ai rajouté dans l'API. En suite pour le problème du constructeur, On est pas obligé d'avoir le même constructeur pour les 3 versions ^^ ! En effet, on vient de définir l'API "commune" aux 3, mais les autres fonctions sont pas définies et surtout sont pas obligées d'être les même. Bon, ceci dit, il vaut mieux éviter d'utiliser d'autres fonctions non communes dans le programme principal, mais si on est obligé, comme pour le constructeur, ça se justifie. Donc pour moi, le code teensy aura un constructeur avec des pins teensy, le code nucleo aura des pins nucleo, et les simulateur aura une adresse IP et un nom de joint.

EDIT:

Je pense que ça devrait être une référence vers un Output, T étant le type choisi pour le ratio donc a priori int16_t

A voir, mais pour moi ça ne fait que repousser le problème. Il faudra bien que ces Output soient différentiés à un moment ou à un autre, et on revient au problème initial.

EDIT2 :

@polyedre : on a une petite "confusion" entre la méthode "set" et "put". Pour la petite histoire, on utilisait "set" dans les premières version d'Aversive++ parce que c'était le plus naturel. Mais après on a eu quelques remarques du genre que si notre device est en Input et Output, on avait l'impression qu'on manipulait la même variable interne (get et set, logique) alors que en vrais faire un "set" modifiait un truc différent de ce qu'on récupérait avec "get". Par ailleurs on rapproché la notion de device de la notion de stream (en s'inspirant de la norme POSIX) avec sont getchar et putchar, d'où le duo "get"/"put" au lieu de "get"/"set". Un device représente donc plus un flux (stream) de commandes ou données capteurs qu'une variable du programme. En vrais les 2 me vont mais il faudra faire un choix que je te laisse faire maintenant que tu a le pourquoi ^^ !

EDIT3 : EDIT = le nouveau PS (désolé)

Elioty commented 4 years ago

Mais bon, en vrais, on a quand même une bonne idée du comportement de l'overflow des entier signés (au pire à tester pour vérifier).

Justement non. On sait côté jeu d'instruction en lisant la documentation des instructions arithmétiques mais côté compilateur, vu que c'est UB, on n'en a aucune idée. Voir cet exemple sous godbolt. L'overflow des signés étant UB, le compilateur/optimiseur peut supposer que le développeur ne va jamais en faire et donc faire des optimisations comme celles que l'on peut voir sur l'exemple. En -O0, on voit que les deux fonctions donnent le même code assembleur. En -Os par contre, la fonction signée ne fait même pas de comparaison et retourne 1 en dur alors que la fonction non-signée fait bien une comparaison puis deux affections conditionnelles pour retourner le bon résultat.

EDIT : Je viens de tester tous les niveaux d'optimisation (1, 2, 3, s, fast), il donnent tous le même code assembleur.

astralien3000 commented 4 years ago

On peut mettre 64 bits si tu a peur de l'overflow signé aussi ^^, je trouve ça un peu overkill mais ça passe. Si on a 1/10 mm par tick encodeur, sur 32 bits on peut toujours faire ~200 km avant l'overflow. Autre solution : saturer. Si on atteint INT_MAX ou INT_MIN, on limite.

Elioty commented 4 years ago

200km de toupie, ou à tourner en rond (seuls moyens pour que les encodeurs ne fassent qu'avancer dans une direction), ça va en prendre pas mal de temps quand même 🤣 pour le coup démarrer à 0 sur un signé devrait aller quand même. On fait un robot sur une pauvre table ici, pas un avion ou une voiture 😅