I am writing a program that has no less that five switch case statements. During the program serial data will be reviewed to make decisions whether to continue, save data, or to stop do to error or by request.
I have made a short sketch with some comments and code to try to show the area I need help in. I have a function called iso_compare_string. With that I want to check results from an array called "data". If my results do not match my "if' statement I want to control my switch case int. "insync" and enter my case "1" to allow a reset to be made using another function.
I think I am going to have problems with global and private variables. I don't know how to pass a int. from my function to change the switch case int. "insyc".
Could some one help me understand this. I have looked online for an answer and I have not found switch case mixed with function results that I could use in my situation.
const int dataStreamSize=69;
unsigned char data[dataStreamSize];
int insync = 0;
/////////////////////////function in question//////////////////
byte iso_compare_string (int insync)
{
if (((data[0] == 0x80 && data[1] == 0xF1) )) {
/// WE HAVE A MATCH AND 52 BYTES TO READ
insync = (0);
}
else{
//WE HAVE A PROBLEM NEED TO RESET
insync = (1);
}
}
void setup() {
// initialize serial communication:
Serial.begin(9600);
}
void loop() {
////// do serial communication to fill data array///////
//call iso_compare_string to check data
iso_compare_string();
// do something different depending on the
// communication value called insync:
switch (insync) {
case 0: // state when things are fine
Serial.println("dark");
//read 52 bytes and continue
break;
case 1: // state when a problem has a occured
Serial.println("we have a problem");
// run reset function
break;
}
}
You define the iso_compare_string() function as returning a byte but you never return one. To do this place a return statement at the end of this function:-
byte iso_compare_string (int insync)
{
if (((data[0] == 0x80 && data[1] == 0xF1) )) {
/// WE HAVE A MATCH AND 52 BYTES TO READ
insync = (0);
}
else{
//WE HAVE A PROBLEM NEED TO RESET
insync = (1);
}
return(insync);
}
Now you need to collect it at the other end:-
so use,
insync = iso_compare_string();
Note there are two totally different variables hear both called insync, that could be confusing.
If the variable you want to pass to the function is a global variable, there is really no need to pass it to the function.
If you still want to,
iso_compare_string(insync);
The iso_compare_string function is defined to return a byte, and yet there is no return statement to actually return anything. The global variable insync will not be impacted by the iso_compare_string function, since the name insync in the function returns to the value passed to the function, so that value will be lost when the function ends.
The solution depends on how you want to restructure the function.
Thanks for the reply's. Hopefully, I am getting closer to understanding. I have modified the code a bit to better explain my needs and I am also trying to use the advice given so far. I have changed the main loop a bit, and I have changed my function data type and got rid of the private int. insync.
In my situation my switch "int insync" is going to default to case"0" on start-up. I will then do communication to load the array called "data". Next, I call function "iso_compare_string()". Then, inside that function I will decide whether to change "insync" to equal "1" or "2". I then need the result to select case 1 or 2.
Keep in mind that later I will have other functions that will need to be able to change the switch "insync" also and therefore change the case that is being ran.
const int dataStreamSize=69;
unsigned char data[dataStreamSize];
int insync = 0;
/////////////////////////function in question//////////////////
int iso_compare_string ()
{
if (((data[0] == 0x80 && data[1] == 0xF1) )) {
/// WE HAVE A MATCH AND 52 BYTES TO READ
insync = (1);
}
else{
//WE HAVE A PROBLEM NEED TO RESET
insync = (2);
}
return(insync);
}
void setup() {
// initialize serial communication:
Serial.begin(9600);
}
void loop() {
// do something different depending on the
// communication value called insync:
switch (insync) {
case 0: ////// do serial communication to fill data array///////
//call iso_compare_string to check data
iso_compare_string();
Serial.println("checking data");
break;
case 1: // state when things are fine
Serial.println("normal communication");
//read 52 bytes and continue
break;
case 2: // state when a problem has a occurred
Serial.println("we have a problem");
// run reset function
break;
}
}
Then, inside that function I will decide whether to change "insync" to equal "1" or "2". I then need the result to select case 1 or 2.
You can't do that. Once the switch statement is entered only one of the case statement executes. If you change the value on which you are doing the switching then that value is only used next time the code encounters the switch statement.
Mike's right. If you need to change the value of insync when it is 0, and expect to execute different code as a result of that change, switch is NOT the way to go. You will need to use if tests (not if/else if, either).
Why the brackets? Are you worried about the evaluation order of 1? Or have you maybe forgotten a function call, like:
insync = f(1);
Whilst it compiles it makes one wonder what you were thinking of when you wrote it.
This looks fundamentally wrong:
switch (insync) {
case 0: ////// do serial communication to fill data array///////
//call iso_compare_string to check data
iso_compare_string();
Serial.println("checking data");
break;
The function iso_compare_string calculates insync, but you are testing its value before you know what it is! I know you set it as zero initially but it still looks strange. Anyway, if you must do it this way, you can use the returned result:
switch (insync) {
case 0: ////// do serial communication to fill data array///////
//call iso_compare_string to check data
Serial.println("checking data");
if( iso_compare_string() == 2)
return; // problem occurred
break;
@Grumpy Mike and PaulS, I am going to take your advice and move to "if" statements instead of case. My goal is to make the code as clean as possible but, it has to work first.
@Nick, The brackets were mistakes by me. I think moved them from another spot in the code and forgot to get rid of them.
Thanks to all of you for the help, now I know why I couldn't find any use of case mixed with functions, at least not like I was looking for.
Your use of case could be ok - a state machine does the same thing, insync is your state variable and controls which of your cases is executed. If on the other hand, you are hoping to execute more than one case per iteration of loop, you will need to do some redesign. Hard to tell with where you are so far.
While you're removing brackets, please get rid of the pair on the return statement too - unnecessary (and a pet peeve).
wildbill:
Your use of case could be ok - a state machine does the same thing, insync is your state variable and controls which of your cases is executed.
This is what I want to do. Run 1 case then, if my function changes the insync variable, go to a different case the next iteration of loop.
If on the other hand, you are hoping to execute more than one case per iteration of loop, you will need to do some redesign. Hard to tell with where you are so far.
I only want to run one case at a time, maintain or adjust the insync variable then, run the appropriate case on the next iteration of loop.
While you're removing brackets, please get rid of the pair on the return statement too - unnecessary (and a pet peeve).
This is my first function that I have written so, thank you for the advice. I will write some adjusted code and see what you guys think. I have another idea about using case that I did not use yet and it might fix my coding mistakes.
I am just starting to grasp functions, so I am still learning how to describe what I am trying to do.
Here is my latest code. I changed the variable inside of my function from "insync" to "r" to eliminate some confusion that I had and cause to others who might read this. Next, I got rid of the unnecessary ()'s.
If I understand correctly my function will = variable "r" when it has been ran. So to transfer it's value to "insync" my statement has to be: insync = iso_compare_string;
I have placed the above inside of the first case. I think that during the next iteration of the loop my insync variable will a 1 or 2. Then, the corresponding case 1 or 2 will be ran.
const int dataStreamSize=69;
unsigned char data[dataStreamSize];
int insync = 0;
/////////////////////////function in question//////////////////
int iso_compare_string ()
{
int r;
if (((data[0] == 0x80 && data[1] == 0xF1) )) {
/// WE HAVE A MATCH AND 52 BYTES TO READ
r = 1;
}
else{
//WE HAVE A PROBLEM NEED TO RESET
r = 2;
}
return r;
}
void setup() {
// initialize serial communication:
Serial.begin(9600);
}
void loop() {
// do something different depending on the
// communication value called insync:
switch (insync) {
case 0: ////// do serial communication to fill data array///////
//call iso_compare_string to check data
iso_compare_string();
Serial.println("checking data");
insync = iso_compare_string();
break;
case 1: // state when things are fine
Serial.println("normal communication");
//read 52 bytes and continue
break;
case 2: // state when a problem has a occurred
Serial.println("we have a problem");
// run reset function
break;
}
}
cyclegadget:
If I understand correctly my function will = variable "r" when it has been ran. So to transfer it's value to "insync" my statement has to be: insync = iso_compare_string;
I have placed the above inside of the first case. I think that during the next iteration of the loop my insync variable will a 1 or 2. Then, the corresponding case 1 or 2 will be ran.
No, that is assigning the address of the function iso_compare_string (which will never change) to insync.
You want:
insync = iso_compare_string ();
That calls the function. I haven't looked at the rest of it.
There is an example of filling a buffer, and testing its contents when it is "ready" and an example state machine. Your code looks a bit confusing for something that is basically reading a serial stream and doing something with it when it fills up.
Thanks Nick. I will have a look at those links. As you can see I have some things to learn about using functions and probably using case. I have read the replies over and over but, I still have work to do to figure this all out.
Before we leave the subject of brackets, take another look at this line:
if (((data[0] == 0x80 && data[1] == 0xF1) ))
Six of them. Only two are needed.
Some peculiarity here:
case 0: ////// do serial communication to fill data array///////
//call iso_compare_string to check data
iso_compare_string();
Serial.println("checking data");
insync = iso_compare_string();
break;
Thanks to the support of everyone in this thread! I struggled to get my questions worded in the proper manner but with help, I have switch case working the way I was hoping.
I have my array defined at the beginning to simulate a serial result to be tested in my function called iso_compare_string. The test returns int r which determines the next case that needs to be ran. For my program, if the serial data checks true, I will be toggling between case 0 and case 1. If the serial data checks false I will move to case 2.
I also did my best to follow wildbill's instructions on proper brackets.
Lastly, I posted the results from the serial monitor.
unsigned char data[] = {0x80, 0xF1};
int insync = 0;
/////////////////////////function in question//////////////////
int iso_compare_string ()
{
int r;
if (data[0] == 0x80 && data[1] == 0xF1) {
/// WE HAVE A MATCH AND 52 BYTES TO READ
r = 1;
}
else{
//WE HAVE A PROBLEM NEED TO RESET
r = 2;
}
return r;
}
void setup() {
// initialize serial communication:
Serial.begin(9600);
}
void loop() {
// do something different depending on the
// communication value called insync:
switch (insync) {
case 0: ////// do serial communication to fill data array///////
//call iso_compare_string to check data
Serial.println("checking data");
insync = iso_compare_string();
break;
case 1: // state when things are fine
Serial.println("normal communication");
//read 52 bytes and continue
insync = 0;
break;
case 2: // state when a problem has a occurred
Serial.println("we have a problem");
// run reset function
break;
}
}
Here is the results from the serial monitor with the code given.
checking data
normal communication
checking data
normal communication
checking data
normal communication
checking data
normal communication
