-
Arduino strange problem (variable assignment not working in case statement):
Posted by Powermax on 09 Jan, 2017 20:40 -
I have been recently working on developing a lab bench power supply unit (thread found here: https://www.eevblog.com/forum/projects/anything-wrong-with-this-linear-psu-design/) and plan to use a small arduino (ATmega328p based clone board) but have ran into a very strange problem while using the switch case statement.
Below is my code. Sorry if it's poorly written and sloppy, I am no expert coder (and typically rely on hardware solutions before turning over to software solutions
one example is my use of a capacitor RC network to act as a digital buffer on an input pin. You'll see what I mean if you read the code.Code: [Select]const byte VoltageSetPin = 10;
const byte CurrentSetPim = 9;
// mode 0 = voltage set
// mode 1 = current set
// mode 2 =
byte mode = 0;
const int pinA = 2; // One of the pins from the encoder
const int pinB = 3; // The other pin from the encoder
const int pinC = 4; // The encoder is pushable, this pin is used to switch between modes 0 and 1
const int button1 = A1; // not yet implemented
const int button2 = A2; // not yet implemented
const int button3 = A3; // not yet implemented
const int button4 = A4; // not yet implemented
bool PastValueA = LOW;
bool PastValueB = LOW;
bool ValueA = LOW;
bool ValueB = LOW;
bool ValueC = LOW;
int vel = 0;
float VoltageSet = 0;
float CurrentSet = 0;
// this code waits for the specified pin to charge back up, the time it takes will depend on the RC time constant.
void WaitForHigh(bool pin){
while(!digitalRead(pin)){
pinMode(pin, INPUT_PULLUP);
}
pinMode(pin, INPUT);
}
/*
* Events 1 through 4 are interupt handlers for the the rotary encoder. They trigger on transisions and measure the state of the other pin to determine rotation direction.
* because they will get triggered several times during the delay at the beginning, that will give my an indication of the frequency of rotation. I want to change this to instead
* calculate the amount of microseconds elapsed between triggers as this is a more accurate method (no chance of triggers not occuring during other parts of the code.
*
* switch is supposed to be faster than if/else blocks, so I used them instead.
*/
void event1(){
switch(digitalRead(pinB)){
case HIGH: vel++; break;
case LOW: vel--; break;
}
}
void event2(){
switch(digitalRead(pinA)){
case LOW: vel++; break;
case HIGH: vel--; break;
}
}
void event3(){
switch(digitalRead(pinB)){
case LOW: vel++; break;
case HIGH: vel--; break;
}
}
void event4(){
ValueA = digitalRead(pinA);
switch(digitalRead(pinA)){
case HIGH: vel++; break;
case LOW: vel--; break;
}
}
void setup() {
/* These pins are used by the encoder. I want to be able to capture every event. */
pinMode(pinA, INPUT_PULLUP);
pinMode(pinB, INPUT_PULLUP);
/*
* because I ran out of interrupts for the rest of these pins, I have added an external 0.1uF capacitor to the pins
* to act as a rough-and-ready buffer. As soon as the switch is pressed, the charged capacitor is shorted to ground
* and the result is that it will stay discharged, at least for a reasonable length of time (judged by the RC time
* constant.) This capacitor also eliminates the need for debouncing.
*
* Once the code finally gets around to digitalRead();ing the pin, the capacitor will keep the pin at a LOW
* state until after that operation and then we can recharge and reset the capacitor. A 100K or larger resistor can
* help ensure nothing bleeds away the charge of the capacitor. If the value is too low, then the capacitor may
* charge back up before the long code gets around to digitalRead()ing it. If it's too high (basically non-existent)
* then there is a change internal leakage current will cause the voltage to sag to zero leading to a false trigger.
*
* the code directly below initially charges the capacitor so the code does not start thinking that the button was pressed.
*/
pinMode(pinC, INPUT_PULLUP);
//pinMode(button1, INPUT_PULLUP);
//pinMode(button2, INPUT_PULLUP);
//pinMode(button3, INPUT_PULLUP);
//pinMode(button4, INPUT_PULLUP);
WaitForHigh(pinC);
//pinMode(pinC, INPUT);
//pinMode(button1, INPUT);
//pinMode(button2, INPUT);
//pinMode(button3, INPUT);
//pinMode(button4, INPUT);
Serial.begin(9600);
interrupts();
attachInterrupt(digitalPinToInterrupt(pinA), event1, RISING);
attachInterrupt(digitalPinToInterrupt(pinB), event2, RISING);
attachInterrupt(digitalPinToInterrupt(pinA), event3, FALLING);
attachInterrupt(digitalPinToInterrupt(pinB), event4, FALLING);
}
void loop() {
vel = 0; // it is mandatory to set the velocity back to zero after the code loops and finishes. This ensures that there is an accurate reading.
delay(100); // simulates reallly long-ass code, allows time for the interrupts to gather data and count how revolutions on the encoder
// if you pressing the switch is detected, then first pull the pin back high quickly but safely (using INPUT_PULLUP)
// then set pinC back to INPUT (for normal operation) and
if(!digitalRead(pinC)){
WaitForHigh(pinC);
pinMode(pinC, INPUT);
switch(mode){
case 0:
mode = 1;
break;
case 1:
mode = 0;
break;
}
}
switch(mode){
// Adjust voltage via the function below. If it is out of range, then clamp it by overwriting the value with the maximum value.
case 0:
// I multiply velocity to it's absolute value so that I get primitive velocity control. (if you turn the pot 2X fast, the voltage will 4X faster.)
//Serial.print("Set Voltage: ");
VoltageSet += vel * vel * vel * 0.001;
if(VoltageSet > 15){
//Serial.print("upper V threshold reached! ");
VoltageSet = 15;
}
else if(VoltageSet < 0){
//Serial.print("lower V threshold reached! ");
VoltageSet = 0;
}
break;
// Adjust current via the function below. If it is out of range, then clamp it by overwriting the value with the maximum value.
case 1:
// I multiply velocity to it's absolute value so that I get primitive velocity control. (if you turn the pot 2X fast, the voltage will 4X faster.)
//Serial.print("Set Current:");
CurrentSet += vel * vel * vel * 0.002;
if(CurrentSet > 5){
//Serial.print("upper I threshold reached! ");
CurrentSet = 5;
}
else if(CurrentSet < 0){
//Serial.print("lower V threshold reached! ");
CurrentSet = 0;
}
break;
// select saved values
case 2: ; break;
}
Serial.println(/*"pinC: " + String(ValueC) + */ "\t vel:" + String(vel) + '\t' + "Mode " + String(mode) + "\t Set Voltage: " + String(VoltageSet) + "\t Set Current: " + String(CurrentSet));
}
More specifically, here is the section I am having a problem with:Code: [Select]switch(mode){
// Adjust voltage via the function below. If it is out of range, then clamp it by overwriting the value with the maximum value.
case 0:
// I multiply velocity to it's absolute value so that I get primitive velocity control. (if you turn the pot 2X fast, the voltage will 4X faster.)
//Serial.print("Set Voltage: ");
VoltageSet += (vel * vel * vel * 0.001);
if(VoltageSet > 15){
//Serial.print("upper V threshold reached! ");
VoltageSet = 15;
}
else if(VoltageSet < 0){
//Serial.print("lower V threshold reached! ");
VoltageSet = 0;
}
break;
// Adjust current via the function below. If it is out of range, then clamp it by overwriting the value with the maximum value.
case 1:
// I multiply velocity to it's absolute value so that I get primitive velocity control. (if you turn the pot 2X fast, the voltage will 4X faster.)
//Serial.print("Set Current:");
CurrentSet += (vel * vel * vel * 0.002);
if(CurrentSet > 5){
//Serial.print("upper I threshold reached! ");
CurrentSet = 5;
}
else if(CurrentSet < 0){
//Serial.print("lower V threshold reached! ");
CurrentSet = 0;
}
break;
// select saved values
case 2: ; break;
}
The particular code of interest is line 145 and line 160.
Code: [Select]
CurrentSet += (vel * vel * vel * 0.002);
Code: [Select]
VoltageSet += (vel * vel * vel * 0.001);
It appears as if these lines do not properly execute unless I uncomment the Serial.print() command directly above them. I feel this may be an issue with the compiler not interpreting these for some strange reason. This code is implementing velocity control over a rotary encoder. "val" is the frequency of rotation, which itself is not very accurate due to it's simple implementation, but I'll worry about improving it later. If I instead assign a literal expression in place of the "val" variable, then it works fine. I can have it steadily increment the voltage or current set up and up X number of times per loop. As soon as I replace that with the variable expression it stops working, unless I place a Serial.print statement directly above it.
I don't get it...
Any ideas? -
Hi,
I have been recently working on developing a lab bench power supply unit (thread found here: https://www.eevblog.com/forum/projects/anything-wrong-with-this-linear-psu-design/) and plan to use a small arduino (ATmega328p based clone board) but have ran into a very strange problem while using the switch case statement.
Below is my code. Sorry if it's poorly written and sloppy, I am no expert coder (and typically rely on hardware solutions before turning over to software solutions one example is my use of a capacitor RC network to act as a digital buffer on an input pin. You'll see what I mean if you read the code.Code: [Select]const byte VoltageSetPin = 10;
const byte CurrentSetPim = 9;
// mode 0 = voltage set
// mode 1 = current set
// mode 2 =
byte mode = 0;
const int pinA = 2; // One of the pins from the encoder
const int pinB = 3; // The other pin from the encoder
const int pinC = 4; // The encoder is pushable, this pin is used to switch between modes 0 and 1
const int button1 = A1; // not yet implemented
const int button2 = A2; // not yet implemented
const int button3 = A3; // not yet implemented
const int button4 = A4; // not yet implemented
bool PastValueA = LOW;
bool PastValueB = LOW;
bool ValueA = LOW;
bool ValueB = LOW;
bool ValueC = LOW;
int vel = 0;
float VoltageSet = 0;
float CurrentSet = 0;
// this code waits for the specified pin to charge back up, the time it takes will depend on the RC time constant.
void WaitForHigh(bool pin){
while(!digitalRead(pin)){
pinMode(pin, INPUT_PULLUP);
}
pinMode(pin, INPUT);
}
/*
* Events 1 through 4 are interupt handlers for the the rotary encoder. They trigger on transisions and measure the state of the other pin to determine rotation direction.
* because they will get triggered several times during the delay at the beginning, that will give my an indication of the frequency of rotation. I want to change this to instead
* calculate the amount of microseconds elapsed between triggers as this is a more accurate method (no chance of triggers not occuring during other parts of the code.
*
* switch is supposed to be faster than if/else blocks, so I used them instead.
*/
void event1(){
switch(digitalRead(pinB)){
case HIGH: vel++; break;
case LOW: vel--; break;
}
}
void event2(){
switch(digitalRead(pinA)){
case LOW: vel++; break;
case HIGH: vel--; break;
}
}
void event3(){
switch(digitalRead(pinB)){
case LOW: vel++; break;
case HIGH: vel--; break;
}
}
void event4(){
ValueA = digitalRead(pinA);
switch(digitalRead(pinA)){
case HIGH: vel++; break;
case LOW: vel--; break;
}
}
void setup() {
/* These pins are used by the encoder. I want to be able to capture every event. */
pinMode(pinA, INPUT_PULLUP);
pinMode(pinB, INPUT_PULLUP);
/*
* because I ran out of interrupts for the rest of these pins, I have added an external 0.1uF capacitor to the pins
* to act as a rough-and-ready buffer. As soon as the switch is pressed, the charged capacitor is shorted to ground
* and the result is that it will stay discharged, at least for a reasonable length of time (judged by the RC time
* constant.) This capacitor also eliminates the need for debouncing.
*
* Once the code finally gets around to digitalRead();ing the pin, the capacitor will keep the pin at a LOW
* state until after that operation and then we can recharge and reset the capacitor. A 100K or larger resistor can
* help ensure nothing bleeds away the charge of the capacitor. If the value is too low, then the capacitor may
* charge back up before the long code gets around to digitalRead()ing it. If it's too high (basically non-existent)
* then there is a change internal leakage current will cause the voltage to sag to zero leading to a false trigger.
*
* the code directly below initially charges the capacitor so the code does not start thinking that the button was pressed.
*/
pinMode(pinC, INPUT_PULLUP);
//pinMode(button1, INPUT_PULLUP);
//pinMode(button2, INPUT_PULLUP);
//pinMode(button3, INPUT_PULLUP);
//pinMode(button4, INPUT_PULLUP);
WaitForHigh(pinC);
//pinMode(pinC, INPUT);
//pinMode(button1, INPUT);
//pinMode(button2, INPUT);
//pinMode(button3, INPUT);
//pinMode(button4, INPUT);
Serial.begin(9600);
interrupts();
attachInterrupt(digitalPinToInterrupt(pinA), event1, RISING);
attachInterrupt(digitalPinToInterrupt(pinB), event2, RISING);
attachInterrupt(digitalPinToInterrupt(pinA), event3, FALLING);
attachInterrupt(digitalPinToInterrupt(pinB), event4, FALLING);
}
void loop() {
vel = 0; // it is mandatory to set the velocity back to zero after the code loops and finishes. This ensures that there is an accurate reading.
delay(100); // simulates reallly long-ass code, allows time for the interrupts to gather data and count how revolutions on the encoder
// if you pressing the switch is detected, then first pull the pin back high quickly but safely (using INPUT_PULLUP)
// then set pinC back to INPUT (for normal operation) and
if(!digitalRead(pinC)){
WaitForHigh(pinC);
pinMode(pinC, INPUT);
switch(mode){
case 0:
mode = 1;
break;
case 1:
mode = 0;
break;
}
}
switch(mode){
// Adjust voltage via the function below. If it is out of range, then clamp it by overwriting the value with the maximum value.
case 0:
// I multiply velocity to it's absolute value so that I get primitive velocity control. (if you turn the pot 2X fast, the voltage will 4X faster.)
//Serial.print("Set Voltage: ");
VoltageSet += vel * vel * vel * 0.001;
if(VoltageSet > 15){
//Serial.print("upper V threshold reached! ");
VoltageSet = 15;
}
else if(VoltageSet < 0){
//Serial.print("lower V threshold reached! ");
VoltageSet = 0;
}
break;
// Adjust current via the function below. If it is out of range, then clamp it by overwriting the value with the maximum value.
case 1:
// I multiply velocity to it's absolute value so that I get primitive velocity control. (if you turn the pot 2X fast, the voltage will 4X faster.)
//Serial.print("Set Current:");
CurrentSet += vel * vel * vel * 0.002;
if(CurrentSet > 5){
//Serial.print("upper I threshold reached! ");
CurrentSet = 5;
}
else if(CurrentSet < 0){
//Serial.print("lower V threshold reached! ");
CurrentSet = 0;
}
break;
// select saved values
case 2: ; break;
}
Serial.println(/*"pinC: " + String(ValueC) + */ "\t vel:" + String(vel) + '\t' + "Mode " + String(mode) + "\t Set Voltage: " + String(VoltageSet) + "\t Set Current: " + String(CurrentSet));
}
More specifically, here is the section I am having a problem with:Code: [Select]switch(mode){
// Adjust voltage via the function below. If it is out of range, then clamp it by overwriting the value with the maximum value.
case 0:
// I multiply velocity to it's absolute value so that I get primitive velocity control. (if you turn the pot 2X fast, the voltage will 4X faster.)
//Serial.print("Set Voltage: ");
VoltageSet += (vel * vel * vel * 0.001);
if(VoltageSet > 15){
//Serial.print("upper V threshold reached! ");
VoltageSet = 15;
}
else if(VoltageSet < 0){
//Serial.print("lower V threshold reached! ");
VoltageSet = 0;
}
break;
// Adjust current via the function below. If it is out of range, then clamp it by overwriting the value with the maximum value.
case 1:
// I multiply velocity to it's absolute value so that I get primitive velocity control. (if you turn the pot 2X fast, the voltage will 4X faster.)
//Serial.print("Set Current:");
CurrentSet += (vel * vel * vel * 0.002);
if(CurrentSet > 5){
//Serial.print("upper I threshold reached! ");
CurrentSet = 5;
}
else if(CurrentSet < 0){
//Serial.print("lower V threshold reached! ");
CurrentSet = 0;
}
break;
// select saved values
case 2: ; break;
}
The particular code of interest is line 145 and line 160.
Code: [Select]
CurrentSet += (vel * vel * vel * 0.002);
Code: [Select]
VoltageSet += (vel * vel * vel * 0.001);
It appears as if these lines do not properly execute unless I uncomment the Serial.print() command directly above them. I feel this may be an issue with the compiler not interpreting these for some strange reason. This code is implementing velocity control over a rotary encoder. "val" is the frequency of rotation, which itself is not very accurate due to it's simple implementation, but I'll worry about improving it later. If I instead assign a literal expression in place of the "val" variable, then it works fine. I can have it steadily increment the voltage or current set up and up X number of times per loop. As soon as I replace that with the variable expression it stops working, unless I place a Serial.print statement directly above it.
I don't get it...
Any ideas?
Just a quick note, define vel as volatile.
Cheers.
---
Daniel -
Hi Powermax,
I have checked your code.
The first thing I have noticed, is that you use the same val variable for both the current and voltage setting, but this is not the problem currently.
The main problem could be that val is set to zero in the beginning of the loop, then comes a 100ms delay which is too sort for the quadrature encoder to cause exceptions and modify the value of val. (for a full rotation there is 4 state changes from the encoder, this means that if you want a state change in 100ms, you have to rotate the knob faster than 2.5 rotation/second). The proof is that when you enable the Serial.print() it adds an additional delay which improves the situation.
On the other hand, I don't think your quadrature encoder handling could work, because you have to implement some sort of FSM to track in which state the encoder is and which direction it is turned. (See attached images).
I would suggest to use the Encoder library (I have used this with success) from Paul Stoffregen (you can install it trough the Library Manager of the Arduino IDE). Here is explanation: https://www.pjrc.com/teensy/td_libs_Encoder.html
This library does not support acceleration but you can implement it, by measuring the time between state changes and calculate the rotation speed from it.
There is another library which supports acceleration (I haven't used this) : https://github.com/0xPIT/encoder/tree/arduino
Regards,
Ákos
-
I posted this in the arduino forum and almost immediately got replies, including the fix. I did not know I had to include "volatile" modifier with variables that are modified in the interrupts. That fixed it.
-
Hi Powermax,
I have checked your code.
The first thing I have noticed, is that you use the same val variable for both the current and voltage setting, but this is not the problem currently.
The main problem could be that val is set to zero in the beginning of the loop, then comes a 100ms delay which is too sort for the quadrature encoder to cause exceptions and modify the value of val. (for a full rotation there is 4 state changes from the encoder, this means that if you want a state change in 100ms, you have to rotate the knob faster than 2.5 rotation/second). The proof is that when you enable the Serial.print() it adds an additional delay which improves the situation.
On the other hand, I don't think your quadrature encoder handling could work, because you have to implement some sort of FSM to track in which state the encoder is and which direction it is turned. (See attached images).
I would suggest to use the Encoder library (I have used this with success) from Paul Stoffregen (you can install it trough the Library Manager of the Arduino IDE). Here is explanation: https://www.pjrc.com/teensy/td_libs_Encoder.html
This library does not support acceleration but you can implement it, by measuring the time between state changes and calculate the rotation speed from it.
There is another library which supports acceleration (I haven't used this) : https://github.com/0xPIT/encoder/tree/arduino
Regards,
Ákos
Thanks for the input, I'll try to figure out what you mean and look into it. I do want to change my method of how I get velocity, by instead measuring the time elapsed between triggering the interrupts, so that I instead measure period instead or frequency over a specified interval. I do not currently know how to implement that.
You mentioned Serial.print adds a delay, but it should not be that much. Serial.print dumps the data into a buffer which then takes it's good sweet time to empty over the hardware UART, (as I learned while interning at NASA Langley) so the actual command executes pretty quick so long as the buffer stays empty enough. Purposely adding a delay() of even sevel milliseconds or really any other command did not fix it, I think it has something to do with the compiler's optimization algorithms causing trouble. Luckily the "Volatile" modifyer fixed this problem!
-
Hi Powermax,
You can use e.g. the TimerOne library for a start: http://playground.arduino.cc/Code/Timer1
You could check the value of vel in a timer interrupt and set a speed multiplicator. if there was n state change during e.g. 100ms, then the multiplicator could increased. if there was no state change, the multiplicator could be set to 0.
Regards,
Ákos -
I fixed the code, and now I am getting the full resolution for sure! However it's kind of not as good as expected. Not sure if the contacts inside on my garbage encoder are crusty, or if the code is simply not catching all the pulses. I may end up using the extra resolution as redundant ECC. If it triggers on more than 2 pulses then it counts, otherwise, no.
My encoder is notched, but it does a full cycle per 1/20th of a turn, and increments up 4 times per notch as a result. However sometimes it misses a pulse or 2 and sometimes I even get negative pulses. Maybe I'll download a proper library and see and compare the performance of my implementation against it. This is a problem because the last thing I want is a dicky voltage/current set. My Rigol DS1054Z is already annoying enough as it is with this type of problem, and it's way less severe with it.
-
I went ahead and got my arduino to output 14 bits of resolution natively, and used a 4 stages of RC networks (22K and 0.47uF) for filtering, but my UT61E claims 1.92mV of AC voltage still present, my Fluke 117 is OL. (but I think the fluke is wrong, I suspect the AC mv range is broke, as both AC and DC mV range have identical readings. I suspect maybe a blown DC blocking capacitor. Maybe time for a teardown?
Got it for like $25 paired with a cheap $2 garbage multimeter so no clue of it's origins.)
So after messing with this, I think I might be able to achieve 4 sig fig accuracy, although it might be hard. My filtering takes almost a second to settle, which is not too huge of a deal. At the half point (output set to 7.5V to allow the PWM to have the strongest primary frequency content) those 3 millivolts will be multiplied by 3 in the supply design, which is 10 mV, which will make a significant error on the LSB particularly at lower voltages.
I would not mind that inaccuracy too much if it was not periodic, audible, visible on a scope, etc. It just bothers me! So a proper 12 bit to 14 bit DAC may be a better option.
-
I posted this in the arduino forum and almost immediately got replies, including the fix. I did not know I had to include "volatile" modifier with variables that are modified in the interrupts. That fixed it.
Well...."almost" "immediately" is almost correct.
Since you never heard of volatile before, it seems appropriate to suggest you read this:Quotehttp://www.gammon.com.au/interrupts
-
Even with the volatile fix your code is still unsafe in a way that (for this code) will only show up once in a while as unexplained behaviour.
The problem is that 'vel' is a 16-bit variable and your AVR is 8-bits. It requires two load/store instructions to fetch/store a 16-bit value to SRAM. Your non-IRQ code can have executed the first of the two load instructions when the IRQ happens and the IRQ code then overwrites both bytes of the 16-bit variable, corrupting it for the main code when the IRQ returns and executes the second of its load instructions.
For you, this will happen rarely if ever because of the way you are using vel as a simple counter but this is something to bear in mind for future. Luckily there is a very trivial fix. Either surround the access in your main code with cli()/sei() or use the atomic access macros in avr_libc. -
With many C-like compilers I've had problems with switch statements that has had a function as it's parameter. Sometimes they work, sometimes not, in a very unpredictable manner. Storing the function value in a variable and then using that variable as the switch parameter has solved this issue every time. Try it and see if you get more reliable results.
OWH! Another thing, you do not have any default: in your SWITCH statements. Those should be there, even though they do nothing. In your case you are mostly checking for 0 or 1, so either case 1: or case 0: should be a default: -
Interrupts are a huge trap for new players.
They seem like a beautiful answer to everything but in reality they cause many intermittent problems and hard to find bugs like these.
-
... and you can expect "special effects" when you tried to combine interrupt sources that comes from SPI ("guarded" with SPI transactions) and non-SPI sources/peripherals (at least on Arduino Due).
-
PWM output filter design, attached.
Also keep in mind a PWM output is presenting two variables in the
conversion transformation, duty cycle and Vdd rail stability/noise.
Regards, Dana.
-
... and you can expect "special effects" when you tried to combine interrupt sources that comes from SPI ("guarded" with SPI transactions) and non-SPI sources/peripherals (at least on Arduino Due).
If I read that right then the author (you?) doesn't understand that the pending IRQ bit in an interrupt register is set immediately that the peripheral wants to raise the interrupt. It has to be, the peripheral has nowhere else to stash it. It will not, however, be acted on by the MCU core unless the corresponding mask bit is enabled. Generally, toggling the mask bit ON will cause the MCU to immediately evaluate the pending bit and interrupt the core unless it's already in an IRQ, or the MCU supports interrupt priorities and the currently executing IRQ is of a higher priority than the IRQ that was just unmasked.