Hi
I try to write code simulates the ignition, then there is a problem if dwell is equal to 1500 period / REV reduced. But if you dwell equals = 100. The period of time it is normal period / rev affect the calculation rpm.
Please suggest how I should solve the issue.
Thank you.
This code
int coil = 7;
unsigned long rpm;
unsigned long last_time;
unsigned long period;
volatile int advance = 3409; //advance 45 at 2300 rpm for test
int dwell = 1500;
void setup() {
Serial.begin(9600);
attachInterrupt(0, rpm_cal, RISING);
pinMode(coil,OUTPUT);
}
void loop() {
Serial.print("RPM");
Serial.print(" ");
Serial.print(rpm);
Serial.print(" ");
Serial.print("period");
Serial.print(" ");
Serial.println(period);
}
void rpm_cal()
{
delayMicroseconds(advance);
digitalWrite(coil,HIGH);
delayMicroseconds(dwell);
digitalWrite(coil,LOW);
rpm = 60000000*(micros()-last_time);
period = micros()-last_time;
last_time = micros();
}
It is a bad idea to do lots of stuff in your interrupt service routine.
I would get the interrupt routine to just set a flag that the event has occured, move all the rest of that stuff into loop( ), and get rid of the delays.
Also, you calculation of rpm seems to be wrong. If your time between interrupts is longer, then your machine must be spinning more slowly. But in your calculation, rpm would be bigger if the period was longer - makes no sense.
I would also want to be calculating period and last_time, at the time when the interrupt occurs, not the time after you did that other stuff.
I also think you need to review your serial.printing. You cannot print that much stuff 2300 times per second, especially not at 9600 baud.
michinyon:
It is a bad idea to do lots of stuff in your interrupt service routine.
I would get the interrupt routine to just set a flag that the event has occured, move all the rest of that stuff into loop( ), and get rid of the delays.
Also, you calculation of rpm seems to be wrong. If your time between interrupts is longer, then your machine must be spinning more slowly. But in your calculation, rpm would be bigger if the period was longer - makes no sense.
I would also want to be calculating period and last_time, at the time when the interrupt occurs, not the time after you did that other stuff.
I also think you need to review your serial.printing. You cannot print that much stuff 2300 times per second, especially not at 9600 baud.
I'd grab micros() that is not shut down (pretty sure it returns the last value in ISR) and set a flag.
Always run 115200 (or faster) serial unless the target can't take it. It clears the serial output buffer quicker.
Also serial output, only print when something changes starting each line with time stamp, much nicer output.
Thank you. all
I tried move some code out of interrupt. rpm calculate is stable but Degrees ignition does not match.
I install hall sensor at 90 degrees at high rpm(2300) ignition at 45(90-45) degree and low rpm ignition at 85(90-5) degree before TDC.
form formula delay(us) = (1/(rpm/360))x(angle/60) = (1/(2300/60)) x(45/60) = 3260 us
TheTCI system want to dwell for a charge coil. dwell = 1500 us
delay = 3260 -1500 = 1760 us
I tried the flashlight simulation timinglight.The results showed that it is swinging.
A solution to the problem?
I recently started a program for arduino.
//#include <LiquidCrystal_I2C.h>
//#include <Wire.h>
// LiquidCrystal_I2C lcd(0x27, 16, 2);
const byte flash = 11;
int coilOut = 7; //Coil output on pin 7
int led = 13; //arduino led
int trigger = 3;
volatile byte rpmcount;
unsigned long rpm;
unsigned long timeold;
volatile int advance = 0; //testing stuff
int degree;
int dwell = 1000;
boolean fire = false;
unsigned long preriod = 0;
void setup()
{
pinMode(trigger,INPUT);
attachInterrupt(0, rpm_cal, RISING);
pinMode(coilOut, OUTPUT); //Coil output
Serial.begin(9600); //Initialize serial port
// lcd.begin();
pinMode(flash, OUTPUT);
}
void loop()
{
if (fire == true){
delayMicroseconds(1760);
digitalWrite (coilOut, HIGH); //Charge coil
delayMicroseconds(1500);
digitalWrite (coilOut, LOW);
fire = false;
}
//rpmcount = 0;
Serial.println(rpm,DEC);
//Serial.println(advance,DEC);
//Serial.println(" RPM");
speed_fan();
}
void rpm_cal() //Interrupt routine run when tach pulse goes high
{
rpm = 60000000ul/(micros() - timeold);
timeold = micros();
fire = true;
}
void speed_fan()
{
const byte POTENTIOMETER = 0;
int reading;
int value;
reading = analogRead(POTENTIOMETER);
value = map(reading, 0, 1024, 0, 255);
analogWrite(flash, value);
}
It seems to me, you need two separate pieces of functionality. One to keep track of the engine speed, and when the magnet passes the hall effect sensor, and the other to control the sparks. To get both of those to work optimally, you don't want any delays.
The other problem I can see in your code, is that "advance" is effectively a negative time. The greater the advance, the more time that the spark happens BEFORE top dead center.
In your code, the larger the advance, the longer the delay, and the later the spark will be. That doesn't look right.
Oldsnake00:
form formula delay(us) = (1/(rpm/360))x(angle/60) = (1/(2300/60)) x(45/60) = 3260 us
us = angle / rpm = us? I don't see that.
Could you include labels with your numbers and have an answer that actually holds start to end? That's how you check your work, by the lables as well as by the values.
Variables rpm and fire both need to be declared volatile
Don't call micros() multiple times within the ISR. Call it once, assign it to a variable and then just reference the variable
Try to move your rpm calculation out of the ISR. At the very least, do the division operation outside the ISR as it will be SLOW
Increase your serial speed to 115200. At 9600bps it's taking ~3300 uS every single loop just to send the 4 bytes of the rpm. It's the equivalent of adding a delayMicroseconds(3300) into your loop().
I would also highly recommend making it only print the rpm out every 10 loops or so as you simply don't need it updating every loop
If you're serious about getting accurate timing, you really need to use the hardware interrupt timers as they will be FAR more accurate than using delay. I've got ignition accuracy down to around +/- 3uS using this method, and that includes doing a full 8x8 table lookup, so it's definitely possible.
GoForSmoke:
micros() and millis() fetch a number from a register and are the products of timer interrupt.
micros() is always rounded to the nearest 4.
micros() includes 2 division operations using longs. If using it more than once within a short space of time it is quicker to assign to a variable and reference that.
Serial output is supposed to be improved since 1.0, so I've been informed.
Yep 1.0 added async serial, but it only works provided you not trying to transmit at a rate that fills the send buffer. In OPs code, I can almost guarantee that the outgoing buffer is being filled really quickly. Upping the baud rate allows the buffer to be flushed quicker than at 9600. Only transmitting the rpm every x loops is even better.
noisymime:
micros() includes 2 division operations using longs. If using it more than once within a short space of time it is quicker to assign to a variable and reference that.
His use or the code for the call?
Yep 1.0 added async serial, but it only works provided you not trying to transmit at a rate that fills the send buffer. In OPs code, I can almost guarantee that the outgoing buffer is being filled really quickly. Upping the baud rate allows the buffer to be flushed quicker than at 9600. Only transmitting the rpm every x loops is even better.
Yes, once the buffer is full you have to wait until it is not full.
When trying to work to close timing it is better not to serial print at all.
Increase your serial speed to 115200. At 9600bps it's taking ~3300 uS every single loop just to send the 4 bytes of the rpm. It's the equivalent of adding a delayMicroseconds(3300) into your loop().
[/quote]
No, it's not the same at all. The data is sent using interrupts, so very little CPU time is lost, and increasing the BAUD rate does not reduce this small overhead by even a single clock cycle.