RPM Sensor for Car

H

hsn6827

Member
Hi to all,
I have made and engine RPM sensor with Teensy 4.1 board. It is reading alternators rpm and write it to Dwin HMI. I have used LJ12A3-4-z/AY PNP inductive sensor. It trigers once per revolotion. My problem is this : It reads normally until the 3600 rpm. But it can not read more than this value although it has 500 Hz switching capacity. (it should read 30.000 PRM for 1 trigger per revolotion) What can be the problem? Could you investigate my code and circuit below please?

Code:

#include <TeensyThreads.h>
#include <Arduino.h>
int rpm =0; //Engine RPM value
const int rpmsensorPin = 40; // Engine RPM Proximity Sensor Input Pin
volatile int rpmpulseCount = 0;
unsigned long rpmlastTime = 0;
//**************** Dwin HMI Communication Parameters **************
#define mySerial Serial1 //UART communication for Dwin HMI
elapsedMillis tRPM;
unsigned char rpmdeger[8] = {0x5a, 0xa5, 0x05, 0x82, 0x52 , 0x00, 0x00, 0x00};
//==================================== ISR ===========================================================
void rpmpulseISR() { rpmpulseCount++;}

//*********************** Engine RPM Calculation & Write to Dwin *************************************
void taskRPM()
{
while (1)
{
if (tRPM >= 1000)
{
noInterrupts();
rpm = (rpmpulseCount*60/3); //Alternator/engine speed ratio is 3
rpmpulseCount = 0;
rpmdeger[6] = highByte(rpm);
rpmdeger[7] = lowByte(rpm);
mySerial.write(rpmdeger, 8);
tRPM = 0;
}
threads.delay(10);
interrupts();
}
}
void setup()
{
Serial.begin(115200);
mySerial.begin(115200);
threads.addThread(taskRPM);
}
void loop()
{
// No need to add code here
}

Circuit: Attached.
 

Attachments

  • Circuit.png
    Circuit.png
    119.1 KB · Views: 52
@hsn6827: I'd first get rid of the unnecessary threading. Since you're only using one thread, threading is only adding overhead, with no benefit. Instead, make use of the timer library, setup a periodic timer, & count RPM from there. Or, better yet, use the frequency measurement capability (don't know if this is part of the timer library, or some other library), which easily takes care of everything for you.

Also, in your current implementation, what is the inclusion of threads.delay(10) intended to do for your specific environment and/or application ?? Using any kind of delay will definitely reduce the maximum rate that you might be able to measure.

Hope that helps . . .

Mark J Culross
KD5RXT
 
I agree with Mark that threading seems to be overkill for your application, especially since once you enter the thread, the while loop prevents you ever leaving it and so the thread scheduler never gets a chance to do anything. The delay(10) also doesn't seem to have any useful purpose.
Turning interrupts off for as long as you do might be a problem too. The only purpose to turning off interrupts in your code would be to prevent an interrupt occurring while you are accessing rpmpulseCount.
Try changing your code to this:
Code: 
  while (1) {
    if (tRPM >= 1000) {
      noInterrupts();
      rpm = (rpmpulseCount*60/3); //Alternator/engine speed ratio is 3
      rpmpulseCount = 0;
      interrupts();
      rpmdeger[6] = highByte(rpm);
      rpmdeger[7] = lowByte(rpm);
      mySerial.write(rpmdeger, 8);
      tRPM = 0;
    }
  }


Pete
 
kd5rxt-mark said:
@hsn6827: I'd first get rid of the unnecessary threading. Since you're only using one thread, threading is only adding overhead, with no benefit. Instead, make use of the timer library, setup a periodic timer, & count RPM from there. Or, better yet, use the frequency measurement capability (don't know if this is part of the timer library, or some other library), which easily takes care of everything for you.

Also, in your current implementation, what is the inclusion of threads.delay(10) intended to do for your specific environment and/or application ?? Using any kind of delay will definitely reduce the maximum rate that you might be able to measure.

Hope that helps . . .

Mark J Culross
KD5RXT
Click to expand...
Thank you very much for your suggestion @kd5rxt-mark. Acctually there are diffrent functions in my code like DC servo control, data reading & writing to Dwin HMI via UART, Can-Bus control, some button control etc. It is multi tasking code. Main aim of threads is preventing function interrupts. Because when i tried to code it without threads, i can not read datas thats sending from HMI because of high frequency interrupts of RPM sensor. So i have added thredas. Code is a little bit long. So i did not share all of it. I have shared only problematic part. Other parts are OK.
I can remove 10 ms delay. No problem. But as well as i understand, circuit can be problematic. Do you have any comment about it?




el_supremo said:
I agree with Mark that threading seems to be overkill for your application, especially since once you enter the thread, the while loop prevents you ever leaving it and so the thread scheduler never gets a chance to do anything. The delay(10) also doesn't seem to have any useful purpose.
Turning interrupts off for as long as you do might be a problem too. The only purpose to turning off interrupts in your code would be to prevent an interrupt occurring while you are accessing rpmpulseCount.
Try changing your code to this:
Code: 
  while (1) {
    if (tRPM >= 1000) {
      noInterrupts();
      rpm = (rpmpulseCount*60/3); //Alternator/engine speed ratio is 3
      rpmpulseCount = 0;
      interrupts();
      rpmdeger[6] = highByte(rpm);
      rpmdeger[7] = lowByte(rpm);
      mySerial.write(rpmdeger, 8);
      tRPM = 0;
    }
  }


Pete
Click to expand...
Thank you @el_supremo . For the thred usage, i have explained upside. There are other functions. So it is not possible to run them without threads. I have tried and failded. I have also tried to code only RPM sensor run. Result is same. Could you comment my circuit if you have any expreince please?
 
You could temporarily short out LED_SENS1. Teensy port pin was not initialized in setup to have a pullup. If this makes code work, then you can add a pullup resistor external to Teensy, or init port pin with pullup. An open-collector circuit needs a good pullup resistor.
 
hsn6827 said:
Thank you very much for your suggestion @kd5rxt-mark. Acctually there are diffrent functions in my code like DC servo control, data reading & writing to Dwin HMI via UART, Can-Bus control, some button control etc. It is multi tasking code. Main aim of threads is preventing function interrupts. Because when i tried to code it without threads, i can not read datas thats sending from HMI because of high frequency interrupts of RPM sensor. So i have added thredas. Code is a little bit long. So i did not share all of it. I have shared only problematic part. Other parts are OK.
Click to expand...
It is always best, when possible, to post your complete sketch, not just the portion that you think might be problematic. With a complete post, anyone else can copy/paste your sketch into the Arduino IDE & attempt to reproduce your problem for troubleshooting purposes. Although you're confident that "Other parts are OK", there's always the possibility that the problem only occurs when everything is running together. Similarly, any comments on "only using one thread" & "only adding overhead" would have been unnecessary with the entire sketch visible.

Mark J Culross
KD5RXT
 
BillFM said:
You could temporarily short out LED_SENS1. Teensy port pin was not initialized in setup to have a pullup. If this makes code work, then you can add a pullup resistor external to Teensy, or init port pin with pullup. An open-collector circuit needs a good pullup resistor.
Click to expand...
Thank you @BillFM . Sorry, i skipped to copy that part while extracting RPM sensor codes. Actually, it has been initialized at setup as:
pinMode(sensorPin, INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(sensorPin), pulseISR, FALLING);

Ok. Let me short out LED_SENS1 and try. Can it be acceptable to add soft pullup for Teensy input?






kd5rxt-mark said:
It is always best, when possible, to post your complete sketch, not just the portion that you think might be problematic. With a complete post, anyone else can copy/paste your sketch into the Arduino IDE & attempt to reproduce your problem for troubleshooting purposes. Although you're confident that "Other parts are OK", there's always the possibility that the problem only occurs when everything is running together. Similarly, any comments on "only using one thread" & "only adding overhead" would have been unnecessary with the entire sketch visible.

Mark J Culross
KD5RXT
Click to expand...
Yes you are right. I tought this but I have tried diferent simple RPM counter codes also. Result was same. It could read up to 3600 rpm. So I understood that the problem was related with hardware. To make the analysis easy, and prevent confusion, I extracted RPM codes from all code and write to here. No problem, if you still think that it will be useful, I can share it in a file.
 
Last edited:
Hello.
Since the opto is currently detecting signals up to 3600rpm (60Hz), and 60Hz is a pretty slow frequency, and the internal Teensy pull-up is enabled, this suggests the internal pull-up is not sufficient to do the job (above 3600rpm). Hence my suggestion to short out LED and let existing 1k Ohm (R12) resistor act as the pull-up to see what happens.

There could be other factors at play here. I've not looked at opto datasheet and we don't know anything about the 7V signal source. Could be that opto LED is not driven hard enough to have a decent energy transfer to the photo-transistor. If that's the case, photo-transistor might not be turning fully ON and can't sink very much current. i.e., not reaching logic 0 voltage level.

When you say it doesn't work above 3600rpm, what do you observe?
No rpm changes reported?
Erratic rpm changes?
Incorrect rpm reported?
 
Have you gotten another Teensy or other MCU. If so program it to output 70Hz. Disconnect your RPM sensor and feed the MCU/Teensy pulses into your sensor/RPM measuring Teensy. If you see 4200RPM then your code is working ok. You could alter the rate of pulsing to determine the actual MAX Rpm that the Teensy/software can support.
 
BillFM said:
Hello.
Since the opto is currently detecting signals up to 3600rpm (60Hz), and 60Hz is a pretty slow frequency, and the internal Teensy pull-up is enabled, this suggests the internal pull-up is not sufficient to do the job (above 3600rpm). Hence my suggestion to short out LED and let existing 1k Ohm (R12) resistor act as the pull-up to see what happens.

There could be other factors at play here. I've not looked at opto datasheet and we don't know anything about the 7V signal source. Could be that opto LED is not driven hard enough to have a decent energy transfer to the photo-transistor. If that's the case, photo-transistor might not be turning fully ON and can't sink very much current. i.e., not reaching logic 0 voltage level.

When you say it doesn't work above 3600rpm, what do you observe?
No rpm changes reported?
Erratic rpm changes?
Incorrect rpm reported?
Click to expand...
Hello @BillFM. Ok, I will try it with shorted out led and mention about result. I am also suspicious from the that led also.

I have also tried the +12V signal source for opto. It was more problematic than 7V source. Because when i aplied 7V, it was measuring up to 3600 rpm and staying at this value. But when i aplied 12V it was measuring until 3600 rpm normally. After this value it was showing abnormal values like 0, -250,5000 etc. So i cancelled +12V feeding to sensor.


BriComp said:
Have you gotten another Teensy or other MCU. If so program it to output 70Hz. Disconnect your RPM sensor and feed the MCU/Teensy pulses into your sensor/RPM measuring Teensy. If you see 4200RPM then your code is working ok. You could alter the rate of pulsing to determine the actual MAX Rpm that the Teensy/software can support.
Click to expand...
Thank you @BriComp . It is logical way to understand for code confirmation. I can try it. I have ESP32 board.
 
hsn6827 said:
threads.delay(10);
interrupts();
Click to expand...
There is value in the .delay() - with a value in the proper range to avoid overwork or missed events - it exits the thread for the next.
That code will exit before enabling the interrupts(); as written though? Not sure that is a problem - but seems wrong.
 
I have tried a similar approach in the past with very poor success. I always ran into bounce issues, amongst many other issues. I finally found a winning solution. Maybe this will work in your case.

1) I had to get off the LED or infrared,sensors and move to a magnetic one, I can give you a schematic if you need.

2) the sensor pulse pin connect to pin 23 for a 4.0/4.1 (pin three for a 3.2)

3) then use the frequency count or frequency measure library. it has a great example for measuring RPMs. I’ve been using this set up for a few years and find it to be ridiculously reliable and accurate

Hope this helps
 
defragster said:
There is value in the .delay() - with a value in the proper range to avoid overwork or missed events - it exits the thread for the next.
That code will exit before enabling the interrupts(); as written though? Not sure that is a problem - but seems wrong.
Click to expand...
I have tried enabling interrupt function before and after delay. Also tried cancelled, increased and decreased delay time. But nothing changed. Thank you for your suggestion @defragster.


KrisKasprzak said:
I have tried a similar approach in the past with very poor success. I always ran into bounce issues, amongst many other issues. I finally found a winning solution. Maybe this will work in your case.

1) I had to get off the LED or infrared,sensors and move to a magnetic one, I can give you a schematic if you need.

2) the sensor pulse pin connect to pin 23 for a 4.0/4.1 (pin three for a 3.2)

3) then use the frequency count or frequency measure library. it has a great example for measuring RPMs. I’ve been using this set up for a few years and find it to be ridiculously reliable and accurate

Hope this helps
Click to expand...
Thank you for your suggestions @KrisKasprzak. I have tried some methods to understand the problem yesterday. As @BriComp s suggestion, I have connected an arduino board's output pin to my RPM sensor input pin and coded the arduino as 150 Hz ( 9000 RPM) output frequency. I could see this value with small diffrence (8940 rpm). So i suppose my problem is related with sensor. It's switching frequency is not enough although it seems enough in product manual theoretically.
I have searched about cars RPM sensor. They are magnetic hall effect sensor. So I am searching about suitable hall effect and high switching frequency inductive sensor. I am not sure which one is OK. Hall effect magnetic sensor is good option but I can not fix a magnet on alternator. There is not any place. If i stamp a magnet on alternator wing , it can cause balance problem even if it is small and it can fall down after for a while. So i am searching about high quality inductive sensors which can catch max 10000 rpm.

If you can share your sensor schematic and frequency measuring code, i would be glad. Your experince is very important for me.
I can not connect sensor input to pin 23 because of PCB design. It is very hard to change it now. But for next project, i will design it as your suggestion. But what is the diffrence of those pins? pin 23 is PWM pin. Do you mean about it? Does it cause any trouble to connect it to an analog pins?
 
As @BriComp s suggestion, I have connected an arduino board's output pin to my RPM sensor input pin and coded the arduino as 150 Hz ( 9000 RPM) output frequency. I could see this value with small diffrence (8940 rpm). So i suppose my problem is related with sensor. It's switching frequency is not enough although it seems enough in product manual theoretically.
Click to expand...
Yes, when making hardware and software changes/new designs I find it quite useful to have a third method of confirming performance. Glad you found he problem. Better that there had not been a problem of course.
 
To make use of a different pin, simply connect a blue wire between your existing pin (40) & your desired pin (23). In addition to defining your desired pin (23) as an input, make sure to continue to define the existing pin (40) as an input so that the external signal is not affected by the extra connection.

Mark J Culross
KD5RXT
 
kd5rxt-mark said:
To make use of a different pin, simply connect a blue wire between your existing pin (40) & your desired pin (23). In addition to defining your desired pin (23) as an input, make sure to continue to define the existing pin (40) as an input so that the external signal is not affected by the extra connection.

Mark J Culross
KD5RXT
Click to expand...
Ok. I will try it. Could you share your sensor code please?
 
KrisKasprzak said:
Correction : Above I said to use pin 23 for the signal with a 4.0 but it’s pin 22.
Click to expand...
Yes, I saw it from GitHub link. No problem. But my pin 22 is being used for another input. So it is hard to change it on PCB. So firstly, I will switch to magnetic hall effect sensor and try it. If problem ontinues, I will evaluate for other options like using frequency library with pin-22. Thanks.
 
You must log in or register to reply here.
Back
Top