SPI master/slave, encoder and screen: slave/master not communicating (resolved)

[gregoirem]

Hello,

I am starting with arduino nano and am trying to get an arduino to read value from an encoder and print it on a screen. Ultimately, I want to have 2 encoders, one on each axis of my telescope and do something with it. But first I need to be able to read data from the encoder.

I have a sketch which works, but it misses steps. I tested it by puting the encoder in place on the telescope, loking through it, tturning and coming back to the same spot...the value azTripodCount should be the same...but it's not. If I turn in one direction only, it'll be too high, if I turn in the other direction, it'll be too low, if I turn both ways, it varies.

I think I am missing step because my arduino board is doing too much with the screen.

#include <avr/interrupt.h>   // Needed to use interrupts    
#include <Adafruit_GFX.h>    // Core graphics library
#include <Adafruit_ST7735.h> // Hardware-specific library for ST7735

#define TFT_CS        10
#define TFT_RST        8
#define TFT_DC         9

Adafruit_ST7735 tft = Adafruit_ST7735(TFT_CS, TFT_DC, TFT_RST);

volatile long altTripodCount = 0; //This variable will increase or decrease depending on the rotation of encoder
volatile long azTripodCount = 0; //For now, I use only azimuth, I will see later to get altitude and azimuth

volatile unsigned int printNumber = 0;
static unsigned int printInterval = 5000;

const unsigned int gearRatio = 144;
const float pi = 3.14159265;
const unsigned int stepsPerRevEncoder = 1200;
const unsigned int maxCount = gearRatio * stepsPerRevEncoder;


void setup() {  
  //Setup screen
  tft.initR(INITR_BLACKTAB);      // Init ST7735S chip, black tab
  tft.fillScreen(ST77XX_BLACK);
  tft.setTextColor(ST77XX_RED);

  //Setup interupts
  DDRD &= B11000011;                     //Configure PORTD pin 2 to 5 as inputs  // 2:green alt; 3:green az; 4: white alt; ,5: white az
  PORTD |= B00111100;                    //Activate pull-ups in PORTD pin 2 to 5
  EICRA |= (1 << ISC01 | 1 << ISC00);    // set INT0 to trigger on rising edge
  EIMSK |= (1 << INT0);                  // Turns on INT0 interruption
  }
  
  void loop() {
    // Write out to serial monitor the values
    printAltAz();

    // Keep track of changes
    previousAzTripodCount = azTripodCount;
    ++printNumber;
  
    delay(5000);
  
  }


ISR(INT0_vect)
{
  // Activated if channel A changing state
  // Check channel B to determine the direction
//    if(((PIND & (1<<PD2))<<2) ^ (PIND & ((1<<PD4)))) {
//      --azTripodCount;
//    }else{
//      ++azTripodCount;
//    }

// Or same thing faster
   switch (PIND & 20) {//20 = b0010100
      case 20:
        ++azTripodCount;
        break;
      case 0:
        ++azTripodCount;
        break;
      default:
        --azTripodCount;
  }
}

  void printAltAz() {
  if (azTripodCount > maxCount){
  azTripodCount = azTripodCount - maxCount;
  }
  else if (azTripodCount < 0){
  azTripodCount = azTripodCount + maxCount;
  }
  
  tft.setCursor(0, 30);
  tft.fillRect(0,30,72,80,ST77XX_BLACK); //5 wide, 7 high, 1 in between
  tft.setCursor(0, 30);
  tft.setTextColor(ST77XX_RED);
  tft.setTextSize(2);
  tft.println(altTripodCount);
  tft.println(azTripodCount);
  tft.setTextSize(2);
  tft.println(toRad(altTripodCount));
  tft.println(toRad(azTripodCount));
  tft.println(printNumber);
  }

  float toRad(long count){
  return count*2*pi/maxCount;
  }

As you can see, i tried a few tricks to make it faster, but it still misses steps. Also those tricks might not be as clever as I thought, i'm only a beginner :-\

So I tried to hook up 2 arduinos: the master would deal with the screen and anything else I will add later, even if it's slow it's ok. And when it needs to use the value of the encoder, it would get it from the slave.

The master code:

#include <SPI.h>  
#include <Adafruit_GFX.h>    // Core graphics library
#include <Adafruit_ST7735.h> // Hardware-specific library for ST7735

#define TFT_CS        12
#define TFT_RST        8
#define TFT_DC         9

Adafruit_ST7735 tft = Adafruit_ST7735(TFT_CS, TFT_DC, TFT_RST);

volatile long altTripodCount = 0; //This variable will increase or decrease depending on the rotation of encoder
volatile long azTripodCount = 0;
long previousAzTripodCount = 1;

volatile unsigned int printNumber = 0;
static unsigned int printInterval = 500;

const unsigned int gearRatio = 144;
const float pi = 3.14159265;
const unsigned int stepsPerRevEncoder = 1200;
const unsigned int maxCount = gearRatio * stepsPerRevEncoder;


void setup() { 
  Serial.begin(115200);
  //Setup screen
  tft.initR(INITR_BLACKTAB);      // Init ST7735S chip, black tab
  tft.fillScreen(ST77XX_BLACK);
  tft.setTextColor(ST77XX_RED);

   //Set SlaveSelection pin as output.
  pinMode(SS, OUTPUT);
  //and Make it HIGH to prevent to start communication right away
  digitalWrite(SS, HIGH);
  //Start the SPI communication.
  SPI.begin();
  }
 
  
 void loop() {

  //Receive azValue from the slave (I actually just try to send a 0 or 1 atm, I'll see later to send a long)

//Enable slave arduino with setting the SlaveSelection pin to 0Volt
  digitalWrite(SS, LOW);
  // Wait for a moment 
  delay(10);
  //We sent the data here and wait for the response from device
  char receivedValue = SPI.transfer(1); // Sending 1, but it could be anything else
  //And then write the answer to the serial port
  Serial.println("received_value");
  Serial.println(receivedValue);
  //Disable slave arduino with setting the SlaveSelection pin to 5Volt
  digitalWrite(SS, HIGH);

    // Write out to serial monitor the values
    printAltAz();

    // Keep track of changes
    previousAzTripodCount = azTripodCount;
    ++printNumber;
  
    delay(printInterval);
  
  }

  void printAltAz() {
  if (azTripodCount > maxCount){
  azTripodCount = azTripodCount - maxCount;
  }
  else if (azTripodCount < 0){
  azTripodCount = azTripodCount + maxCount;
  }
  
  tft.setCursor(0, 30);
  tft.fillRect(0,30,72,80,ST77XX_BLACK); //5 wide, 7 high, 1 in between
  tft.setCursor(0, 30);
  tft.setTextColor(ST77XX_RED);
  tft.setTextSize(2);
  tft.println(altTripodCount);
  tft.println(azTripodCount);
  tft.setTextSize(2);
  tft.println(toRad(altTripodCount));
  tft.println(toRad(azTripodCount));
  tft.println(printNumber);
  Serial.println(printNumber);
  Serial.println(toRad(azTripodCount));
  }

  float toRad(long count){
  return count*2*pi/maxCount;
  }

The slave code:

#include <SPI.h>

volatile long altTripodCount = 0; //This variable will increase or decrease depending on the rotation of encoder
volatile long azTripodCount = 0;

volatile unsigned int printNumber = 0;

char i = 0;

void setup() {
  Serial.begin(115200);
  DDRD &= B11000011;  //Configure PORTD pin 2 to 5 as inputs  // 2:green alt; 3:green az; 4: white alt; ,5: white az
  PORTD |= B00111100;  //Activate pull-ups in PORTD pin 2 to 5
  EICRA |= (1 << ISC01 | 1 << ISC00); // set INT0 to trigger on rising edge
  EIMSK |= (1 << INT0);     // Turns on INT0 interruption

  pinMode(SS,INPUT);      // Set SS as input
  pinMode(SCK, OUTPUT);
  pinMode(MOSI,OUTPUT);   // Set MOSI as output
  pinMode(MISO,INPUT);    // Set MISO as input

  // SPCR - SPI Control Register
  // According to struct table we enable the SPI and Interface
  SPCR  |= 0b11000000;
  // SPSR - SPI Status Register
  SPSR  |= 0x00;
  
}

void loop(){
  delay(1000);
}



ISR (SPI_STC_vect)                        //Inerrrput routine function 
{
    Serial.println("slave interupted");
    //Here we read the SPI lines
    //This line will check data for every ASCII codes for 8-bit received data
    //SPDR -> SPI Data Read bit
    SPDR = i;
    i ++;
    if ( i > 255) 
      i = 0;
    while(!(SPSR & (1 << SPIF)));
    //Load the received data to the variable
    char received = SPDR;
    //And send it to the serial communication bus
    Serial.println(received);
}


ISR(INT0_vect)
{ 
   Serial.println("slave interupted encoder");
   switch (PIND & 20) {//20 = b0010100
      case 20:
        ++azTripodCount;
        break;
      case 0://20 = b0000000
        ++azTripodCount;
        break;
      default:
        --azTripodCount;
  }
}

I moved the CS pin from my screen to the pin 12, because pin 10 is the SlaveSelect pin. The screen doesn't work anymore. I used Serial debuging to see what's going on, and there are a few more porblems.The encoder interupt still works on the slave but the master receives only ⸮ .

Cheers,

Gregoire

[Klaus_K]

I have not looked through your entire code but noticed a few things you should look at.

• Stop using delay(). Have a look at the following example to learn how to control timing without delay().

File -> Examples -> 02.Digital -> BlinkWithoutDelay

• Break up functions that use external interfaces like your TFT. A simple way to do this is to create a state machine with switch-case. Every time you call the function one case is executed and a state variable is incremented. So instead of printing the entire screen only one line is printed every time. Place timing code inside the functions that need a lot of time e.g., TFT and test it first. Now you can call the function as often as you can, but it will only execute once every 1ms or 100ms ...

• Ensure your loop is running as often as possible. That is why it is called loop. Do not program your Arduino like a human solving one task after another. Your Arduino is can do many things all at the same time. Never stop for anything (no delay, no while loops, waiting for status changes).

• Move your code into functions and call them from loop. This will make it easier to keep track of things. Each task function should do one thing. e.g., read buttons, print to TFT, read sensor, ...

• Low level code should always be inside a separate function. Do not mix high level code and low-level code. That makes your code hard to read.

[gregoirem]

Hello Klaus, and thanks for your input.

I have cleaned a bit my single-arduino code, separating low-level and high level code, implemented a refresh by line screen function (only a skeleton function to be fleshed up later, but that is not my priority, although tanks for the idea).

So here is the single code, and I'd have a few questions:

#include <avr/interrupt.h>   // Needed to use interrupts    
#include <Adafruit_GFX.h>    // Core graphics library
#include <Adafruit_ST7735.h> // Hardware-specific library for ST7735


// Define screen pins in use
#define TFT_CS         7
#define TFT_RST        8
#define TFT_DC         9

Adafruit_ST7735 tft = Adafruit_ST7735(TFT_CS, TFT_DC, TFT_RST);

// Behaviour parameters
static unsigned int printInterval = 5000;

// Hardware description parameters
const long gearRatio = 144;  // This is the gearratio between the encoder and the axis
const float pi = 3.14159265;
const long stepsPerRevEncoder = 300;  // Number of steps per revolution of the encoder
const long maxCount = gearRatio * stepsPerRevEncoder;  //  Max count of encoder step before returning to 0

// Variables
volatile long altEncoderCount = 0;  // This variable will increase or decrease depending on the rotation of encoder
long previousAltEncoderCount = 1;

// Screen parameters
const int pixel_width = 10;
const int pixel_height = 14;
const int pixel_spacing = 2;



void setup() {
  Serial.begin(9600);
  Serial.println(gearRatio);
  Serial.println(stepsPerRevEncoder);
  Serial.println(maxCount);
  // Setup screen
  tft.initR(INITR_BLACKTAB); // Init ST7735S chip, black tab
  tft.fillScreen(ST77XX_BLACK);
  tft.setTextSize(2);
    tft.setTextColor(ST77XX_RED);

  // Configure pins and activate interupt (using register rather than interrupt library to save memory space on the chip)

  DDRD &= B11101011;  //Configure PORTD pin 2 and 4as inputs  // 2:green alt; 4: white alt;
  PORTD |= B00010100;  //Activate pull-ups in PORTD pin 2 and 4
  EICRA |= (1 << ISC01 | 1 << ISC00); // set INT0 to trigger on rising edge
  EIMSK |= (1 << INT0);     // Turns on INT0 interruption
  }
  
void loop() {  
  if(altEncoderCount != previousAltEncoderCount)

    // Keep track of changes
    previousAltEncoderCount = altEncoderCount;
  
    // refresh altitude information on screen
    refreshScreen(1);
 
  delay(printInterval);
  
}


ISR(INT0_vect)
{
  // Activated if channel A of the altitude encoder (pin 2) changes state
  // Check channel B of the altitude encoder to determine the direction (pin 4)
  
   switch (PIND & 20) {//20 = b0010100, used to read pin 2 and 4
      case 20:
        ++altEncoderCount;
        break;
      case 0:
        ++altEncoderCount;
        break;
      default:
        --altEncoderCount;
  }
}

void refreshScreen(int caseNumber) {
  switch(caseNumber){
    case 1: // altitude encoder value
      altEncoderCount = checkEncoderCount(altEncoderCount);
      tft.setCursor(pixel_width, pixel_height);
      tft.fillRect(pixel_width, pixel_height, 5*pixel_width+4*pixel_spacing,pixel_height, ST77XX_BLUE);
      tft.println(altEncoderCount);
      tft.setCursor(pixel_width, 2*pixel_height + pixel_spacing);
      tft.fillRect(pixel_width, 2*pixel_height + pixel_spacing, 5*pixel_width+4*pixel_spacing, pixel_height,ST77XX_BLUE);
      tft.println(count2Rad(altEncoderCount));
      break;
      
    case 2: // provision for more cases
      break;
  }
}

float count2Rad(long count){
  return count*2*pi/maxCount;
}

long checkEncoderCount(long encoderCount){
  if (encoderCount > maxCount){
    return encoderCount - maxCount;
   }
  else if (encoderCount < 0){
    return encoderCount + maxCount;
  }
  else{
    return encoderCount;
  }
}

First, regarding the use of delay. I initially implemented something similar as the BlinkWithoutDelay strategy you pointed out, but I thought that the arduino would spend its time evaluating if statements. I know that in theory the interupt calls should be triggered anyway, but in reality it doesn't happen, some steps are missed. Or am I rong in thinking than my encoder is faultless and the missed steps might come from the encoder and not from missed interupts? How could I test that?

Another two things I don't understand:
I said I moved the pin 10 to pin 12 for the tft screen to free the SlaveSelect pin...it didn't work. I then moved it to the pin 7, and it works. Why is that?

Edit: I found the solution to what follows:
I had put wrongly my encoder as having 1200 steps per revolution, but it actually has 600, and as I trigger only on rising edge, that give basically 300 trigger per encoder revolution. I had then 1200 * 144 (gear ratio between the encoder and the measured axis) steps per axis revolution, and because I stored it in an unsigned int, it was equal to 41728. This is I believe 172800-65536-65536. Fair enough, my bad. But when I store it in a long, it does the same... I don't understand that. In the end, I'm using only 300 triggers per encoder revolution, giving my 144*300 = 43200 steps per axis revolution, so an int does the job, but I'd like to see what Im missing there.

Edit: that is because all need to be long to result in a long, pb solved

Thanks again for any light anyone can shed on those problems, ans I will keep trying to split that into two arduinos (a slave that monitors the encoder and share this information with the master, the master doing all the rest) and post updates as I go.

[gregoirem]

I now managed to get things working. I have started back at the basics, with an example of simple SPI communications (post here: Simple SPI example not working (resolved) - Networking, Protocols, and Devices - Arduino Forum)

I have posted the working version of the code above here in case it can help someone else.

It seems that in order to comunicate between slave and master, you need to use the designated pins (with arduino Nano, MOSI, MISO, and SCK on pins 11 to 13). The slave select pin must be 10 on slave boards, but can be moved to another pin on the master one. I have left the first slave select on pin 10 on the master anyway, but will need to use another one for the second slave I will add later. The TFT screen can be set to use any pins. So I moved it to pins 5 to 9 to free up pins 10 to 13.

I also restructured the code, to have only high level code in the loop routine, every low level code being in it's own routine.

A few more explanation about the code are within it in comments.

This is still work in progress, I now have to add another slave with the second encoder, and a few rotary buttons, but I am pleased to say that no steps are missed anymore! Having a dedicated slave board basically only dealing with encoder and a few calls from master solved the problem (missed encoder interupts because the board was too busy dealing with the screen).

Master code:

#include <avr/interrupt.h>   // Needed to use interrupts    
#include <Adafruit_GFX.h>    // Core graphics library
#include <Adafruit_ST7735.h> // Hardware-specific library for ST7735
#include <SPI.h>


// Define screen pins in use
#define TFT_SCK        5
#define TFT_SDA        6
#define TFT_CS         7
#define TFT_RST        8
#define TFT_DC         9

Adafruit_ST7735 tft = Adafruit_ST7735(TFT_CS, TFT_DC, TFT_SDA, TFT_SCK, TFT_RST);

// Behaviour parameters
static unsigned int printInterval = 1000;

// Hardware description parameters
const long gearRatio = 144;  // This is the gearratio between the encoder and the axis
const float pi = 3.14159265;
const long stepsPerRevEncoder = 600;  // Number of steps per revolution of the encoder
const long maxCount = gearRatio * stepsPerRevEncoder;  //  Max count of encoder step before returning to 0

// Variables
volatile long altEncoderCount = 0;  // This variable will increase or decrease depending on the rotation of encoder
long previousAltEncoderCount = 1;

// Screen parameters
const int pixel_width = 10;
const int pixel_height = 14;
const int pixel_spacing = 2;

volatile byte byte_1;
volatile byte byte_2;
volatile byte byte_3;

// Setup
void setup() { 
  //Setup screen
  tft.initR(INITR_BLACKTAB);      // Init ST7735S chip, black tab
  tft.fillScreen(ST77XX_BLACK);
  tft.setTextColor(ST77XX_RED);

  // Put SCK, MOSI, SS pins into output mode
  // also put SCK, MOSI into LOW state, and SS into HIGH state.
  // Then put SPI hardware into Master mode and turn SPI on
  SPI.begin ();
 }  // end of setip
 
 // Main loop: WIP
 void loop() {
  // Update encoder value from dedicated slave
  getValueFromEncoder();

  if(altEncoderCount != previousAltEncoderCount){

  // Keep track of changes
  previousAltEncoderCount = altEncoderCount;

  // Refresh altitude information on screen
  refreshScreen(1);
  }

  delay(printInterval); // No need to overload the slave, this will be replace by a time controled entry condition in final code  
} // end of loop

// Get value from slave routine
void getValueFromEncoder(){
  // Enable Slave Select
  digitalWrite(SS, LOW);    // SS is pin 10
  
  // Get value from encoder slave
  // Only need to transfer 3 bytes, as the slave will only send a positive long, and the
  // value will be way below 16million (hence MSB will be 00000000, no need to transfer it)
  SPI.transfer (0); // This will receive nothin or last bit of previous call, we don't need to save it, we
                    // just need to make the slave send us the first bit, which we will receive in next call
  delay(10);
  byte_1= SPI.transfer (1); // This will receive what was send by slave after last call: first byte
  delay(10);
  byte_2 = SPI.transfer (2);// This will receive what was send by slave after last call: second byte
  delay(10);
  byte_3 = SPI.transfer (3);// This will receive what was send by slave after last call: third byte
  
  // Disable Slave Select
  digitalWrite(SS, HIGH);

  // Reconstruct the transferred long value
  altEncoderCount = (long)byte_1 << 16 | (long)byte_2 << 8 | byte_3;
} // end of get value form slave routine

// Refresh screen routine
void refreshScreen(int caseNumber) {
  switch(caseNumber){
    case 1: // altitude encoder value
      tft.setCursor(pixel_width, pixel_height);
      tft.fillRect(pixel_width, pixel_height, 5*pixel_width+4*pixel_spacing,pixel_height, ST77XX_BLACK);
      tft.println(altEncoderCount);
      tft.setCursor(pixel_width, 2*pixel_height + pixel_spacing);
      tft.fillRect(pixel_width, 2*pixel_height + pixel_spacing, 5*pixel_width+4*pixel_spacing, pixel_height,ST77XX_BLACK);
      tft.println(count2Rad(altEncoderCount));
      break;
      
    case 2: // provision for more cases
      break;
  }
} // end of refresh screen routine

// Transform encoder count to radian value routine
float count2Rad(long count){
  return count*2*pi/maxCount;
} // end of transform encoder count to radian value routine

Slave code:

// #include <avr/io.h>
#include <SPI.h>

volatile long altEncoderCount = 0; // This variable will increase or decrease depending on the rotation of encoder
volatile byte masterMessage;
volatile long valueToSend;

// Hardware description parameters
const long gearRatio = 144;  // This is the gearratio between the encoder and the axis
const long stepsPerRevEncoder = 600;  // Number of steps per revolution of the encoder
const long maxCount = gearRatio * stepsPerRevEncoder;  //  Max count of encoder step before returning to 0

// Setup
void setup() {
  // Configure pins and activate interupt, using registers

  DDRD  &= 0b11101011;  //Configure PORTD pin 2 and 4 as inputs  // 2:green alt; 4: white alt;
  PORTD |= 0b00010100;  //Activate pull-ups in PORTD pin 2 and 4
  EICRA |= 0b00000011;  // set INT0 to trigger on rising edge only
  EIMSK |= (1 << INT0);     // Turns on INT0 interruption

  pinMode(MISO,OUTPUT);    // Set MISO as input

  // SPCR - SPI Control Register
  // According to struct table we enable the SPI and Interface
  SPCR |= bit (SPE);
  // SPSR - SPI Status Register
  //SPSR  |= 0x00; // Clear SPI Status Register, not too sure if needed, but it works with it...
  // Attach interrupt
  SPI.attachInterrupt();
}  // end of setup


// SPI interrupt routine
ISR (SPI_STC_vect)
{
  
  masterMessage = SPDR;
  switch(masterMessage){
  case 0:
    altEncoderCount = formatEncoderCount(altEncoderCount); 
    valueToSend = altEncoderCount; // This ensure same encoder value will be used in subsequent master calls
                                   // even if encoder turns in between
    SPDR = (valueToSend & 0xFF0000 ) >> 16;
    break;
  case 1:
    SPDR = (valueToSend & 0xFF00 ) >> 8;
    break;
  case 2:
    SPDR = valueToSend & 0xFF;
    break;
  case 3:
    break; // Do nothing, this call from master just receives what was sent back after it's last call
  }
}  // end of interrupt routine SPI_STC_vect

// Main loop - do nothing
void loop (void)
{
}  // end of loop

// Encoder interrupt routine
ISR(INT0_vect)
{
  // Activated if channel A of the altitude encoder (pin 2) changes state
  // Check channel B of the altitude encoder to determine the direction (pin 4)
  
   switch (PIND & 20) {//20 = b0010100, used to read pin 2 and 4
      case 20:
        ++altEncoderCount;
        break;
      case 0:
        ++altEncoderCount;
        break;
      default:
        --altEncoderCount;
  }
}  // end of encoder interrupt routine

// Encoder value formatting routine
long formatEncoderCount(long encoderCount){
  // This format the encoder value to it's value between 0 and maxCount (corresponding to a full rotation of the axis)
  if (encoderCount > maxCount){
    return encoderCount - maxCount;
   }
  else if (encoderCount < 0){
    return encoderCount + maxCount;
  }
  else{
    return encoderCount;
  }
}  // end of encoder value formatting routine

And obviously, I still take any advice or comment if you think anything is still a bit dodgy

Cheers for the help