Entrées / sorties : écriture d'un bit sans perturber les autres bits du même port

[electroremy]

Bonjour,

J'ai rencontré un problème qui m'a donné un peu de fil à retordre.

J'avais adapté des librairies pour faire fonctionner ensemble, sur un Arduino UNO :

- le shield Ethernet 2 avec une puce W5500
- un écran LCD graphique avec une puce ILI9341
- une dalle tactile avec une puce XPT2046

Voici le câblage :

- Le shield Ethernet 2 utilise le bus SPI
- L'écran LCD graphique utilise le bus SPI, la librairie est une version optimisée de celle d'Adafruit pour exploiter au mieux la vitesse du hardware SPI
- La dalle tactile est en SPI mais gérée en bitbanging, avec une version optimisée de la librairie URTouch

Un circuit convertisseur 3.3V/5V est intercalé entre l'Arduino et l'écran tactile

Je ne suis pas arrivé à gérer la dalle tactile avec le hardware SPI mais l'ensemble fonctionne très bien

 

J'ai voulu adapter mon projet pour le faire fonctionner sur un Arduino MEGA

Le câblage et le code ont dû être modifiés car les broches SPI ne sont pas les mêmes sur l'UNO et le MEGA :

#if defined(ARDUINO_AVR_UNO)
    #define    ILI9341_CS_PIN    0 // (PD0)
    #define    ILI9341_DC_PIN    9 // (PB1)
    #define    ILI9341_RST_PIN    8 // (PB0)
    //      ILI9341_MISO    12   (PB4) No choice, Mega Hardware SPI MISO - Also used by Ethernet Shield
    //      ILI9341_MOSI    11   (PB3) No choice, Mega Hardware SPI MOSI - Also used by Ethernet Shield
    //      ILI9341_CK      13   (PB5) No choice, Mega Hardware SPI CK   - Also used by Ethernet Shield
#else
    // ARDUINO MEGA
    #define    ILI9341_CS_PIN    49 // (PL0)
    #define    ILI9341_DC_PIN    48 // (PL1)
    #define    ILI9341_RST_PIN    47 // (PL2)
    //      ILI9341_MISO    50    (PB3) No choice, Mega Hardware SPI MISO - Also used by Ethernet Shield
    //      ILI9341_MOSI    51    (PB2) No choice, Mega Hardware SPI MOSI - Also used by Ethernet Shield
    //      ILI9341_CK      52    (PB1) No choice, Mega Hardware SPI CK   - Also used by Ethernet Shield
#endif

 

Sur l'Arduino MEGA, j'ai voulu câbler les I/O de l'écran tactile sur les I/O qui étaient à proximité de celles utilisées par l'écran graphique LCD

Et rien ne fonctionnait...

Comme j'avais beaucoup optimisé les librairie pour l'Arduino UNO, je me suis dit que j'avais certainement rendu celles-ci incompatibles avec l'Arduino MEGA 

 

Après pas mal de tests, je me suis rendu compte que c'était juste le choix des broches d'I/O qui étaient en cause ; pour résumer :

#if defined(ARDUINO_AVR_UNO)
    #define    T_IRQ    5 // (PD5) 
    #define    T_CS    2 // (PD2) 
    #define    T_DOUT    7 // (PD7) MISO
    #define    T_DIN    6 // (PD6) MOSI
    #define    T_CLK    1 // (PD1) 
#else
    // ARDUINO MEGA

    // FONCTIONNE CORRECTEMENT :
    #define    T_IRQ    33 // (PC4)
    #define    T_CS    34 // (PC3)
    #define    T_DOUT    35 // (PC2)
    #define    T_DIN    36 // (PC1)
    #define    T_CLK    37 // (PC0)

    // NE FONCTIONNE PAS :
    // #define    T_IRQ    42 // (PL7)
    // #define    T_CS    43 // (PL6)
    // #define    T_DOUT    44 // (PL5)
    // #define    T_DIN    45 // (PL4)
    // #define    T_CLK    46 // (PL3)

    // AVEC L'ECRAN TFT BRANCHE AINSI SUR L'ARDUINO MEGA :
    // ILI9341_CS_PIN    49 (PL0)
    // ILI9341_DC_PIN    48 (PL1) 
    // ILI9341_RST_PIN    47 (PL2)
    // ILI9341_MISO     50 (PB3) No choice, Mega Hardware SPI MISO
    // ILI9341_MOSI     51 (PB2) No choice, Mega Hardware SPI MOSI
    // ILI9341_CK       52 (PB1) No choice, Mega Hardware SPI CK
#endif

 

Ce qui pose problème, c'est d'utiliser des broches du même port (en l'occurrence, le port PL) pour :
- les broches CS, DS et RST de l'écran tactile
- les broches de la dalle tactile en SPI bitbanging

 

Le code de la librairie URTouch utilise des macros assembleurs pour modifier un seul bit d'un port :
 

#define cbi(reg, bitmask) *reg &= ~bitmask
#define sbi(reg, bitmask) *reg |= bitmask
#define rbi(reg, bitmask) ((*reg) & bitmask)

#define pulse_high(reg, bitmask) sbi(reg, bitmask); cbi(reg, bitmask);
#define pulse_low(reg, bitmask) cbi(reg, bitmask); sbi(reg, bitmask);

 

On a tendance à l'oublier avec les langages "user friendly" comme Arduino : sur un microcontrôleur 8 bits, on n'écrit pas directement sur une broche d'entrée/sortie, mais on écrit un octet dans un port. 

J'en suis arrivé à la conclusion que les instructions *reg &= ~bitmask et/ou *reg |= bitmask appliquées au registre d'un port pourraient engendrer des changements d'état parasites des bits qui ne sont pas modifiés, ce qui fait planter la puce ILI9341 de l'écran graphique.
Mais je n'en suis pas sûr...
Quel est votre avis ?

J'ai d'abord pensé que la librairie qui gère l'ILI9341 avec le hardware SPI était aussi en cause...
.. mais en faisant un test avec la librairie UTFT en bitbanging sur les mêmes broches le bug est aussi présent !

Voici le code complet de URTouch.h :

// DEFINE HERE THE PIN USED --------------------------------------------------------------
#if defined(ARDUINO_AVR_UNO)
    #define    T_IRQ    5 // (PD5) 
    #define    T_CS    2 // (PD2) 
    #define    T_DOUT    7 // (PD7) MISO
    #define    T_DIN    6 // (PD6) MOSI
    #define    T_CLK    1 // (PD1) 
#else
    // ARDUINO MEGA

    // FONCTIONNE CORRECTEMENT :
    #define    T_IRQ    33 // (PC4)
    #define    T_CS    34 // (PC3)
    #define    T_DOUT    35 // (PC2)
    #define    T_DIN    36 // (PC1)
    #define    T_CLK    37 // (PC0)

    // NE FONCTIONNE PAS :
    // #define    T_IRQ    42 // (PL7)
    // #define    T_CS    43 // (PL6)
    // #define    T_DOUT    44 // (PL5)
    // #define    T_DIN    45 // (PL4)
    // #define    T_CLK    46 // (PL3)

    // AVEC L'ECRAN TFT BRANCHE AINSI SUR L'ARDUINO MEGA :
    // ILI9341_CS_PIN    49 (PL0)
    // ILI9341_DC_PIN    48 (PL1) 
    // ILI9341_RST_PIN    47 (PL2)
    // ILI9341_MISO     50 (PB3) No choice, Mega Hardware SPI MISO
    // ILI9341_MOSI     51 (PB2) No choice, Mega Hardware SPI MOSI
    // ILI9341_CK       52 (PB1) No choice, Mega Hardware SPI CK
#endif
// ---------------------------------------------------------------------------------------

#if !defined(TFT_SIZE_WIDTH) 
#define TFT_SIZE_WIDTH 240
#endif
#if !defined(TFT_SIZE_HEIGHT) 
#define TFT_SIZE_HEIGHT 320 
#endif
#define PORTRAIT            0
#define LANDSCAPE            1

// Now calibration data is saved on EEPROM, and calibration software is included in the "setup menu" of the interface :-)
// So you don't have to burn a separate calibration sketch and include calibration data in the INO file for each bord and each screen
int    touch_x_left, touch_x_right, touch_y_top, touch_y_bottom;

 // Hardware specific defines 
#define cbi(reg, bitmask) *reg &= ~bitmask
#define sbi(reg, bitmask) *reg |= bitmask
#define rbi(reg, bitmask) ((*reg) & bitmask)
#define pulse_high(reg, bitmask) sbi(reg, bitmask); cbi(reg, bitmask);
#define pulse_low(reg, bitmask) cbi(reg, bitmask); sbi(reg, bitmask);
#define regtype volatile uint8_t
#define regsize uint8_t
#define PREC_LOW            1
#define URTouch_Prec 102

class URTouch {
    public:
        int16_t    TP_X ,TP_Y;
        void    InitTouch(byte orientation);
        bool    read();
        bool    dataAvailable();
        int16_t    getX();
        int16_t    getY();
        void    calibrateRead();
        byte    FutureOrientation;
        void    UpdateOrientation(); 
    private:
        regtype *P_CLK, *P_CS, *P_DIN, *P_DOUT, *P_IRQ;
        regsize B_CLK, B_CS, B_DIN, B_DOUT, B_IRQ;
        int    disp_x_size, disp_y_size;
        void    touch_WriteData(byte data);
        word    touch_ReadData();
        byte    orient;
};

void URTouch::touch_WriteData(byte data) {
    byte temp;
    temp=data;
    cbi(P_CLK, B_CLK);
    for(byte count=0; count<8; count++)    {
        if(temp & 0x80)
            sbi(P_DIN, B_DIN);
        else
            cbi(P_DIN, B_DIN);
        temp = temp << 1; 
        cbi(P_CLK, B_CLK);                
        sbi(P_CLK, B_CLK);
    }
}

word URTouch::touch_ReadData() {
    word data = 0;
    for(byte count=0; count<12; count++) {
        data <<= 1;
        sbi(P_CLK, B_CLK);
        cbi(P_CLK, B_CLK);                
        if (rbi(P_DOUT, B_DOUT)) data++;
    }
    return(data);
}

void URTouch::UpdateOrientation() {
    orient = FutureOrientation;
}

void URTouch::InitTouch(byte orientation) {
    FutureOrientation        = orientation;
    orient                    = orientation;
    disp_x_size = TFT_SIZE_WIDTH-1;
    disp_y_size = TFT_SIZE_HEIGHT-1;
    P_CLK    = portOutputRegister(digitalPinToPort(T_CLK));
    B_CLK    = digitalPinToBitMask(T_CLK);
    P_CS    = portOutputRegister(digitalPinToPort(T_CS));
    B_CS    = digitalPinToBitMask(T_CS);
    P_DIN    = portOutputRegister(digitalPinToPort(T_DIN));
    B_DIN    = digitalPinToBitMask(T_DIN);
    P_DOUT    = portInputRegister(digitalPinToPort(T_DOUT));
    B_DOUT    = digitalPinToBitMask(T_DOUT);
    P_IRQ    = portInputRegister(digitalPinToPort(T_IRQ));
    B_IRQ    = digitalPinToBitMask(T_IRQ);
    pinMode(T_CLK,  OUTPUT);
    pinMode(T_CS,   OUTPUT);
    pinMode(T_DIN,  OUTPUT);
    pinMode(T_DOUT, INPUT);
    pinMode(T_IRQ,  OUTPUT);
    sbi(P_CS, B_CS);
    sbi(P_CLK, B_CLK);
    sbi(P_DIN, B_DIN);
    sbi(P_IRQ, B_IRQ);
}

bool URTouch::read() {
    unsigned long tx=0, temp_x=0;
    unsigned long ty=0, temp_y=0;
    unsigned long minx=99999, maxx=0;
    unsigned long miny=99999, maxy=0;
    byte datacount=0;
    cbi(P_CS, B_CS);                    
    pinMode(T_IRQ,  INPUT);
    for (byte i=0; i<URTouch_Prec; i++) {
        if (!rbi(P_IRQ, B_IRQ))    {
            touch_WriteData(0x90);        
            pulse_high(P_CLK, B_CLK);
            temp_x = touch_ReadData();
            if (!rbi(P_IRQ, B_IRQ))    {
                touch_WriteData(0xD0);      
                pulse_high(P_CLK, B_CLK);
                temp_y = touch_ReadData();
                if ((temp_x>0) and (temp_x<4096) and (temp_y>0) and (temp_y<4096)) {
                    tx += temp_x;
                    ty += temp_y;
                    if (temp_x<minx) minx = temp_x;
                    if (temp_x>maxx) maxx = temp_x;
                    if (temp_y<miny) miny = temp_y;
                    if (temp_y>maxy) maxy = temp_y;
                    datacount++;
                }
            }
        }
    }
    pinMode(T_IRQ,  OUTPUT);
    tx -= minx + maxx;
    ty -= miny + maxy;
    datacount -= 2;
    sbi(P_CS, B_CS);                    
    if ((datacount==(URTouch_Prec-2)) or (datacount==PREC_LOW)) {
        if (orient == PORTRAIT)    {
            TP_X = ty/datacount;
            TP_Y = tx/datacount;
        }    else    {
            TP_X = tx/datacount;
            TP_Y = ty/datacount;
        }
        return true;
    }    else    {
        return false;
    }
}

bool URTouch::dataAvailable() {
    bool avail;
    pinMode(T_IRQ,  INPUT);
    avail = !(rbi(P_IRQ, B_IRQ));
    pinMode(T_IRQ,  OUTPUT);
    return avail;
}

int16_t URTouch::getX() {
    int c;
    if (orient == PORTRAIT)    {
        c = long(long(TP_X - touch_x_left) * (disp_x_size)) / long(touch_x_right - touch_x_left);
    }    else    {
        c = long(long(TP_X - touch_y_top) * (-disp_y_size)) / long(touch_y_bottom - touch_y_top) + long(disp_y_size);
    }
    return c;
}

int16_t URTouch::getY() {
    int c;
    if (orient == PORTRAIT)    {
        c = long(long(TP_Y - touch_y_top) * (disp_y_size)) / long(touch_y_bottom - touch_y_top);
    }    else    {
        c = long(long(TP_Y - touch_x_left) * (disp_x_size)) / long(touch_x_right - touch_x_left);
    }
    return c;
}

void URTouch::calibrateRead() {
    unsigned long tx=0;
    unsigned long ty=0;
    cbi(P_CS, B_CS);                    
    touch_WriteData(0x90);        
    pulse_high(P_CLK, B_CLK);
    tx=touch_ReadData();
    touch_WriteData(0xD0);      
    pulse_high(P_CLK, B_CLK);
    ty=touch_ReadData();
    sbi(P_CS, B_CS);                    
    TP_X=ty;
    TP_Y=tx;
}

Voici comment le code est utilisé dans le projet :

URTouch ts; 

void setup() {
        ...
    ts.InitTouch(PORTRAIT); 
        ...
}
void loop() {

    if (ts.dataAvailable()) {
        if (ts.read()) {
            Touch_X = ts.getX();
            Touch_Y = ts.getY();
            ...
        }
    }
    ...
}

    

[Kachidoki]

Salut,

Il y a 18 heures, electroremy a dit :

J'en suis arrivé à la conclusion que les instructions *reg &= ~bitmask et/ou *reg |= bitmask appliquées au registre d'un port pourraient engendrer des changements d'état parasites des bits qui ne sont pas modifiés, ce qui fait planter la puce ILI9341 de l'écran graphique.
Mais je n'en suis pas sûr...
Quel est votre avis ?

Tu as bien sniffé, mais ce n'est pas exactement des changements d'état au sens glitch du terme. En fait les macros en C ne sont pas atomiques.

En clair tu as trois opérations : Lecture, modification, écriture.

Après lecture, si tu as une lib ou une interruption qui modifie un bit qui la concerne, lorsque toi tu vas appliquer l'écriture de tes bits (donc tout le port comme tu le souligne) tu va venir écraser ce bit par l'état du moment de la lecture.

Il te faut soit protéger les macros en les encadrant par une section critique (en gros disable interrupt), soit utiliser la méthode du PIN toggle si applicable.

A+

Modifié (le) il y a 22 heures par Kachidoki

[electroremy]

Il y a 2 heures, Kachidoki a dit :

Après lecture, si tu as une lib ou une interruption qui modifie un bit qui la concerne, lorsque toi tu vas appliquer l'écriture de tes bits (donc tout le port comme tu le souligne) tu va venir écraser ce bit par l'état du moment de la lecture.

Tu m'as grillé  - j'avais trouvé l'explication mais tu as répondu avant que je le fasse 

Mais je suis content, j'avais peur que le sujet n'intéresse pas grand monde !

 

Je vais juste donner un peu plus détails - ça pourra être utile pour les autres "Arduinistes" du forum 

 

Alors, dans le code de la librairie URTouch, ces deux lignes modifient un bit du port de façon plus rapide que digitalWrite()

*reg &= ~bitmask

ou encore

*reg |= bitmask

 

On peut découvrir la complexité de digitalWrite() ici :

https://garretlab.web.fc2.com/en/arduino/inside/hardware/arduino/avr/cores/arduino/wiring_digital.c/digitalWrite.html

 

void digitalWrite(uint8_t pin, uint8_t val)
{
        uint8_t timer = digitalPinToTimer(pin);
        uint8_t bit = digitalPinToBitMask(pin);
        uint8_t port = digitalPinToPort(pin);
        volatile uint8_t *out;
 
        if (port == NOT_A_PIN) return;
 
        // If the pin that support PWM output, we need to turn it off
        // before doing a digital write.
        if (timer != NOT_ON_TIMER) turnOffPWM(timer);
 
        out = portOutputRegister(port);
 
        uint8_t oldSREG = SREG;
        cli();
 
        if (val == LOW) {
                *out &= ~bit;
        } else {
                *out |= bit;
        }
 
        SREG = oldSREG;
}

 

Chose intéressante, digitalWrite() fait la même chose que les macros de URTouch (voyez les lignes *out &= ~bit et *out |= bit)  mais avec plus de précautions...

... en particulier, digitalWrite() désactive temporairement les interruptions avant d'exectuer les instructions qui modifient le registre de sortie

Cette désactivation temporaire est réalisée par les lignes de code suivantes :

        uint8_t oldSREG = SREG;
        cli();
...
        SREG = oldSREG;

SREG est le registre dont un des bits autorise ou pas les interruptions ; il contient aussi d'autres bits ayant un rôle important

cli(); désactive les interruptions

En sauvegardant SREG, en désactivant les interruptions, puis en restaurant SREG, on peut travailler "proprement" avec le hardware car on remet le microcontrôleur de l'Arduino dans le même état qu'on l'a trouvé.

 

En effet, si les lignes de code

*reg &= ~bitmask

et

*reg |= bitmask

ne font qu'une seule ligne de C++, elles sont compilées en assembleur par plusieurs instructions qui se suivent

Comme l'a dit @Kachidoki : ce ne sont donc pas des fonctions "atomiques" qui s'execute d'un seul bloc.

C'est le "piège" du C++ : sa syntaxe peut être très compacte mais une petite ligne de C++ peut correspondre à un nombre d'instructions assembleur plus ou moins nombreuses. Dans certains cas, le C++ peut donner l'impression à tord d'avoir optimisé un programme en écrivant un code C++ plus court... qui aboutira au final à un code assembleur de taille identique  Là où c'est moins drôle, c'est que le C++ peut parfois faire passer pour atomique une instruction qui ne l'est pas.

 

Il est donc possible qu'une interruption interviennent en plein milieu de la série d'instruction assembleur correspondant à *reg &= ~bitmask ou *reg |= bitmask ; si le code de cette interruption modifie aussi les registres utilisés par le code on se retrouve avec un bug assez difficile à trouver.

Bref, l'optimisation faite par l'auteur de URTouch a été un peu trop loin 

 

Chose intétessante : la librairie que j'utilise pour l'afficheur ILI9341 utilise une autre librairie optimisée pour lire et écrire rapidement sur les broches : FastPin, qui est assez réputée.

https://github.com/JakesMD/FastPin/blob/main/src/FastPin.cpp

Voyons le code de FastPin :

#include "FastPin.h"
 
FastWritePin::FastWritePin(byte pin) { _pin = pin; }
 
void FastWritePin::begin(bool initialVal) {
    pinMode(_pin, OUTPUT);
    digitalWrite(_pin, initialVal);  // Turns off PWM timers.
 
    byte port = digitalPinToPort(_pin);
    _bitMask = digitalPinToBitMask(_pin);
    _outputRegister = port != NOT_A_PIN ? portOutputRegister(port) : NULL;
}
 
void FastWritePin::write(bool val) {
    if (_outputRegister == NULL) return;
 
    byte oldSREG = SREG;
    cli();
 
    if (val == LOW) {
        *_outputRegister &= ~_bitMask;
    } else {
        *_outputRegister |= _bitMask;
    }
 
    SREG = oldSREG;
}
 
FastReadPin::FastReadPin(byte pin) { _pin = pin; }
 
void FastReadPin::begin(bool pullup) {
    if (pullup) {
        pinMode(_pin, INPUT_PULLUP);
    } else {
        pinMode(_pin, INPUT);
    }
    digitalRead(_pin);  // Turns off PWM timers.
 
    byte port = digitalPinToPort(_pin);
    _bitMask = digitalPinToBitMask(_pin);
    _inputRegister = port != NOT_A_PIN ? portInputRegister(port) : NULL;
}
 
bool FastReadPin::read() { return (_inputRegister != NULL && *_inputRegister & _bitMask) ? HIGH : LOW; }

Même si FastPin est plus rapide que digitalWrite(), on voit bien que FastPin procéde prudemment en sauvegardant SREG, en désactivant les interruptions, et en restorant SREG.

 

A bientôt

[Kachidoki]

C'est un peu mon métier. 

C'est comme lorsque tu utilises des RTOS, il ne faut jamais négliger les mutex d'accès à une ressource partagée, et parfois même ne pas oublier de sauvegarder le contexte de la ressource (registres spécifiques utilisés par un périphérique partagé). Je ne parle même pas des sauvegardes de contexte étendu lorsqu'on utilise des tâches qui ont besoin d'une FPU par exemple (ça concerne plutôt les Cortex). C'est le type de code qui ne s'invente pas, il faut savoir que ça existe et comprendre pourquoi. Si on réfléchi à une fonction "unitairement", il est facile d'oublier de se protéger des accès concurrents.

Bien entendu, on limite toujours les sections critiques au juste nécessaire, sinon ça créé du jitter sur le traitement des interruptions.

Et ne pas oublier de se protéger contre les sections critiques imbriqués.  Une fonction qui créé une section critique qui appelle une autre fonction qui elle-même créé une section critique. Si le nesting n'est pas géré, la première qui va libérer les interruptions va casser la section critique parente.

Ce genre de protection manque souvent dans les codes développés par des personnes qui ne sont pas forcément du métier, et on se retrouve facilement avec des libs communautaires bancales. Souvent ce genre de code marchotte dans certaines conditions, puis on ajoute une ligne de programme qui n'a rien à voir, ça change les timings de traitement et plus rien de fonctionne.

[electroremy]

Il y a 3 heures, Kachidoki a dit :

Ce genre de protection manque souvent dans les codes développés par des personnes qui ne sont pas forcément du métier, et on se retrouve facilement avec des libs communautaires bancales. Souvent ce genre de code marchotte dans certaines conditions, puis on ajoute une ligne de programme qui n'a rien à voir, ça change les timings de traitement et plus rien de fonctionne.

C'est exactement ça 

Avec l'Arduino on peut avoir des forks de lib qui fonctionnent très bien mais dès qu'on s'écarte un peu trop du cas d'usage classique ça ne fonctionne plus.

C'est parfois cruel, l'Arduino permet à des non-spécialistes de réaliser facilement un circuit programmable embarqué, et paf, d'un seul coup ils se retrouvent bloqués par un problème ayant un niveau de complexité beaucoup plus élevé, qui demande de mettre les mains dans le cambouis et faire du reverse engineering quand la doc ne suffit pas.

La courbe d'apprentissage est assez raide 

Cependant, certains sites sont très bien faits, comme https://www.locoduino.org/spip.php?rubrique14 qui aborde des notions assez pointue de façon plutôt accessible.

 

 

Modifié (le) il y a 15 heures par electroremy