Probleme with SPI transaction

Hello

I made an audio amplifier, witch use an arduino nano for driving a volume digital pot (PGA2310), a LCD display, an a digital encodeur with push button.
A long press (>3 s) on the encodeur push button, the ampli turn on, or turn off. A short press on it change the input line. Turn right or left the encodeur increase or decrease the volume.
The LCD display the input fonction and gain.
I use attachinterupt for the encodeur, and SPI for the digital pot.
The problem here is when the the void digitalpot() for SPI transaction is called, the program freeze...
If the void digitalpot() is disabled, the program run normaly (but of course no instruction for the digital pot).
I use in the past a similar program for an another project, with a standalone Atmega328P, and this one run normaly.
Someone can detect an error in this program?

Thanks

#include <SPI.h>
#include <LiquidCrystal.h>
LiquidCrystal lcd(A0, A1, A2, A3, A4, A5);

const int IN3 = 6;    // pin commande relai IN3 (1 sur schéma)
const int IN2 = 5;    // pin commande relai IN2
const int IN1 = 4;    // pin commande relai IN1 (3 sur schéma)
const int INBLT = 7;  // pin commande relai IN1
const int CodA = 3;   // pin A encodeur
const int CodB = 2;   // pin B encodeur
const int BS = 10;    // pin bouton select encodeur
const int ChipSelectPin = 9;     // PIN SELECT PGA2310
const int ALIM = 8;   // pin controle ALIM
int npot = 0 ;
int gain = 0;
long unsigned tempsA ;
long unsigned tempsB;  //codeur B (ON/OFF ou chgt affichage)
bool etatA = 0 ;
bool dernierEtatA = 0 ;
bool etatBS = 0 ;
bool dernieretatBS = 0 ;
boolean relai = 0;      // état du relais de commande de l'alim
boolean relaiIN1 = 0;   // état du relais IN1
boolean relaiIN2 = 0;   // état du relais IN2
boolean relaiIN3 = 0;   // état du relais IN3
boolean relaiBLT = 0;   // état du relais IN4
byte affich ;           // défini l'entrée d'affichage
enum : byte {ampli_OFF, ampli_ON} etat_ampli;


void setup() {

  etat_ampli = ampli_OFF;
  SPI.begin();
  lcd.begin(16, 2);
  lcd.clear();
  pinMode(ChipSelectPin, OUTPUT);
  pinMode(BS, INPUT);
  pinMode(CodA, INPUT);
  pinMode(CodB, INPUT);
  pinMode(ALIM, OUTPUT);
  pinMode(IN1, OUTPUT);
  pinMode(IN2, OUTPUT);
  pinMode(IN3, OUTPUT);
  pinMode(INBLT, OUTPUT);

}

void loop() {

  attachInterrupt(digitalPinToInterrupt(CodA), codeur, CHANGE);

  int gain = (31.5 - (0.5 * (255 - npot))); // transpose valeur niveau volume binaire en dB

  etatBS = digitalRead(BS);
  if ((etatBS) == 1 && (dernieretatBS) == 0) {
    tempsB = millis();
  }
  if ((etatBS) == 0 && (dernieretatBS) == 1 && (relai == 1)) {
    affich++; if (affich > 3) {
      affich = 0;
    }
  }
  delay(20);

  if ((etatBS == HIGH) && (dernieretatBS == HIGH)) { //teste si bouton resté enfoncé
    if ((millis() - tempsB) > 3000) { //teste durée d'appui
      lcd.clear();
      switch (etat_ampli) {
        case ampli_OFF:
          while (etatBS == 1) {
            etatBS = digitalRead(BS);
            lcd.setCursor(0, 0);
            lcd.print("    ARGO ON     ");
            lcd.setCursor(64, 0);
            lcd.print("                ");
          }
          delay(1000);
          relai = 1; digitalWrite(ALIM, relai);
          etat_ampli = ampli_ON;

          break;
        case ampli_ON:
          while (etatBS == 1) {
            etatBS = digitalRead(BS);
            lcd.setCursor(0, 0);
            lcd.print("    ARGO OFF    ");
            lcd.setCursor(64, 0);
            lcd.print("                ");
          }
          delay(1000);
          relai = 0; digitalWrite(ALIM, relai);
          etat_ampli = ampli_OFF;
          break;
          affich--; if (affich < 0) {
            affich = 0;
          }
      }
    }
  }
  dernieretatBS = etatBS;



  if (relai == 0) {
    lcd.clear();
    relaiIN1 = 0;
    relaiIN2 = 0;
    relaiIN3 = 0;
    relaiBLT = 0;
    digitalWrite(IN1, relaiIN1);
    digitalWrite(IN2, relaiIN2);
    digitalWrite(IN3, relaiIN3);
    digitalWrite(INBLT, relaiBLT);
  }
  if (relai == 1) {
    if (affich == 0) {
      lcd.setCursor(0, 0);
      lcd.print("    input 1     ");
      char tampon[16] = "";
      sprintf(tampon, "%s%3d%s", "  gain ", gain, " dB   ");
      lcd.setCursor(64, 0);
      lcd.print(tampon);
      relaiBLT = 0; digitalWrite(INBLT, relaiBLT);
      relaiIN1 = 1; digitalWrite(IN1, relaiIN1);
      relaiIN2 = 0; digitalWrite(IN2, relaiIN2);
      relaiIN3 = 0; digitalWrite(IN3, relaiIN3);
    }
    if (affich == 1) {
      lcd.setCursor(0, 0);
      lcd.print("    input 2     ");
      lcd.setCursor(64, 0);
      char tampon[16] = "";
      sprintf(tampon, "%s%3d%s", "  gain ", gain, " dB   ");
      lcd.print(tampon);
      relaiIN1 = 0; digitalWrite(IN1, relaiIN1);
      relaiIN2 = 1; digitalWrite(IN2, relaiIN2);
      relaiIN3 = 0; digitalWrite(IN3, relaiIN3);
      relaiBLT = 0; digitalWrite(INBLT, relaiBLT);

    }
    if (affich == 2) {
      lcd.setCursor(0, 0);
      lcd.print("    input 3     ");
      lcd.setCursor(64, 0);
      char tampon[16] = "";
      sprintf(tampon, "%s%3d%s", "  gain ", gain, " dB   ");
      lcd.print(tampon);
      relaiIN1 = 0; digitalWrite(IN1, relaiIN1);
      relaiIN2 = 0; digitalWrite(IN2, relaiIN2);
      relaiIN3 = 1; digitalWrite(IN3, relaiIN3);
      relaiBLT = 0; digitalWrite(INBLT, relaiBLT);
    }
    if (affich == 3) {
      lcd.setCursor(0, 0);
      lcd.print("   Bluetooth    ");
      lcd.setCursor(64, 0);
      char tampon[16] = "";
      sprintf(tampon, "%s%3d%s", "  gain ", gain, " dB   ");
      lcd.print(tampon);
      relaiIN1 = 0; digitalWrite(IN1, relaiIN1);
      relaiIN2 = 0; digitalWrite(IN2, relaiIN2);
      relaiIN3 = 0; digitalWrite(IN3, relaiIN3);
      relaiBLT = 1; digitalWrite(INBLT, relaiBLT);
    }
  }

}


void codeur() {
  if (relai == 1) {
    etatA = digitalRead(CodA);
    if ( etatA != dernierEtatA ) {
      if ( abs(millis() - tempsA) > 20 ) { // anti rebond

        if (digitalRead(CodB) != dernierEtatA) { // Si B different de l'ancien état de A alors sens anti horaire
          npot--; if (npot < 0) {
            npot = 0;
          }
        }

        else { // sinon sens horaire

          npot++; if (npot > 256) {
            npot = 256; // incrémentation niveau pot
          }

        }
        tempsA = millis(); // memorisation du temps pour A

      }
      dernierEtatA = etatA ;// memorisation de l'état de A
      digitalPot(npot);
      delay(10);

    }

  }

}

void digitalPot(int npot) {
  SPI.beginTransaction(SPISettings(8000000, MSBFIRST, SPI_MODE0));
  digitalWrite(ChipSelectPin, LOW);
  SPI.transfer(npot);//pot left
  SPI.transfer(npot);//pot right
  digitalWrite(ChipSelectPin, HIGH);
  SPI.endTransaction();

}

You are calling digitalPot() from codeur() which is an ISR. In an ISR interrupts are disabled, but interrupts need to be enabled in order for the SPI transactions to complete.

There are several issues with your ISR:

  1. Do not use a delay() in an ISR.
  2. npot is used in loop() and in the ISR. It should be declared with the volatile keyword and interrupts should be disabled when accessing it from loop().
  3. relai should be declared with the volatile keyword.
  4. Why are you calling attachInterrupt() at the beginning of loop()? Just call it once from setup().
  5. Do you really need an interrupt? If so, then do a very minimal amount of processing in it.

Thank you for your answer. I'm not sure to understand.
Do you mean that I must not called digitalPot() from codeur(), but from loop() ?

Yes.

I need tu use interrupt, because without the program can't read all change of the encodeur.
I transfert the attachInterrupt to the setup loop. Thanks for this.
I delete the delay in the codeur loop.
What do you mean with "volatile keyword"? How can I declared npot with a volatile keyword in the codeur loop?

I tranfer digitalPot to the loop(), but don't fix the problem. :pensive:

I knew that wouldn't fix it. It was just uneccessary.

How fast does the encoder change?

I have tested without interrupt on my last projet. It run, but if i turn quicly the encodeur only 1 of 3" position was read. It is not a big problem. I will test this program without interrupt, and I tell you...
I

For an encoder an interrupt may be necessary. However, you should understand the limitations of an interrupt. If you just want to increment and decrement the counters that is fine. However, you must disable the interrupts in loop() while reading or writing the counters because it may corrupt them if you don't.

Consider posting a schematic, not a frizzy picture. I did a quick look and you have 3 devices on the SPI bus but only one CS. If that is correct it will not work. Posting links to technical information on all the hardware devices also helps.

Hello

I'll post a schematic this evening (I'm at work for the moment), but I have only one device on the SPI bus (the PGA2310, https://www.ti.com/lit/ds/symlink/pga2310.pdf?ts=1649753672147&ref_url=https%253A%252F%252Fwww.google.com%252F). Provided that the SPI bus is only COPI (pin D11), CIPO (D12) and SCK (D13).
Do you see something else?
Thank you

Hello

I think I have find the answer of my problem: I use the CS pin of the arduino board (D10) as an input (to read the push button of the encodeur). Even if I don't use this pin for the chip select pin slave device, this pin must be declared as an OUTPUT pin, otherwise the arduino board is put in slave mode...
I have test this with a breadboard (swap the pin 10 as a output), it's seems to be OK. I need to modify my PCB and test this. I'll keep you up-to-date.

Thanks to all!

Hi

I made this modify. It's work fine! The CS pin of the controller must be an output, otherwise the SPI transfert not operate. I didn't know this.
The amplifier is not operate, but this is an other story...

Thanks to all!