First of all, my project actually works well enough, but I've read enough over the past few days to make me think there may be a better way.
The gist of it is, I have an aftermarket tachometer that reads a pulsed squarewave input to produce the tachometer reading, and I'd like not to take it apart. The vehicle has been converted to electric, and there's no good place to mount an RPM sensor. However, the EV controller outputs the signal over CAN
Two issues I'm running into... I'm using the loop to make sure I produce enough pulses for the tach to see the signal, but that obviously causes a delay, and that delay shows up as some judder in the needle, ESPECIALLY at low RPM. In a regular car, it'd be fine because you're not likely to see anything under 800 or so, but in an EV, the RPM may actually be 50 or 100 while you're going down the road.
The other issue was that at lower RPM, the delay time for delayMicrosecond was high enough that it was... causing some erratic behavior, so I just stuffed in an if statement that said "if it's under 2000 use 'delay' instead.
While i was reading up on how to do this, I saw some mention of using the system clock instead of the delay as a way to trigger the pulse without holding the rest of the code hostage during the loops and delays. But I wasn't that far yet and didn't understand it well enough to go down that route blind.
This is what I have:
#include <mcp2515.h>
struct can_frame canMsg;
MCP2515 mcp2515(10);
int rpm = 3000;
unsigned long rpmDelay;
void setup()
{
//setup the CANBus module
mcp2515.reset();
mcp2515.setBitrate(CAN_500KBPS,MCP_8MHZ);
mcp2515.setListenOnlyMode();
//Setup Serial
Serial.begin(9600);
Serial.print("CANBus Tachometer Driver");
pinMode(3, OUTPUT);
}
void loop()
{
//get CAN Message
if ((mcp2515.readMessage(&canMsg) == MCP2515::ERROR_OK) && (canMsg.can_id == 0x281)){
int x=canMsg.data[0];
int y=canMsg.data[1];
//combine the 2 digit hex for field 0 and field 1 to get decimal rpm
rpm = x+(y*16*16);
Serial.print("RPM: ");
Serial.println(rpm);
if(rpm > 2000){
//set rpmDelay in microseconds based on decimal rpm
rpmDelay = 60000000 / rpm / 2; //60000000 microseconds in a minute divided by RPM divided by 2
for(int t=0; t < 20; t++){
digitalWrite(3, HIGH);
delayMicroseconds(rpmDelay);
digitalWrite(3, LOW);
delayMicroseconds(rpmDelay);
}
} else {
if(rpm > 0){
//set rpmDelay in milliseconds based on decimal rpm
rpmDelay = 60000 / rpm / 2; //60000 milliseconds in a minute divided by RPM divided by 2
} else {
rpmDelay = 60000 / 100/ 2; //set rpm to 100
}
for(int t=0; t < 10; t++){
digitalWrite(3, HIGH);
delay(rpmDelay);
digitalWrite(3, LOW);
delay(rpmDelay);
}
}
}
}
Thank you! I think that's exactly what I was looking for... one question I had though, since it's constantly counting upwards (currentMicros) what happens when the size is greater than a long?
UNSIGNED long. The counter rolls over and "currentMicros - lastTachMicros" still produces the correct answer. That is why you always subtract a past time from a more recent time to get an elapsed time.
Awesome! I wondered. I'm new to Arduino, and my experience has been pretty random. Javascript which is very forgiving and SQL which is "Oh, that number's too big, I guess I'll just break".
I guess one last question - once I've tested and made sure this works well enough, where's the best place to share this so other people can just grab the code, follow the instructions and have a working tachometer? When I was using Google to piece this together, I found stuff all over the place and one place didn't stand out much more than any other.
Just finished testing it in the vehicle. I pulled out the delay loops and noticed it was a little erratic on the sampling, then realized I accidentally removed the part that checks if RPM is 0, so at startup I was basically dividing by zero as fast as I could. After adding the section to check for that, it was much better. The other change I made was pulling the lastTachMicros initialization up above the setup, to keep from zeroing it every time through the loop.
It works a little bit better than it did initially, especially getting rid of the long wait, low RPM for-loop. It's still a bit less smooth at lower rpms (longer waits) than higher ones, which might be a limitation of the tach, or just a necessary product of longer wait times showing up in the pulse rate. It's still more than accurate enough, and not likely to come up often.
Thank you, johnwasser, for your help!
For anyone else looking to do something similar (or anyone whose eagle eyes may see a logical flaw I missed or more room to improve) here's the current version:
//CANBus Tachometer RPM pulser v1.0
#include <mcp2515.h>
struct can_frame canMsg;
MCP2515 mcp2515(10);
int rpm = 3000;
unsigned long rpmDelay;
static unsigned long lastTachMicros = 0;
void setup()
{
//setup the CANBus module
mcp2515.reset();
mcp2515.setBitrate(CAN_500KBPS,MCP_8MHZ);
mcp2515.setListenOnlyMode();
//Setup for serial, if desired
//Serial.begin(9600);
//Serial.print("CANBus Tachometer Driver");
pinMode(3, OUTPUT);
}
void loop()
{
//tie pulse to clock
unsigned long currentMicros = micros();
if (currentMicros - lastTachMicros >= rpmDelay){
lastTachMicros = currentMicros;
digitalWrite(3, !digitalRead(3)); //toggle
}
//get CAN Message 0x281, position 0 and 1
if ((mcp2515.readMessage(&canMsg) == MCP2515::ERROR_OK) && (canMsg.can_id == 0x281)){
int x=canMsg.data[0];
int y=canMsg.data[1];
//combine the 2 digit hex for field 0 and field 1 to get decimal rpm
rpm = x+(y*16*16);
//serial logging, if desired
//Serial.print("RPM: ");
//Serial.println(rpm);
if(rpm > 0){
//set rpmDelay in microseconds based on decimal rpm
rpmDelay = 60000000 / rpm / 2; //60000000 microseconds in a minute divided by RPM divided by 2
} else {
rpmDelay = 60000000 / 100 / 2; //default to 100 rpm
}
}
}