PCintPort behaves strange when a Servo.attach() is not used

Projects Programming
1

I am building a quadcopter from scratch (on a Leonardo). It's almost finished.
Up till now, i have been using the Servo.h library to control the escs/motors. However, for a good, stable quad i need a higher update frequency (something around 200 - 400Hz, instead of the 50Hz of Servo.h). Decided to turn to analogWrite().

So when i removed the Servo.h stuff, i got very strange readings from the rc receiver (i am using PCIntPort interrupts to read those).
I isolated the problem in a short sketch:

#include <PinChangeInt.h>
#include <Servo.h>

#define CH1_PIN 8

uint16_t ch1;
uint16_t ch1_start;
volatile uint16_t ch1_shared;

void setup() {
  pinMode(CH1_PIN, INPUT);

  PCintPort::attachInterrupt(CH1_PIN, handleCh1, CHANGE);

  Servo motor1;
  motor1.attach(9);
}

void loop() {
  noInterrupts(); //disable interrupts so that ch1_shared doesnt update while we are reading it
  if (ch1_shared < 2000) ch1 = ch1_shared; //if reading is valid, store it
  interrupts();
  
  Serial.println(ch1);
  delay(20); //to keep serial happy
}

void handleCh1() {
  if(PCintPort::pinState) //if pin high
    ch1_start = TCNT1; //store start of the pulse
  else //if pin low
    ch1_shared = (TCNT1 - ch1_start)>>1; //store length of the pulse
}

This sketch prints in the serial the expected values:

1511
1511
1510
1510
1512

However when i comment out this line:

motor1.attach(9);

i get these really odd readings:

11
11
2
14
28
40
54
66
66
66
66
60
47
35
21
8

What is causing this? It seems to have to do with the timer(s), but i am not very comfortable with timers. Any help greatly appreciated.

-Ilias

2 
  noInterrupts(); //disable interrupts so that ch1_shared doesnt update while we are reading it
  if (ch1_shared < 2000) ch1 = ch1_shared; //if reading is valid, store it
  interrupts();

You are turning interrupts off and on on every pass through loop. Do that ONLY when the value needs to be copied.

Why are you not using regular external interrupts? It's probably better to move whatever is hogging the external interrupt pins.

  Serial.println(ch1);
  delay(20); //to keep serial happy

Bullpoop. Look at the blink without delay example. Read it. Understand it. Embrace it. Forget that the stupid delay() function even exists.

3

Why are you not using regular external interrupts? It's probably better to move whatever is hogging the external interrupt pins.

There are 5 external interrupts on the leonardo, 2 of which are pins TX and RX. That leaves me with 3, which is not enough for 4 channels.

You are turning interrupts off and on on every pass through loop. Do that ONLY when the value needs to be copied.

Bullpoop. Look at the blink without delay example. Read it. Understand it. Embrace it. Forget that the stupid delay() function even exists.

Good points, but once again this is just a short sketch i wrote to isolate the problem, without putting much thinking to it. Here it is corrected:

#include <PinChangeInt.h>
#include <Servo.h>

#define CH1_PIN 8

uint16_t ch1;
uint16_t ch1_start;
volatile uint16_t ch1_shared;

void setup() {
  pinMode(CH1_PIN, INPUT);

  PCintPort::attachInterrupt(CH1_PIN, handleCh1, CHANGE);

  Servo motor1;
  motor1.attach(9);
}

void loop() {
  unsigned long currentMillis = millis();
  if(currentMillis - previousMillis > interval) {
    previousMillis = currentMillis;
    noInterrupts(); //disable interrupts so that ch1_shared doesnt update while we are reading it
    if (ch1_shared < 2000) ch1 = ch1_shared; //if reading is valid, store it
    interrupts();
    Serial.println(ch1);
  }
}

void handleCh1() {
  if(PCintPort::pinState) //if pin high
    ch1_start = TCNT1; //store start of the pulse
  else //if pin low
    ch1_shared = (TCNT1 - ch1_start)>>1; //store length of the pulse
}
4 
    noInterrupts(); //disable interrupts so that ch1_shared doesnt update while we are reading it
    if (ch1_shared < 2000) ch1 = ch1_shared; //if reading is valid, store it
    interrupts();

No improvement. You are still turning interrupts off and on unnecessarily.

Do it this way:

    if (ch1_shared < 2000) 
    { // Always use these
        noInterrupts(); //disable interrupts so that ch1_shared doesnt update while we are reading it
        ch1 = ch1_shared; //if reading is valid, store it
        interrupts();
    } // Always use these

2 of which are pins TX and RX.

RX and TX are not connected to Serial on the Leonardo. They are connected to Serial1. Are you using Serial1? That is, are you actually using pin 0 and 1 for serial purposes?

    Serial.println(ch1);

Rather useless without Serial.begin() anywhere...

5

RX and TX are not connected to Serial on the Leonardo. They are connected to Serial1. Are you using Serial1? That is, are you actually using pin 0 and 1 for serial purposes?

Sorry, forget what i just said. The 2 interrupts i can't use are pins 2 and 3, because they are also the SDA and SCL pins, which i need to communicate with the IMU.

Rather useless without Serial.begin() anywhere...

Has been working for me up till now, and obviously, it still does.

No improvement. You are still turning interrupts off and on unnecessarily.

Sure, but can we please focus on the actual problem and not optimisation of this (useless for any reason other than illustrating the problem) sketch?