Oooooh, ok. Your first post on the subject (and general standards) use CRLF not LFCR. I'll give that a try once I switch the wires back (I had them right the first time). Still not sure why my tach is reporting faster speeds than seems accurate. :/ Sensor not close enough? Need a different capacitor?
That did it! Falcon flashing, and speed auto-adjusting! Sweet! Thanks very much to you both, tauro0221 and Pyramid! Very cool! Will post video tomorrow. Latest and greatest code attached.
Here's the real kick in the pants, though (Pyramid, you'll especially get a laugh out of this)...
I bought the Falcon (once I finally found one at a non-inflated price) because my turntabulator app was showing the speed of my deck as being pretty slow: 33.20 on the center 33 pulley groove. This was driving me nuts, as my Kill-A-Watt was showing 60Hz from both the wall and from a TrippLite Double Conversion UPS. I tried the Falcon at stock spec (60.00Hz), and again, 33.20 on center groove, 33.25 max. Well, guess what speed turntabulator is reporting while the tach/Falcon auto adjust...
33.22.
LMAO!
It's definitely strange, though, that this same app will report exactly 33.33 on a Technics SL1200. I tried it on multiple units, all set to +0.0 adjust. *scratches head*
Here's the real kick in the pants, though (Pyramid, you'll especially get a laugh out of this)...
I bought the Falcon (once I finally found one at a non-inflated price) because my turntabulator app was showing the speed of my deck as being pretty slow: 33.20 on the center 33 pulley groove. This was driving me nuts, as my Kill-A-Watt was showing 60Hz from both the wall and from a TrippLite Double Conversion UPS. I tried the Falcon at stock spec (60.00Hz), and again, 33.20 on center groove, 33.25 max. Well, guess what speed turntabulator is reporting while the tach/Falcon auto adjust...
33.22.
LMAO!It's definitely strange, though, that this same app will report exactly 33.33 on a Technics SL1200. I tried it on multiple units, all set to +0.0 adjust. *scratches head*
Latest version in action: https://youtu.be/MDAqJ7FbcW4
I wonder if I should try smoothing the numbers using 3+ shifting timestamps instead of 2 and getting the average of them. I'm not sure if that would be better or worse for accuracy and/or limiting excessive speed correction from the Falcon.
I wonder if I should try smoothing the numbers using 3+ shifting timestamps instead of 2 and getting the average of them. I'm not sure if that would be better or worse for accuracy and/or limiting excessive speed correction from the Falcon.
You are seeing some fairly large changes in the reading from rev to rev; I don't think the table is that unstable, it is mostly likely the response time of the software. One of the reasons I don't like using C for time critical applications is you have no idea how many instructions are being executed, the response time of the ISR (just to vector to the routine) or the priority structure of the underlying subroutines and timers (if the function micros() has to wait for some other routine to finish, it will give slightly wrong results). At this resolution, every instruction cycle counts.
You can average the display reading to smooth it out, but I wouldn't average the data going to the Falcon, it will seriously screw up the control algorithm and the unit will really hunt. If fact, it could become unstable and increase the amplitude of oscillations in error over time until it becomes audible.
I have an Ardruino headed my way in the next few days; I'll see if I can improve on what you and tauro0221 have done so far.
You should be able to remove the delay(1000) at the end of your main loop. It basically does very little and I doubt it is causing the problem, but you want to remove any superfluous code to eliminate any possibility of that happening.
In Packgrog's version, the display only gets written to when the interrupt occurs (every 1.8 sec @ 33 RPM) or when the main loop times out (every 5 sec).
So I ditched the delay in the loop, and refactored the interrupt handler to try cutting down excess calls to lcd.print(). Any thoughts on this?
First things first, I moved setting prev_pulse_time = last_pulse_time to the end of the handler method, so that last_pulse_time = micros() could be the very first thing to happen in the handler, to limit the delay as much as possible. I also changed the lcd.print() functionality to limit the time spent on it, since I suspect that's likely an expensive call.
I did stumble across some comments on a potential speedup from a weird refactoring of "if ((10.0 <= rpm) && (rpm < 100))", but the code didn't really make sense to me. It seemed flat-out wrong (speed is great, but useless if functionally incorrect). I'm also not sure this would be one of the bigger fish.
I wonder if using local variables, and the corresponding creation/destruction, could add an unnecessary delay? Making all variables global goes against my coding instincts, but perhaps it could save some cycles here. I wonder if making outstr global char[8], prepopulated with the LNCR at the end, and just doing the dtostrf and single Serial.print() would save some more.
Thoughts?
Code:
// Sensor pulse completion interrupt callback handler
void calculate_rpm() {
last_pulse_time = micros();
if (cycle_count < 2) {
// Build up a couple of cycles to limit poor reporting.
cycle_count++;
lcd.setCursor(0, 1);
lcd.print(" RPM: ");
} else {
// Avoid DIV0, just in case.
float rpm = (last_pulse_time > prev_pulse_time) ? 60000000.0/(last_pulse_time - prev_pulse_time) : 0;
lcd.setCursor(7, 1);
lcd.print(rpm, 4);
// Send update to Phoenix PSU via serial port (D0 and D1) in format XX.XXX[lf][cr]
if ((10.0 <= rpm) && (rpm < 100)) {
char outstr[6];
dtostrf(rpm, 6, 3, outstr);
Serial.print(outstr);
Serial.print("\n\r");
}
}
prev_pulse_time = last_pulse_time;
}First things first, I moved setting prev_pulse_time = last_pulse_time to the end of the handler method, so that last_pulse_time = micros() could be the very first thing to happen in the handler, to limit the delay as much as possible. I also changed the lcd.print() functionality to limit the time spent on it, since I suspect that's likely an expensive call.
I did stumble across some comments on a potential speedup from a weird refactoring of "if ((10.0 <= rpm) && (rpm < 100))", but the code didn't really make sense to me. It seemed flat-out wrong (speed is great, but useless if functionally incorrect). I'm also not sure this would be one of the bigger fish.
I wonder if using local variables, and the corresponding creation/destruction, could add an unnecessary delay? Making all variables global goes against my coding instincts, but perhaps it could save some cycles here. I wonder if making outstr global char[8], prepopulated with the LNCR at the end, and just doing the dtostrf and single Serial.print() would save some more.
Thoughts?
Hi,
.
The last skecht from Packgrog's version he has a delay of "delay(1000)" at the the end of the loop.. The delay instruction will hold timers in the Arduino system until the delay is over. Nothing will be processes. Maybe I missed the last skecht.
void loop() {
// Show waiting message after 5 seconds of no new pulses, but only if pulses were detected.
if (micros() - last_pulse_time > 5000000 && cycle_count > 0) {
lcd.setCursor(0, 1); // Set cursor at first character, second line.
lcd.print(" Awaiting Signal");
cycle_count = 0;
}
delay(1000); <<<<<<<<<<<<<<<<<<<<<<<<delay
}
.
The last skecht from Packgrog's version he has a delay of "delay(1000)" at the the end of the loop.. The delay instruction will hold timers in the Arduino system until the delay is over. Nothing will be processes. Maybe I missed the last skecht.
void loop() {
// Show waiting message after 5 seconds of no new pulses, but only if pulses were detected.
if (micros() - last_pulse_time > 5000000 && cycle_count > 0) {
lcd.setCursor(0, 1); // Set cursor at first character, second line.
lcd.print(" Awaiting Signal");
cycle_count = 0;
}
delay(1000); <<<<<<<<<<<<<<<<<<<<<<<<delay
}
(BTW, tauro0221, if you "Go Advanced", you can wrap your code snippets in the CODE tag, so the formatting won't get borked).
My only concern with ditching the delay is that the loop comparison "if (micros() - last_pulse_time > 5000000 && cycle_count > 0)" will be running CONSTANTLY. I'm not sure if that would be a help of a hindrance constantly making so many calls to micros().
My only concern with ditching the delay is that the loop comparison "if (micros() - last_pulse_time > 5000000 && cycle_count > 0)" will be running CONSTANTLY. I'm not sure if that would be a help of a hindrance constantly making so many calls to micros().
The goal is to capture the count in micros as soon as the detection of the trigger event occurs. Any delays, or any CHANGES in delay from one reading to the next will cause the reading to bobble. Once the count is captured (by your last_pulse_time = micros() statement), almost nothing else will matter or change the computation, for the next 1.8 seconds at least. Any inconsistency in time vectoring to the ISR will cause the reading to vary.
If the resolution of the timer tick is 4.00µS, the a count of 450,000 produces a result of 33.333 RPM. For the display to show 33.352 RPM, the count would need to be 449,748; 252 counts is a lot to be off by. It may be a combination of trigger ambiguity and software processing. One of the things I did to confirm my trigger was occurring at the same point in time every rev, was to output a very short but very bright pulse from a white LED at the time the trigger was detected. I taped a small portion of an engineer's rule (with 1/64" graduations) to the platter surface and positioned a 40 AWG wire so that it would be midway through the rule when the strobe light flashed. I then observed the trigger point on the engineer's rule at every flash (through a 10x microscope) and looked for drift or bobble from one trigger to the next. This was easier to do in assembly language and any inconsistent software delays in creating the strobe flash will exacerbate the problem, but it should tell you if your detection circuit is inconsistent or not.
As a rule, you generally want to keep the ISR as short as possible. Your doing the calculation and output in the ISR probably doesn't adversely affect the outcome, but I perfer to use flags to signal the main loop to do all the processing and output. YMMV.
You could use a different call to millis() if this was a concern (assuming this uses a different timer than micros().
The difference in 33.333 and 33.352 RPM is ~1mS (252 counts x 4µS).
I'm fairly new to the Arduino, but one of the questions I had for a friend of mine who has programmed them for a while: Which of the 3 timers does micros() use? If I write my own routine to increment a count every xx.xµS and I use the same timer, what happens? What happens when you attempt to assign a priority to an ISR that is already in use by micros() or millis()?
Unfortunately, I haven't been able to find any answers.
Unfortunately, I haven't been able to find any answers.
Not a bad idea there. Probably wouldn't make a difference in the time capturing accuracy, but also probably a worthwhile exercise.
I'm not sure whether the 3 pulse check is necessary, as you're ultimately only checking the speed difference every third pulse. However, your approach captured the micros(); value sooner (I suspect that the case statement is faster than an assignment), and your interrupt handler was much smaller, doing the bulk of the processing in the loop. I've changed it to be more like yours, as per Pyramid's suggestion. Perhaps that will provide better results.
I will say one thing for your approach, tauro0221: The speed calculations were much closer to what I expected, and matched more closely to the Turntabulator reading. Who knows, maybe it's just delay from trying to do "prev_pulse_time = last_pulse_time" before "last_pulse_time = micros()", and none of the other changes matter for speed accuracy. Or maybe the collection of changes will add up to overall better efficiency as well.
I will say one thing for your approach, tauro0221: The speed calculations were much closer to what I expected, and matched more closely to the Turntabulator reading. Who knows, maybe it's just delay from trying to do "prev_pulse_time = last_pulse_time" before "last_pulse_time = micros()", and none of the other changes matter for speed accuracy. Or maybe the collection of changes will add up to overall better efficiency as well.
Here's some tweaks to offload the bulk of the work to the loop, and grab micros() first and foremost in interrupt handler. Perhaps the size of the ISR last time was causing further delay just from loading the function. Updating only 2 tiny global variables is about as small as I'm going to be able to get that.
It's been a long time since I had to be this nitpicky with code time sensitivity.
It's been a long time since I had to be this nitpicky with code time sensitivity.