Still not accessing 'count' atomically in non-ISR code.
Yes, but with the attach and detach code and a one second reading period there are no interrupts running when count is accessed.
Can you expand on that, I've gone over ISR requirements and I believe I'm doing that correctly?
Documentation suggests that millis(), delay() will work fine so long as they are not in the ISR ... which in my case they are not. The global "count" is setup as volatile.
Cheers, Rob.
Tried your suggestion, unfortunately same results.
I'm still not understanding why the reducing the fan speed (126) increases the count value? Is it possible I have the Corsair 3 and 4 pins backwards where Pin 4 is really the Tach/Signal/Sense and Pin 3 is Control/PWM? But if that were the case then I would expect changes analogWrite(fan_control_pin, 255) would not work but I know they do??
Cheers, Rob.
EDIT: no, tried with pin 3 and 4 swapped, no reading as expected ... so it's wired correctly.
Cheers, Rob.
What voltage are you using to power the Arduino? Your diagram suggests that it's 5V to the barrel jack, which isn't enough.
Also, what voltage is the rpm signal from the fan?
You can certainly test the Arduino. In my testing of your code, I jumpered pin2 to pin3 and changed the code so that pin3 was at 50% duty cycle( analogWrite(fan_control_pin, 127); and I was getting correct readings for count.
The issue is with the wiring or the voltages, not your code.
// UNO R3
// Set a fan speed 0-255 (Corsair SP120 4 wire PWM) 12V DC 0.25A
// 4 Black wires with pin 1 Identified ... see reference: https://landing.coolermaster.com/faq/3-pin-and-4-pin-fan-wire-diagrams/
const int fan_control_pin = 3;
unsigned long start_time;
volatile unsigned int count = 0;
int rpm;
void setup() {
// put your setup code here, to run once:
pinMode(fan_control_pin, OUTPUT);
analogWrite(fan_control_pin, 0);
pinMode(2,INPUT_PULLUP);
Serial.begin(9600);
// attachInterrupt(digitalPinToInterrupt(2), counter, RISING);
}
void loop() {
// Test, max speed for this fan is 2350 RPM
// Actual reported fan speed is 2160 @ 255 with 0.11A draw
// Actual reported fan speed is 8400 @ 126 with 0.03A draw ???
// Actual reported fan speed is 750 @ 0 with 0.02A draw
analogWrite(fan_control_pin, 127);
// Fan RPM
start_time = millis();
count = 0;
attachInterrupt(digitalPinToInterrupt(2), counter, FALLING);
while((millis() - start_time) < 1000)
{
// 1 second pause so count increments
}
detachInterrupt(digitalPinToInterrupt(2));
// rpm = count * 60 / 2;
rpm = (count/2)*6;
Serial.print("The fan speed is ");
Serial.print(rpm);
Serial.print(" rpm - count: ");
Serial.println(count);
}
void counter()
{
count++;
}Output
14:55:52.743 -> The fan speed is 1470 rpm - count: 490
14:55:53.765 -> The fan speed is 1470 rpm - count: 491
14:55:54.780 -> The fan speed is 1470 rpm - count: 490
14:55:55.751 -> The fan speed is 1470 rpm - count: 490
14:55:56.772 -> The fan speed is 1470 rpm - count: 490
14:55:57.787 -> The fan speed is 1470 rpm - count: 490
Oops, made adjustments, 12V to barrel now, still same issue with RPM.
RPM signal is 1.81V.
What do you mean Jumpered Pin2 to Pin3? Don't understand that?
Cheers, Rob.
EDIT: Tried your code change pinMode(2,INPUT_PULLUP); and still same results.
How do you measure this? Do you have a scope? The rpm signal needs to be 5v high, but if it is a pulse, you may be measuring an average voltage.
What do you mean Jumpered Pin2 to Pin3? Don't understand that?
I connected a wire between pin2 and pin3, so that the interrupt on pin2 was reading the pwm output on pin 3. This was to test your code. No fan involved.
EDIT: My suspicion is that there is an internal pullup to 12v with the hall sensor outputting the pulses, and you need to have this input to 5V for the Arduino. Hopefully you have not damaged the Arduino.
If that is the signal you are getting at pin2, then your code should be able to count the pulses.
You were asked previously about the ground connections. Can you document you wiring.
Document beyond the Visio diagram? I can take some pictures ... here I put my Thermal Image Camera to the Arduino and the Buck converter and "I think" they are running within tolerance ... 124 F and 102 F ... I assume that's safe temps?
I'll get some pics of my wiring up in a moment ... I'm currently running a power source from my Siglent SPD3303X (3 channel) one emulates 24V-13A PSU I plan to use and the other channel is 12V for fan ...
As @TomGeorge said
Can you check that ALL gnds are connected together?
Especially through the DC-DC converters.
To be certain, I would run a jumper from pin1 on the 4 pin fan to a ground pin on the arduino. Connect the grounds after the two converters.
Verify that all those 4 black dupont connecting wires to ground on the breadboard are good. And that the holes on the bread board ground connect to each other.
I'm more of a software person than hardware, but I have trouble believing that the scope signal you showed of the 5V pulse can not be read by an interrupt on pin 2 if the grounds are properly connected.
The point was the non-ISR code, not the ISR code.
Your Arduino Uno is an 8-bit AVR. Access to data types greater than 8-bits are non-Atomic. Every access to them requires multiple machine instructions. So, in your non-ISR code (i.e. the loop() function), an interrupt could occur between low and high byte access. The ISR could then modify the variable corrupting the access in loop().
I did miss that this in not the case in your code because, as @ cattledog pointed out, you're doing all the attach / detach flailing inside your loop(). So it's not your problem. But, your attach / detach technique is unorthodox to say the least and not what an experienced coder would recommend.
That being said, I'd follow the thread that @ cattledog is pulling on first . Once that is fixed, correct the code so that you only need to attachInterrupt() once in setup(). Also, once you add the DHT22 stuff back in, check the library (as I recommended) for interrupt disable / enable to meet device timing.
Rolling your own 'delay(1000)' is also unorthodox.
Interesting ... millis() returns unsigned long (32bit), but an unsigned int is 16bit, the only 8bit types are boolean, byte, char, unsigned char. My "count" variable is sufficient at 16bit since it gets reset in the loop and would never exceed a value of 65535. I see the warning to stay away from float and I don't use any.
I've gone thru several code (above) iterations with identical results. What would you recommend?
Verified grounding is all good ... agree the dupont connecting wires leave much to be desired, move to a solder junction.
// UNO R3
// Set a fan speed 0-255 (Corsair SP120 4 wire PWM) 12V DC 0.25A
// 4 Black wires with pin 1 Identified ... see reference: https://landing.coolermaster.com/faq/3-pin-and-4-pin-fan-wire-diagrams/
const int fan_control_pin = 3;
unsigned long start_time;
volatile word count = 0;
void setup() {
// put your setup code here, to run once:
pinMode(fan_control_pin, OUTPUT);
analogWrite(fan_control_pin, 51);
Serial.begin(9600);
attachInterrupt(digitalPinToInterrupt(2), counter, RISING);
}
void loop() {
start_time = millis();
Serial.print("Start time: ");
Serial.print(start_time);
Serial.print(" count: ");
Serial.println(count);
if (count > 3000)
{
count = 0;
}
}
void counter()
{
count++;
}
I simplified my code just to report back on interrupt "count" and to make sure "count" would reset to 0. The output as follows:
fan_control_pin 51 -- 30-33ms loop interval -- count increment 0 to 1
fan_control_pin 127 -- 30-33ms loop interval -- count increment 1 to 2
fan_control_pin 255 -- 30-33ms loop interval -- count increment 2 to 3
"count" did reset to 0 when it exceeded 3000.
From this data I would assume the Interrupt is working "as expected" ... is that a valid assumption?
Cheers, Rob.
It certainly appears to me that the interrupt on pin 2 is responding to the fan tachometer signal.
Did you try the self test previously mentioned where the Arduino pwm signal was connected to pin 2. That would prove also that pin 2 has not been damaged.
Did you see if your code with the one second counting period works with whatever wiring arrangement you now have?
Did you change anything in the hardware? It sounds like you may have changed the dupont wires for soldered connections.
I tested the new simple sketch prior to making any wiring ground changes.
Although I didn't see any issues with grounding I went ahead and solder them together.
Yes on PWM signal to pin 2 ... I also tested with my waveform generator at various frequencies ... seems fine with my simple loop code above.
I'll try adding the one second count back and see what happens.
Some progress, this code is working correctly for just RPM of Fan:
// UNO R3
// Set a fan speed 0-255 (Corsair SP120 4 wire PWM) 12V DC 0.25A
// 4 Black wires with pin 1 Identified ... see reference: https://landing.coolermaster.com/faq/3-pin-and-4-pin-fan-wire-diagrams/
const int fan_control_pin = 3;
unsigned long start_time;
volatile word count = 0;
int rpm;
void setup() {
// put your setup code here, to run once:
pinMode(fan_control_pin, OUTPUT);
analogWrite(fan_control_pin, 51);
Serial.begin(9600);
attachInterrupt(digitalPinToInterrupt(2), counter, RISING);
}
void loop() {
start_time = millis();
count = 0;
while ((millis() - start_time) < 1000)
{
// 1 second pause so count increments
}
rpm = (count / 2) * 60;
Serial.print("The fan speed is ");
Serial.print(rpm);
Serial.print(" rpm - count: ");
Serial.println(count);
}
void counter()
{
count++;
}
Here is the output for 20% load, 50% load, 100% load:
So now lets see what happens when I add the DHT22 code back ... stay tuned ...
EDIT: I should mention that what appears to be the source of my issues was having:
analogWrite(fan_control_pin, 51);
in the loop() section where it was being set every time ... not sure why I had done that in my testing ... old age brain fade perhaps. Typically I would only want to set this based on temp/humidity changes and that will be updated every 3 seconds (worst case if threshold was bouncing above/below) or so not every 30ms or so.
So the moral of this story seems to be my coding logic ... however, DHT22 integration pending but I'm now armed with more knowledge so hopefully I can get my code right.
Cheers, rob.