> Si je fais en sorte que Encoder::begin() soit appelée dans une méthode
> begin() de ClosedLoopMotor, c'est un peu plus propre ?
Oui. Il est en général déconseillé d'appeler des fonctions du core
Arduino dans des constructeurs globaux. La raison est que ces
constructeurs sont appelés avant main(), et donc avant que le core ait
fait son initialisation. Certaines fonctions du core fonctionnent avant
cette initialisation, d'autres non, mais, ceci n'étant pas documenté, il
est plus sage d'éviter.
> Dire que j'ai poussé les étudiants à faire une architecture objet,
> pour rendre le code plus lisible :o/
Maintenant il faut leur expliquer que, parfois, les variables globales
c'est bien aussi. :o)
> Est-ce que c'est un truc qui pourrait être amélioré dans la librairie
> Arduino ? Une fonction attachInterrupt() un peu plus sioux ?
Le plus élégant ce serait que, en guise de callback, attachInterrupt()
accepte un std::function. Comme ça tu pourrais lui donner un foncteur,
ou une lambda avec capture. Seulement, ça nécessiterait un certain
support de la part de la STL qui n'est pas disponible sur la plateforme
AVR.
La façon « C traditionnel » ça aurait été que attachInterrupt() accepte
à la fois un pointeur de fonction pour le callback et un `void *` qui
serait passé en paramètre à ce callback. Je pense qu'ils ont évité ça
pour faire une API la plus simple possible pour les débutants.
Dans un cas comme dans l'autre, le std::function ou le `void *` aurait
été stocké dans une variable globale. Dans l'état actuel du core, c'est
à toi de gérer ces variables globales.
> Je n'arrive pas à comprendre comment (et où) utiliser le template que
> tu proposes... J'ai l'impression qu'on ne fait que repousser d'un
> niveau le problème, non ?
Exactement !
> Voici en gros leur architecture...
Puisqu'il faut des variables globales, autant les définir au niveau le
plus bas, dans le seul objet qui les utilise. Je te suggère ça :
Dans la classe Encoder, ajouter le membre
static Encoder *instances[2];
Un membre statique est alloué comme une variable globale. On a donc ici
un tableau de deux pointeurs (un par interruption) vers des objets
Encoder. Ces pointeurs sont maintenant accessibles aux routines
d'interruption, qui peuvent donc ainsi accéder à ces objets.
Ensuite, il faut définir deux routines d'interruption : une pour chaque
interruption. Il n'est pas possible d'utiliser une routine unique, car
celle-ci ne saurait pas quel objet Encoder utiliser. Pour ne pas
dupliquer du code, on peut écrire un template et laisser le compilateur
faire cette duplication de code. Mais il faudra ensuite _explicitement_
instancier le template deux fois.
Ça donne ça :
class Encoder
{
public:
Encoder(uint8_t pin): m_pin(pin) {}
void begin() {
int interrupt = digitalPinToInterrupt(m_pin);
void (*isr)();
switch (interrupt) {
case 0: isr = encoderISR<0>; break;
case 1: isr = encoderISR<1>; break;
default: /* report error? */ return;
}
attachInterrupt(interrupt, isr, CHANGE);
}
private:
static Encoder *instances[2];
uint8_t m_pin;
template<int interrupt> static void encoderISR() {
Encoder *that = instances[interrupt];
// do something with that...
}
};
Remarque que les routines encoderISR<0> et encoderISR<1> sont
instanciées explicitement dans le switch/case. Si jamais la fonction
encoderISR() est un peu longue (ce qui est normalement à éviter) et que
tu veux limiter la duplication de code, tu peux la remplacer par une
méthode normale et ajouter un template « wrapper » autour:
template<int interrupt> static void isr_wrapper() {
instances[interrupt]->encoderISR();
}
void encoderISR() {
// do something with this...
}
À+,
Edgar.
