Is this code freezing because of the interrupt?

Hello everyone,

I am trying to make a throttle control for an electric bike.

Basically, using a ring of magnet and a reed switch I want to detect id a person is pedaling, and if so, enabling the throttle output.

I have an issue…

While pedaling with the gas on, if I stop the pedals suddenly in a certain position (probably with the reed switch closed), the output of the throttle pin is not set to 45 as it should.

If I let the gas (gasInput) go to a lower voltage of about 0.6V on this condition, throttle output will set to 45 as it should have just by not receiving any more reed switch passes. it will not resume to a higher value unless I restart pedaling, as it should.

If I increase the gas also in this wrong condition of not pedaling and throttle enabled, throttle output will follow accordingly.

I tried and tried, I just cannot make this go away!

volatile int rpm;
volatile unsigned long timeold;

volatile int revolutions;

volatile unsigned long last_micros;
int throttle = 9;
int CAenable = 4;
int gasInput = 0;
int legalInput = 5;
int gasValue;


void setup()
{
     pinMode(throttle, OUTPUT);
     analogWrite(throttle, 45);

     pinMode(CAenable, OUTPUT);

     pinMode(gasInput, INPUT);

     pinMode(legalInput, INPUT);
     digitalWrite(legalInput, HIGH);

     digitalWrite(2, HIGH);
     attachInterrupt(0, debounceInterrupt, RISING);

     revolutions = 0;
     rpm = 0;
     timeold = 0;
  
}

void loop()
{

    if (digitalRead(legalInput) == HIGH)
    {
        digitalWrite(CAenable, HIGH);

        if (millis() - timeold >= 500)
        {
        analogWrite(throttle, 45);
        }
 
        if (revolutions >= 6) 
        {
            //Update RPM every xx counts, increase this for better RPM resolution,
            //decrease for faster update
            rpm = 5 * 1000 / (millis() - timeold)*revolutions;

            timeold = millis();
            revolutions = 0;

            if (rpm >= 15 && millis() - timeold <= 500)
            {
                  gasValue = analogRead(gasInput) / 4.3;
                  gasValue = constrain(gasValue, 45, 250);
                  analogWrite(throttle, gasValue);
             }
             else
             {
             analogWrite(throttle, 45);
             }

        }
 
        if (millis()-timeold >= 500) 
        {
             analogWrite(throttle, 45);
             rpm = 0;
             timeold = millis();
             revolutions = 0;
         }
 
    }
    else
    {
        digitalWrite(CAenable, LOW);
        gasValue = analogRead(gasInput) / 4.3;
        gasValue = constrain(gasValue, 45, 250);
        analogWrite(throttle, gasValue);
     }

}

void debounceInterrupt() 
{
    if ((long)(micros() - last_micros) >= 3000) 
    {
        revolutions++;
        last_micros = micros();
    }
}

You REALLY need to use Tools + Auto Format to indent your code properly.

As is, it is impossible to follow.

Sorry, it looked fine on visual studio... I hope is readable now...

    if (digitalRead(legalInput) == HIGH)

I'm sure that legalInput means something to you. If so, I'm relatively certain that you are in the minority.

            rpm = 5 * 1000 / (millis() - timeold)*revolutions;

            timeold = millis();
            revolutions = 0;

How many times could the interrupt have fired while this code is executing? You need to disable interrupts while diddling with multi-byte volatile variables.

    if ((long)(micros() - last_micros) >= 3000)

micros() returns an unsigned long. last_micros is an unsigned long. Why is it necessary to cast that to a long, to compare to an int?

While pedaling with the gas on, if I stop the pedals suddenly in a certain position (probably with the reed switch closed), the output of the throttle pin is not set to 45 as it should.

What is the value of revolutions when this happens? Which block of code is not being executed?

Thank you Paul for looking into it.

I followed your suggestions and cleaned a little more the code.

legalImput is connected to a switch. At present is unconnected, so it should be kept high by the internal pullup.

So I was doing some more testing. I added a led to help me debugging, which should come off when I stop pedaling.

It does turn off if there is little voltage applied to pin gasInput (about 0.7V) and the motor is not running (throttle pin goes trough a RC filter ad into the motor speed controller.

If on pin gasInput I apply more gas, hence the motor will run if pedaling, if I stop pedaling in few fixed positions (again I believe where the magnets meets the reed) the led does not come off.

At this point to have the led come off and the motor stop, I either can move the pedals a little or release the gas. everything goes back as it should.

volatile int rpm;
volatile unsigned long timeold;

volatile int revolutions;

volatile unsigned long last_micros;
int throttle = 9;
int CAenable = 4;
int gasInput = 0;
int legalInput = 5;
int gasValue;
int led = 13;


void setup()
{
    pinMode(led, OUTPUT);
    digitalWrite(led, LOW);

    pinMode(throttle, OUTPUT);
    analogWrite(throttle, 45);

    pinMode(CAenable, OUTPUT);

    pinMode(gasInput, INPUT);

    pinMode(legalInput, INPUT);
    digitalWrite(legalInput, HIGH);

    digitalWrite(2, HIGH);
    attachInterrupt(0, debounceInterrupt, RISING);

    revolutions = 0;
    rpm = 0;
    timeold = 0;



}
void loop()
{

    if (digitalRead(legalInput) == HIGH)
    {
        digitalWrite(CAenable, HIGH);



        if (millis() - timeold >= 900)
        {

            digitalWrite(led, LOW);
            analogWrite(throttle, 45);
        }


        if (revolutions >= 5)
        {
            //Update RPM every xx counts, increase this for better RPM resolution,
            //decrease for faster update
            detachInterrupt(0);

            rpm = 5 * 1000 / (millis() - timeold)*revolutions;

            timeold = millis();
            revolutions = 0;

            attachInterrupt(0, debounceInterrupt, RISING);

            if (rpm >= 10)
            {
                digitalWrite(led, HIGH);

                gasValue = analogRead(gasInput) / 4.3;
                gasValue = constrain(gasValue, 45, 250);
                analogWrite(throttle, gasValue);

            }
        }

    }
    if (digitalRead(legalInput) == LOW)
    {
        digitalWrite(CAenable, LOW);
        gasValue = analogRead(gasInput) / 4.3;
        gasValue = constrain(gasValue, 45, 250);
        analogWrite(throttle, gasValue);
    }

}

void debounceInterrupt()
{

    if ((micros() - last_micros) >= 3000)
    {

        revolutions++;
        last_micros = micros();
    }
}

I think it might be noise from the motor into the reed switch cable...

 detachInterrupt(0);

 rpm = 5 * 1000 / (millis() - timeold)*revolutions;

 timeold = millis();
 revolutions = 0;

 attachInterrupt(0, debounceInterrupt, RISING);

That isn’t what PaulS meant.

noInterrupts ();

 rpm = 5 * 1000 / (millis() - timeold)*revolutions;

 timeold = millis();
 revolutions = 0;

 interrupts ();

There seems something very perverse using a variable called gasValue in code for an ECO-friendly electric bike :)

What are you trying to do with the interrupt?

It is almost certainly unnecessary - nobody can pedal that fast

It is even more certainly not being implemented correctly - but there is not much point spending time on that if it is not needed.

The way you are using millis() - especially when and how you are updating timeold will probably not help the accuracy. Have a look at several things at a time

...R

Robin2:
There seems something very perverse using a variable called gasValue in code for an ECO-friendly electric bike :slight_smile:

What are you trying to do with the interrupt?

It is almost certainly unnecessary - nobody can pedal that fast

Let me explain what I am trying to do…
My bike has a brushless motor rated 1200w, actually peaks at over 5kw. Max speed is about 70 kmph.
I normally use it up to 1kw/40kmph. Until now I only used the throttle (I call it gas)

In europe, the law says that a pedelec must provide electric assistance up to maximum 250w / 25 kmph, and this only if the “cyclist” is pedaling.

I have installed in it a Cycle analyst, a commercial product. This unit has an output which pulls the throttle to a lower value if one of the set parameters (user defined) is grater than it should be. It basically has already implemented a throttle override function.

I want to have a switch that let me select whether the bike will respond directly to my throttle input, for when I ride on selected locations during the weekend, or it will respond to my throttle input only if pedaling and also with enabled the throttle override function from the Cycle Analyst, hence legal, for when I commute.

Thank you Paul and Nick, I will update the code as you showed.
As I understand, the difference between what I did and what you suggest is that in the previous case interrupts would still occur from other background sources…

Robin, I am not really good at this, I am trying. The way I wrote the rpm using interrupts was taken from examples from the internet (I looked for Arduino RPM). The number of magnets on the disc for each pedal turn is 6, so if I pedal at lets say 60 rpm, the actual readings will be of 6 per second. I still don’t know if interrupt is a good idea or not, please comment on this…

I had I quick look at SeveralThingsAtTheSameTimeRev1

I will dig more into it, as my brain is not really code trained. If i will be able to understand it and apply to my sketch, I will certainly do!

Thank you all guys, I am still open to suggestions on how to improve this code!

I am sure what I cooked up is far from decent.

In the mean time, I put a small ceramic cap between interrupt input and gnd, it looks like its working now…

tomy983: Robin, I am not really good at this, I am trying. The way I wrote the rpm using interrupts was taken from examples from the internet (I looked for Arduino RPM). The number of magnets on the disc for each pedal turn is 6, so if I pedal at lets say 60 rpm, the actual readings will be of 6 per second. I still don't know if interrupt is a good idea or not, please comment on this..

It is not clear (to me) from your earlier comments if you need to detect the speed of pedalling or just the fact that the pedals are not stationary.

If you don't need the speed of the pedals I reckon one or two magnets would be plenty.

You certainly can use interrupts to increment a counter with code like this

void myISR() {
   pedalCount ++;
}

and elswehere in your code you can read that value with

prevPedalCount = newPedalCount; // save the previous value
noInterrupts();
  newPedalCount = pedalCount;
interrupts();
countsBetweenReads = newPedalCount - prevPedalCount;

But you could probably just as easily detect the magnet passing the sensor with

magnetDetected = digitalRead(sensorPin);

as long as it is checked frequently.

...R