I am driving BLDC motor using Hall effect sensors. I measure current and rpm and send it every second to Serial monitor in Arduino IDE. What I do is that I have Timer 5 configured to call an ISR every second. In the ISR I just write "true" to variable rpmUpdate and then I test this variable in main loop(). If it is true I send the data over serial and then reset it to "false".
The problem is that from time to time and especially at higher (>1000) rpm the process of sending data disturbs the motor. It keeps spinning but you can hear (and feel if you place a hand on the motor) some "disruption".
My idea is that the sending takes relatively a lot of time and it delays a proper commutation which is handled in main loop() using switch(). My second idea is that the Serial.println() affects also some pins which I use for PWM or interrupt (I use Mega). I did not found what exactly Serial.println() does when you call it so I do not know how it could possibly mess with my program. Is there any alternative to Serial.println()?
The sketch is in the attachment.
I will be grateful for any advice.
Have a nice day.
That code is much too complex for me to make sense of it without a great many explanatory comments.
What is the purpose of line 344 Serial.parseInt(). That is a blocking function and not appropriate in any program that pretends to be responsive.
if you just wany to send data once per second it is not necessary to use an interrupt. You can use millis() for that sort of timing and IMHO make the code easier to follow.
Robin2:
What is the purpose of line 344 Serial.parseInt(). That is a blocking function and not appropriate in any program that pretends to be responsive.
Sorry, this is a mistake. This line should be empty. I must have forgotten to delete that line. BTW I did some research and now I know that Serial.parseInt() can cause dealy problems so I am gonna avoid it in my next code.
Robin2:
if you just wany to send data once per second it is not necessary to use an interrupt. You can use millis() for that sort of timing and IMHO make the code easier to follow.
This post actually got me an idea. I tried to disable global interrupts before sending the data and enable it again after sending:
AFAIK if you call Serial.print() with interrupts disabled all that happens is the data is put in the Serial Output Buffer. The actual sending won't happen until the interrupts are enabled.
Note that if (while interrupts are disabled) you send more data than the buffer can hold Serial.print will block and the code will never get to the point of re-enabling the interrupts to clear the blockage.
With all this in mind I'm not sure why disabling interrupts in the way that you do can have any benefit.
Of course you have all the source code for Serial at your disposal if you feel like studying it.
1) You are doing some things with an interrupt (in the "backgound") that would be better if they were just in loop() (in the "foreground"). Specifically:
Just test for ADC complete instead of using an interrupt to set a flag. You don't handle it any faster with the interrupt, because the value stays there until another conversion completes. (I think you have to manually clear the ADIF.) Using an ISR and then testing the volatile flag is actually slower than just polling for it in loop(). Um, this will also avoid doing a floating-point operation in an ISR. o_O
rpmUpdate can just run off of millis()
interruptTmr1 can just run off of millis()
These interrupts will interfere with the primary foreground task, controlling the bridge.
Serial.print will also interfere, because an interrupt is generated for each character that is transmitted. Sending more than 64 chars at once (or more than an average of 1 char per 1.04ms) will cause blocking, which kills the motor control. Bad juju.
A long time ago, I hacked HardwareSerial into a PollHWSerial that doesn't use interrupts. All the printing capabilities are the same, you just have to call PollSerial.run() to keep sending the characters. Just put that line somewhere in loop.
The version I have attached does not implement input, but it wouldn't be too hard to copy those portions in if you wanted to. Note: you have to use PollSerial everywhere; you can't use Serial.xxx(...) anywhere, only PollSerial.xxx(...). It will still block after 64 characters, so you still have to pace your output.
The TIMER4 input capture should remain as an interrupt. However, the floating-point math should be moved to loop, so the ISR should just save ICR4, clear the overflow flag, and set a "captured" flag.
2) Disabling global interrupts is probably bad. It will keep all ISRs from being called, and would hang if you tried to send too much (depends on your IDE version, too).
3) You might as well move the PIND masking into loop(), out of the hall ISRs:
I deleted ADC ISR and moved it to loop. I also moved the RPM calculations and bit masking of PIND to loop. I didn't delete the ISRs from Timers. The main reason is that this project is a part of my bachelor thesis and I think it' s gonna look "more professional" with ISR than with millis() :).
Now the motor runs smoothly even without blocking the interrupts during sending the data.
Even with all the ISRs I thought it would be a piece of cake for 16 MHz processor to handle a motor whose speed is next to nothing in comparison.