I'm trying to write a program that controls two LEDs through a serial input. They can be turned on and off individually or together through several toggles (state A for one, state B for the other, and state Z for both), and can also be dimmed up and down. I'm positive the serial input is working correctly, cause I'm tracking it. And sometimes the program works correctly. But sometimes is has strange blips where, for instance, light B suddenly turns on when I'm only dealing with state A. Or vice versa.
I feel like I need new eyes, so I was hoping someone here might be able to help.
Thanks in adavnce! Here's the code:
const int ledRed = 5;
const int ledGreen = 6;
int input;
int state = '0';
int lastState = '0';
int lastDimA = 0;
int lastDimB = 0;
int on_offA = -1;
int on_offB = -1;
void setup() {
Serial.begin(9600);
pinMode(ledRed, OUTPUT);
pinMode(ledGreen, OUTPUT);
}
void loop() {
if (Serial.available()>0) {
int input = Serial.read(); //read data
switch (input) {
case 'A':
lastState = state;
state = 'A';
if(state != lastState) {
on_offA *= -1;
}
break;
case 'B':
lastState = state;
state = 'B';
if(state != lastState) {
on_offB *= -1;
}
break;
case 'Z':
lastState = state;
state = 'Z';
if(state != lastState) {
on_offA *= -1;
on_offB *= -1;
}
break;
case '0':
state = '0';
break;
default:
if (state == 'A') {
if (on_offA == 1) {
analogWrite(ledRed, input);
lastDimA = input;
}
}
else if (state == 'B') {
if (on_offB == 1) {
analogWrite(ledGreen, input);
lastDimB = input;
}
}
}
if (on_offA == 1) { // set the LED using the state of the toggle
analogWrite(ledRed, lastDimA);
}else {
digitalWrite(ledRed, LOW);
}
if (on_offB == 1) { // set the LED using the state of the toggle
analogWrite(ledGreen, lastDimB);
}else {
digitalWrite(ledGreen, LOW);
}
}
}
What have you got the line terminator set to in the Serial monitor ? If it is anything other than 'No line ending' then 1 or 2 extra characters will be sent when you press Return and these will trigger your default case.
Can you please explain how the dimming up and down works ?
No, 'A' is not a state. A state is something like LED on pin 5 on. You need more meaningful names for some of your variables. Perhaps serialByte or something. But, not state.
In particular the value of state affects both LEDs, yet you are changing it for both, so any update that affects
state is likely to change both LEDs.
Why do you need the state variable? What does it represent? Give it a more meaningful name and your
code will be easier to understand and bad coding will show up more obviously. You already have 4 state
variables, lastDimA/B, on_offA/B which have meaningful names. Do you actually need more state?
UKHeliBob: I'm not sure what I have my line terminator set to. How do I check that?
PaulS/MarkT:
state is a throw away name that I just threw in there. The reason I need a variable for "state", or whatever I end up calling it, is to differentiate between inputs such as 'A', 'B', 'Z', or '0', which tell my arduino which LEDs to interact with, and inputs such as 0-255, which tell my arduino dimming levels for a specified LED. I previously didn't have "state" and my code was a lot harder to think out, cause I had to enumerate a lot of conditions before doing anything.
The inputs I'm getting from my serial input program are 'A', 'B', 'Z', '0' and the range of numbers from 0-255, as stated above. When my program receives an 'A' or 'B', it is immediately followed by a number (aka: A 111 A 110 A 109, etc...). Before I used the state variable, I was treating all inputs as my state, which meant I had to read my serial input twice in each loop, so as to get the dimming level given for a specified light. With state, I can check only once, cause I'm treating the two different types of inputs differently.
Assuming that explanation clears up my reason for using a state variable, is your other criticism of "state" as a variable simply that you don't approve of simplistic names? or does "state" hold some special importance as far as arduinos and therefore should be avoided? If it's simply the former, thanks for the suggestion, but I think I'll stick with "state" for now...
I realized I forgot to respond to one other comment. State would not be equivalent to Serialbyte. Input is Serialbyte, since both of these things include both 'A', 'B', 'Z' and '0', and 0-255. State, is a special subset of these, dim is now the other... I changed a couple things that I hope make the code clearer:
const int ledRed = 5;
const int ledGreen = 6;
int input;
int state = '0';
int lastState = '0';
int dimA;
int dimB;
int lastDimA;
int lastDimB;
int on_offA = -1;
int on_offB = -1;
void setup() {
Serial.begin(9600);
pinMode(ledRed, OUTPUT);
pinMode(ledGreen, OUTPUT);
}
void loop() {
if (Serial.available()>0) {
int input = Serial.read(); //read data
switch (input) {
case 'A': // deal with red light only
lastState = state;
state = input;
if(state != lastState) {
on_offA *= -1;
}
break;
case 'B': // deal with green light only
lastState = state;
state = input;
if(state != lastState) {
on_offB *= -1;
}
break;
case 'Z': // deal with both lights simultaneously
lastState = state;
state = input;
if(state != lastState) {
on_offA *= -1;
on_offB *= -1;
}
break;
case '0': // deal with neither light;
state = '0';
break;
default: // apply new dim levels;
if (state == 'A') {
if (on_offA == 1) {
dimA = input;
analogWrite(ledRed, dimA);
lastDimA = dimA;
}
}
else if (state == 'B') {
if (on_offB == 1) {
dimB = input;
analogWrite(ledGreen, dimB);
lastDimB = dimB;
}
}
}
if (on_offA == 1) { // turn on red LED to the last dim level is was at when it was previously on
analogWrite(ledRed, lastDimA);
}else {
digitalWrite(ledRed, LOW);
}
if (on_offB == 1) { // turn on green LED to the last dim level is was at when it was previously on
analogWrite(ledGreen, lastDimB);
}else {
digitalWrite(ledGreen, LOW);
}
}
}
I think I see what the problem is. You are sending, from the Serial Monitor, something like "198" to set the brightness. The Serial.read() function returns '1', then '9', then '8', where you appear to be expecting it to return 198.
You need to send something following the value, like "198\n" or "198\r" or "198\n\r". The Serial Monitor has a drop down menu at the bottom that lets you select what to automatically append (nothing, carriage return, line feed, or carriage return and line feed) to everything sent.
Then, you need to collect the characters read into an array, until the delimiter is encountered, and convert the array to an int when the delimiter arrives, if the serial data represents brightness level.
Figured I should let you guys know that I figured out my problem. The program was interpreting the number 65 and 66 as ascii values. So it was switching states at the wrong times. So I just made the serial program skip those numbers when giving dimming levels, and voila.