help simplifying function

[darnell]

ok  my program dont wont to end , after the comport is not open, it continues and keep running the for completely

 the program suppose to shut down after the first error

can some one assist me?

[RobertoBozzolo]

dummy_decoy has answered you in the other thread on why your program does not ends after an error. You should treasure his suggestion about stepping through the code...

If you want to avoid using goto statement you could organize the code this way:

int main()
{
 int error = 0;

 texStart("STTO");

 error = OpenComConfig (physical_port, "", baudrate,             
                     parity, databits, stopbits, inputq, outputq);

 if (!error) error = SetComTime (physical_port, 0);

 if (!error) error = ComWrt(physical_port, set_point_query, sp_command_size);  

 Delay (0.05);     // Adjust time as needed

 if (!error) error = ComRd (physical_port, read_data, sp_command_size);

 // Here only if all is successful: test return string

 texFinish (NORMAL_FINISH);            

  
Error:

  if error < 0) DisplayRS232Error (error);

  return 0;
  
}

But I really don't see anything wrong in using the goto statement, provided it is not misused.

I am a bit disappointed seeing that you don't mind forcing yourself to understand our suggestions and proceed in your way as if nothing has happened!

I wont say anything more about "com10" device name, but you could at least correct this:

switch(status_check) {
     case -3:
          if  ((status_check! = sp_command_size) || (Length != physical_port))  {
            DisplayRS232Error (status_check);
            return 0;
         }
         break;

Can you explain us why you are testing status_check value to be different from sp_command_size (which is assigned value of 4 in your include file) within case -3 section? Obviously it will always be!

With regard to that lenght != physical_port: what if you will be using port 3 in some future installation of your software? Will your device respond with a different number of characters?

Last but not least: please do not open several threads on the same exact subject!

[darnell]

sorry about that roberto, in the real code i did just what you guys said before, this is a test dummy code, in the real code in the open comport, i have "", im more concerned on my error handling right now.

i dont have "COM10" in the real code, im just testing something out, so im not really paying that no attn. at the moment , because im doing it you guys way in the real code,

right now my  concern is breaking everything down to our company standards. we are really picky.

also can you explain

"With regard to that lenght != physical_port: what if you will be using port 3 in some future installation of your software? Will your device respond with a different number of characters?"

actually the user is setting the port from the ini config file in the will code, so he's able to change the port number at anytime . so i have wrote a code to parse the port number from the ini config file.

can you explain this function "if (!error) error = SetComTime (physical_port, 0);"    

and also what is the SetComTime function is doing?

[darnell]

last question so its no point of  doing this when i have several comparison in the code like below  even though i have the

DisplayRS232Error(status_check) 

*****************************************************************

                                                                      
       switch(status_check)                                              
       {                                                                 
                                                                         
           case -3:                                                      
                                                                         
                   if ((status_check!=sp_command_size)     ||            
                   (Length!=physical_port)              ||            
                   (status_check!=process_temp_size)    ||            
                   (status_check!=create_set_point_size)||            
                   (status_check!=turn_system_on_size))               
                {                                                     
                      DisplayRS232Error (status_check);                  
                      break;                                             
                }                                                     
                                                                         
      

[RobertoBozzolo]

---

darnell wrote:

last question so its no point of  doing this when i have several comparison in the code like below  even though i have the

DisplayRS232Error(status_check) 

*****************************************************************

                                                                      
       switch(status_check)                                              
       {                                                                 
                                                                         
           case -3:                                                      
                                                                         
                   if ((status_check!=sp_command_size)     ||            
                   (Length!=physical_port)              ||            
                   (status_check!=process_temp_size)    ||            
                   (status_check!=create_set_point_size)||            
                   (status_check!=turn_system_on_size))               
                {                                                     
                      DisplayRS232Error (status_check);                  
                      break;                                             
                }                                                     
                                                                         
      

---

darnell, do yu know what the switch statement stands for? As you should know, it may be translated in a series of if clauses, so your code looks like this one:

  if (status_check == -3) {      // Rember: if you enter this block, status_check value is -3
      if ((status_check! = sp_command_size) ||    // Of course it will be: you defined sp_command_size as 4
      (Length != physical_port)  ||
      (status_check != process_temp_size) ||  // The same: you defined process_temp_size as 4
      (status_check != create_set_point_size) ||  // The same: you defined create_set_point_size as 6
      (status_check != turn_system_on_size))   // The same: you defined turn_system_on_size as 6
      {                                                     
        DisplayRS232Error (status_check);                  
        break;                                             
      }

  }

[RobertoBozzolo]

---

darnell wrote:

also can you explain

"With regard to that lenght != physical_port: what if you will be using port 3 in some future installation of your software? Will your device respond with a different number of characters?"

actually the user is setting the port from the ini config file in the will code, so he's able to change the port number at anytime . so i have wrote a code to parse the port number from the ini config file.

---

That's exactly what I mean: the com port number is parametric, on one system you are using port 1, while on another you may be using port2 or port 3. Lenght is the result of GetInQLen, that is is the number of bytes ready to be read from the port, i.e. it is the response of the attached device to a query: it does not depends on the port used, I supposed (the device does not knows anything about the computer, you know!!!!)

So what is the meaning of comparing Lenght with com port number??!?!?!

---

darnell wrote:

can you explain this function "if (!error) error = SetComTime (physical_port, 0);"    

and also what is the SetComTime function is doing?

---

You don't want to use goto's? Ok, so the first function sets an error code and every further function must be executed only if this errr code is zero (apart setting the error code itself, of course!).

I will let you the task of reading the online help for SetComTime function

[Al_S]

Darnell:

In the switch statement you posted in message 34, in case -3, you compare status_check to the size of four different things: sp_command_size, process_temp_size, create_set_point_size, turn_system_on_size.  Since you based your switch statement on status_check, in case -3, status_check is -3.  Does it make any sense that the size of anything is -3?  Why compare the Length of anything to the physical_port?

Are those variable names meaningful (really a size or Length)?  If they are, what sense does your comparison make?

Also in your switch case -3, you have the break statement in the if statement.  So if your test condition is not true, you will not reach the break statement, and the switch statement will continue with any other comparisons till the end of your switch statement.  Unless that's intentional, it's best to have a break statement that will be executed at the end of each case, regardless of the results of any comparisons for an if statement.

I don't think that this error handler is a good application for a switch statement.  CVI has the  GetRS232ErrorString() function.  You haven't shown any reason in your code to add anything to the error message it returns.  I like Roberto's if structure.

Here are a few comments on your other  posts I haven't had the time or energy to keep up with.

Please don't continue to post you cvibuild directory, .cdb files and .exe files.  Roberto and I have asked you multiple times to stop this, yet you continue.  We don't need any of that to look at your problem.  It just clogs up NI's servers and our hard drives if we download it.

Multiple times you have posted code with open ended questions like "tell me what you think", or "any suggestions?", but when we offer comments, you back off and say "that's not the real code" or "I was working that code three days ago" or "I'm doing it right in the real code".  Don't post old code or dummy code.  Don't continue to re-post code before you make an effort to correct the problems we have pointed out.

You are keeping multiple experienced people on this forum busy with your puzzling posts.  Puzzling not because of any technical difficulty in your questions, but because of the difficulty trying to figure out what in the world you are trying to do and why.  Puzzling because we rarely see you acting on or learning anything from the good advice given.

Please consider taking an introductory programming class.  It seems like you are not only struggling with C syntax, but also with basic logic which would apply to any programming language.  Did you take a look at the community college info I posted here: http://forums.ni.com/ni/board/message?board.id=180&message.id=43184&query.id=471715#M43184

I posted that particular community college just as an example of what is available somewhere in the U.S.  You can find a college close to you.  It will benefit you, your employer (Keith?), those of us on this forum that are continuing to try to answer your posts, and the others on the forum who would benefit from the greatly increased bandwidth if you stop this serial posting.  

Have you noticed how your posts go on and on, but other people's posts ask a specific question, get a detailed answer, get a thank you, and they're done?  Very few posts by other people have the number of messages that yours do.  You are personally using a lot of the bandwidth on this forum. 

Please take a class or two.  It will benefit all of us. 

[darnell]

so basically you saying that this something i should not be doing below,im kind of confuse

if know what my switch statement is saying. can you be more specific, how is my switch statement suppose to look since im taking this route

  if (status_check == -3) {      // Rember: if you enter this block, status_check value is -3
      if ((status_check! = sp_command_size) ||    // Of course it will be: you defined sp_command_size as 4
      (Length != physical_port)  ||
      (status_check != process_temp_size) ||  // The same: you defined process_temp_size as 4
      (status_check != create_set_point_size) ||  // The same: you defined create_set_point_size as 6
      (status_check != turn_system_on_size))   // The same: you defined turn_system_on_size as 6
      {                                                     
        DisplayRS232Error (status_check);                  
        break;                                             
      }

  }

 

[RobertoBozzolo]

darnell, in this switch case you have 4 of 5 conditions which are always true by definition; on the 5th one see my comment and Al's one.

In my opinion you must put away all of them and leave only DisplayRS232Error.

[darnell]

  it does make sense to  leave it this way. the way i had it, seem like i was double checking for some reason which is not necessary.

 thanks.

    switch(status_check)                                              
       {                                                                 
                                                                         
           case -3:                                                      
                                                                             
                                                                     
                      DisplayRS232Error (status_check);                  
                      break;                                             
                }         

and to end my program i would just put exit(0); in the DisplayRS232Error (status_check); ? correct?

that way my program would end after a error.