SPI and ICR1 problems

Ive been working an small project to control a 10x20 LED matrix from an Arduino. For this I chose to use 4 74HC595 shifters that were controlled by SPI. This was working well, and I was able to draw images/text over the display without issue.

I then decided to include a HC-SR04 into the mix, so that the display could respond to proximity, however this is when problems started. I have been unable to get both the SPI and HC-SR04 working together, I can get either working independently. That is, when in the same program the only way is for the display to stop being updated during the scan of the HC-SR04. This causes a flickering effect on the LED matrix, which is undesirable.

The initial attempts of the integration used the Arduino pulseIn method, which times (in microseconds) from the time a signal goes high to when it goes low. The problem is that this is implemented in the libraries as a blocking call, meaning that the program is unable to update the LED matrix during the pulse. Instead, after looking at the atmega386p documentation, I realised there is the ICR1/ICP1 functionality that would allow the hardware to time the pulse width. The below code shows my attempt at utilising the ICR1 to time a pulse:

/*
 * The below code is designed to mimic the Arduino pulseIn function,
 * however using the ICR mechanism of timer1 with interrupts instead of
 * a software implementation. This means that it will only work on
 * portb, pin0 of the atmega328p, i.e. Arduino pin 8.
 *
 * The below was developed to wait for the rising edge on to be
 * detected by ICR, start timer 1 with prescaler 8, then wait for
 * falling edge and save the value of the timer into ICR1.
 */


/* 
  These variables are used within the ISR for the pulse 
  detection to track state.
 */
volatile bool icr_pulse_done = false;
volatile bool icr_pulse_error = false;
volatile uint16_t icr_pulse_value = 0;

void icr_pulse_enable(void) {
  /* initilise the state tracking variables */
  icr_pulse_error = false;
  icr_pulse_done = false;
  /* configure timer1 in normal mode */
  TCCR1A = 0;
  /* configure icr on rising edge with noise filter */
  TCCR1B = _BV(ICES1) | _BV(ICNC1);
  /* unmask icr */
  TIMSK1 = _BV(ICIE1);
}

/*
 * called when timer1 overflows, i.e. times out looking for an event
 */
ISR(TIMER1_OVF_vect) {
  /* stop timer1 */
  TCCR1B = 0;
  /* mask timer1 interrupts */
  TIMSK1 = 0;
  /* mark error status */
  icr_pulse_error = true;
}

/*
 * called when an edge change is detected by timer1, it can be 
 * called without timer1 being started.
 */
ISR(TIMER1_CAPT_vect) {
  if (TCCR1B & _BV(ICES1)) {
    /* looking for up edge */

    /* clear the timer 1 counter */
    TCNT1 = 0;
    /* enable interrupt for overflow */
    TIMSK1 |= _BV(TOIE1);
    /* start timer 1 with prescaler or 8 */
    TCCR1B = _BV(CS11) | _BV(ICNC1);
  } else {
    /* looking for down edge */

    /* stop timer1 */
    TCCR1B = 0;
    /* mask timer1 interrupts */
    TIMSK1 = 0;
    /* make cycle as complete */
    icr_pulse_done = true;
    /* divide ICR1 by 2 to convert into microseconds */
    icr_pulse_value = ICR1 / (F_CPU/1000000/8);
  }
}

The below code is a sample Arduino program code that utilise the above code to trigger a pulse on the HC-SR04 every 100 milliseconds, and write some dummy data over the SPI:

/*
 * PORTB
 *  pin5 |-> pin13 (SCK)
 *  pin3 |-> pin11 (MOSI)
 *  pin2 |-> pin10 (output/SS)
 *  pin0 |-> pin8 (ICP1, with external 10k pulldown)
 *
 * PORTD
 *  pin7 |-> pin7 (output, low to enable outputs on SN74CH595)
 *  pin6 |-> pin6 (output, trigger HC-SR04)
 *
 * Connected to: SN74HC595
 *  Arduino pin13 (SCK) connected to SN74HC595 pin11 (SRCLK)
 *  Arduino pin11 (MOSI) connected to SN74HC595 pin14 (SER)
 *  Arduino pin10 connected to SN74HC595 pin12 (RCLK)
 *  Arduino pin7 connected to SN74HC595 pin13 (OE)
 *  SN74HC595 pin10 high
 *
 * Connected to: HC-SR04
 *  Arduino pin6 (output) to HC-SR04 trigger
 *  Arduino pin8 (ICP1) to HC-SR04 echo
 */
#define PIN_TRIGGER   6
#define PIN_ECHO      8
#define PIN_SS        10
#define PIN_OE        7

void setup() {
  Serial.begin(9600);
  Serial.println("Hello World");

  /* Set MOSI, SCK and SS output, all others input */
  DDRB |= _BV(DDB3) | _BV(DDB5) | _BV(DDB2);
  /* Enable SPI, Master, set clock rate fck/16 */
  SPCR = _BV(SPE) | _BV(MSTR) | _BV(SPR0);
  /* Set trigger pin and output enable to output */
  DDRD |= _BV(DDD6) | _BV(DDD7);
  /* Set pin7 low */
  PORTD &= ~_BV(PORTD7);
}

/*
 * Writes a byte to the SPI as documented in the atmega328p manual.
 */
void sendspi(uint8_t i_data) {
  digitalWrite(PIN_SS,LOW);
  /* Transmit high byte */
  SPDR = i_data;
  /* Wait for transmission complete */
  while(!(SPSR & (1<<SPIF)))
    ;
  digitalWrite(PIN_SS,HIGH);
  _delay_us(10);
}

uint32_t next_time_scan = 0;

void loop() {
  uint32_t now = millis();

  /* send an echo pulse */
  if (now >= next_time_scan) {
    /* send a pulse every 100 milliseconds */
    next_time_scan += 100;
    /* enable icr interrupt, its looking for the rising edge */
    icr_pulse_enable();
    /* set trigger on hc-sr04 high for 10 microseconds */
    digitalWrite(PIN_TRIGGER,HIGH);
    _delay_us(10);
    digitalWrite(PIN_TRIGGER,LOW);
  }

  /* if pulse detection is complete, print the depth in cm */
  if (icr_pulse_done) {
    uint16_t cm = icr_pulse_value/58;
    Serial.print("Depth: ");
    Serial.println(cm);
    icr_pulse_done = false;
  }

  /* in case of the timer1 overflow, print an error and reset */
  if (icr_pulse_error) {
    Serial.println("Error");
    icr_pulse_error = false;
  }

  /*
   * This line causes problems:
   *   sendspi(((uint8_t*)&now)[0]);
   * It appears any data that is sent over the spi that keeps changing 
   * causes issues with the ICR1.
   * 
   * If this is replaced by a constant value, or one that does not change 
   * often then the issue is resolved. E.g. both of the following lines
   * are ok.
   *   sendspi(((uint8_t*)&now)[1]);
   */
  sendspi(((uint8_t*)&now)[0]);
}

The problem with the above code is that depending on the value written over the SPI, it can effect the value read by the ICR1. As an example, if the value written to the SPI is constant, or changes less every few milliseconds (> 250) then its ok, and the pulse width is detected correctly. However, if the value changes at high frequency then the value obtained by the ICR1 is incorrect.

For example, the below output is when the last line is sendspi(((uint8_t*)&now)[0]), i.e. the lower byte of the milliseconds since power on:

Depth: 63
Depth: 58
Depth: 63
Depth: 63
Depth: 46
Depth: 63
Depth: 63
Depth: 63
Depth: 63

But without moving the hardware, and only reprogramming it by changing the last line to sendspi(((uint8_t*)&now)[1]) the output is:

Depth: 192
Depth: 191
Depth: 192
Depth: 191
Depth: 192
Depth: 192
Depth: 191

which is the correct measurement. This byte only changes every 256 milliseconds.

I have tried looking in the atmega328p documentation but can not see any obvious reason that the use of the SPI should change the value of the ICR1. They are both connected on PORTB, but different pins, moreover, the SPI clock generator appears as a separate logical block on the processor from timer/counter1.

Interestingly, I have noticed that if any of the SPI pins (13, 11, 10) are disconnected from the 74HC595 then the issue is removed, and the pulse width is detected correctly. Could it be that the 74HC595 is feeding back interference?

"Interestingly, I have noticed that if any of the SPI pins (13, 11, 10) are disconnected from the 74HC595 then the issue is removed, and the pulse width is detected correctly. Could it be that the 74HC595 is feeding back interference? "

Doubtful. '328P has 40mA drive capability, 74HC595's 1uA inputs will not affect those.
And even if you had '595 outputs connected to the SPI outputs, that would just result in output drive transistors fighting each other, no logic impact to the rest of the '328 functions.

You don't need the delay in this function
void sendspi(uint8_t i_data) {
The shift registers are all set after the 8 SRCLK clocks and then RCLK going low to high to move the data to their output latches.
Do you have a 0.1uF cap from each shift register Vcc pin to Gnd? (And Not on any control line)

What speed are you using with Serial.begin()? 115200 or something slow like 9600? I'd go with faster.

Probably switching the '595 causes noise on power lines and it makes the input capture read edge that does not exist or makes a glitch in the HC-SR04 to give wrong output (more likely). Try to add decoupling, reduce load on '595 and see if it helps.

CrossRoads:
Doubtful. '328P has 40mA drive capability, 74HC595's 1uA inputs will not affect those.
And even if you had '595 outputs connected to the SPI outputs, that would just result in output drive transistors fighting each other, no logic impact to the rest of the '328 functions.

Thats a good point :wink:

Its just strange that when experiencing the issue of the ICR1 reporting the wrong value, and then one of the SPI pins is disconnected from the 74HC, the ICR1 starts to work correctly. The code being executed by the chip does not change, nor is the atmega328p restarted.

After more playing, I've found that the same behaviour can be observed without using a 74HC595 and simply grounding pins 13, 11 and 10. As soon as one of those pins becomes free, the value reported in ICR1 is valid again.

Could there be any way that these pins/SPI is delaying the initial interrupt TIMER1_CAPT_vect being fired, so the value reported is wrong?

CrossRoads:
You don't need the delay in this function
void sendspi(uint8_t i_data) {
The shift registers are all set after the 8 SRCLK clocks and then RCLK going low to high to move the data to their output latches.

Yep, sorry it was a copy and paste error when minimising the example. It was needed to give the LEDs enough time to illuminate.

CrossRoads:
Do you have a 0.1uF cap from each shift register Vcc pin to Gnd? (And Not on any control line)

Thanks for the tip. I did'nt realise that caps were needed on the Vcc. However the behaviour is the same after making the change.

CrossRoads:
What speed are you using with Serial.begin()? 115200 or something slow like 9600? I'd go with faster.

It was 9600, but tried 115200 too. Made no difference.

Smajdalf:
Probably switching the '595 causes noise on power lines and it makes the input capture read edge that does not exist or makes a glitch in the HC-SR04 to give wrong output (more likely). Try to add decoupling, reduce load on '595 and see if it helps.

Its a good point, I didnt think about the HC-SR04 providing the wrong output.

Just learning now about the need of decoupling caps. Would you suggest the cap on the 74HC595 Vcc pin (already tried adding 0.1uF as CrossRoads suggested), or on the HC-SR04 Vcc?

kazza:
After more playing, I've found that the same behaviour can be observed without using a 74HC595 and simply grounding pins 13, 11 and 10. As soon as one of those pins becomes free, the value reported in ICR1 is valid again.

What? You grounded those pins? CLK, SS and MOSI? Just connected them to GND and let the SPI send data?
STOP IT! You will quickly kill the pins and maybe the whole chip. The problem is very probably with unstable power source: each of those pins will source ~80mA when grounded and trying to pull up. I believe your software is OK, problem is in hardware. Noise on power lines is probably doing this. Try to add some decoupling to the HC-SR04. You can start with 0.1 uF, 10 uF and 100 uF to power lines and see if it helps. If so you can experiment what combination is the best.

Smajdalf:
What? You grounded those pins? CLK, SS and MOSI? Just connected them to GND and let the SPI send data?
STOP IT! You will quickly kill the pins and maybe the whole chip. The problem is very probably with unstable power source: each of those pins will source ~80mA when grounded and trying to pull up. I believe your software is OK, problem is in hardware. Noise on power lines is probably doing this. Try to add some decoupling to the HC-SR04. You can start with 0.1 uF, 10 uF and 100 uF to power lines and see if it helps. If so you can experiment what combination is the best.

Thanks, that has helped :slight_smile:

It appears to have stabilised once reaching 220 uF on the HC-SR04 power line.