| View previous topic :: View next topic |
| Author |
Message |
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
 Posted: Mon May 25, 2020 2:15 pm |
 |
|
| OK. I will look at your code. It may take some time. |
|
 |
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Mon May 25, 2020 8:10 pm |
|
|
I'm trying to understand your program.
Can you tell me what is the purpose of your program ? Explain the inputs to
the PIC, and explain how the PIC is supposed to respond to these inputs. |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Tue May 26, 2020 12:09 am |
|
|
In your text arrays, I notice you never write a final 0x00
to the last byte of the array to terminate the string.
This is required. Examples:
| Quote: | char mystring[6];
digito = 0;
mystring [0] = ' ';
mystring [1] = ' ';
mystring [2] = ' ';
mystring [3] = ' ';
mystring [4] = ' ';
mystring [5] = ' ';
ID_mystring[0] = ' ';
ID_mystring[1] = ' ';
ID_mystring[2] = ' ';
ID_mystring[3] = ' ';
ID_mystring[4] = ' ';
ID_mystring[5] = ' ';
ID_mystring[6] = ' ';
ID_mystring[7] = ' ';
counter = 0;
|
You make this mistake with every buffer you have. More examples:
| Quote: |
buffer[0] = ' ';
buffer[1] = ' ';
buffer[2] = ' ';
buffer[3] = ' ';
buffer[4] = ' ';
pedido[0] = idd[0];
pedido[1] = idd[1];
pedido[2] = 'A';
pedido[3] = 'B';
pedido[4] = 'R';
pedido[5] = 'E';
printf(pedido);
sensor[0] = idd[0];
sensor[1] = idd[1];
sensor[2] = 'O';
sensor[3] = 'P';
sensor[4] = 'E';
sensor[5] = 'N';
printf(sensor);
|
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19260
|
|
| Posted: Tue May 26, 2020 2:38 am |
|
|
There is also a huge problem with the sizes. For example:
| Code: |
if(ID == 255)
{
ID = 0;
}
sprintf(idd,"%u",ID);
|
Id is allowed to be up to 255.
Printed in decimal, this needs three characters _plus another for the
terminator_. Your idd string is only 2 characters long. Result the last
couple of characters will destroy the start of 'sensor'.....
Given the need for null terminators, all the strings need to expand.
 |
|
|
rodrigocirilo
Joined: 14 Sep 2016
Posts: 20
|
|
| Posted: Tue May 26, 2020 6:16 am |
|
|
| PCM programmer wrote: | I'm trying to understand your program.
Can you tell me what is the purpose of your program ? Explain the inputs to
the PIC, and explain how the PIC is supposed to respond to these inputs. |
Well, let's go.
The program is a keyboard where the user will enter passwords and will be sent to a C # server.
At the end of the code there is an If that checks whether the 4 or 6 digits have been entered and as soon as the envia() function is used to send the password to the server, as I will have more than one keyboard connected by RS485 on this server , you need to identify it using a 2-digit ID only.
the reset_cpu () that I use is because in the set_ID() function I choose whether the password will be 4 or 6 digits, in addition to choosing the ID that will be used, so when this function is called it will reset the ID (the ID's always will have a value above 10) so if I type 0 on the keyboard I am setting for 4 or 6 digit passwords, if I type the first digit a number 1 or 2 to create an ID of 11 or 24 value for example, zero no more will have the function of selecting the number of digits of the password.
see that from line 320 to 336 is the part that I select flag_digito4ou6 as 0 or 1 and saved in eeprom, so I reset_cpu so that at the beginning of the program it gets the value set in this variable.
In this flag_digito4ou6 I had the same problem as flag_emergencia, it changed value when called the envia() function, see that at the end of the envia() function I read the eeprom at address 2 to get the real value of flag_digito4ou6 because after running the function envia() it also changes its value. the bug of flag_digito4ou6 I solved it like this. It may not be the best way, but I don't know what's in the envia() function that it changes the value of the variables.
in the reception part, the pic reads a signal that will arrive every 20 seconds saying that the C # application is working, another signal that calls a pin of the pic to be connected, and another signal telling the pic that the password consulted in the database is wrong . |
|
|
rodrigocirilo
Joined: 14 Sep 2016
Posts: 20
|
|
| Posted: Tue May 26, 2020 6:18 am |
|
|
| PCM programmer wrote: | In your text arrays, I notice you never write a final 0x00
to the last byte of the array to terminate the string.
This is required. Examples:
| Quote: | char mystring[6];
digito = 0;
mystring [0] = ' ';
mystring [1] = ' ';
mystring [2] = ' ';
mystring [3] = ' ';
mystring [4] = ' ';
mystring [5] = ' ';
ID_mystring[0] = ' ';
ID_mystring[1] = ' ';
ID_mystring[2] = ' ';
ID_mystring[3] = ' ';
ID_mystring[4] = ' ';
ID_mystring[5] = ' ';
ID_mystring[6] = ' ';
ID_mystring[7] = ' ';
counter = 0;
|
You make this mistake with every buffer you have. More examples:
| Quote: |
buffer[0] = ' ';
buffer[1] = ' ';
buffer[2] = ' ';
buffer[3] = ' ';
buffer[4] = ' ';
pedido[0] = idd[0];
pedido[1] = idd[1];
pedido[2] = 'A';
pedido[3] = 'B';
pedido[4] = 'R';
pedido[5] = 'E';
printf(pedido);
sensor[0] = idd[0];
sensor[1] = idd[1];
sensor[2] = 'O';
sensor[3] = 'P';
sensor[4] = 'E';
sensor[5] = 'N';
printf(sensor);
|
|
Well, even though I didn't write the 0x00 at the end of the matrix, it worked when I tested the send.
Can you show me what that would be like.
In the first example that you set I do that to reset / clean the matrix so that there is no dirt on it for the next shipment, I don't know if this cleaning is necessary. |
|
|
rodrigocirilo
Joined: 14 Sep 2016
Posts: 20
|
|
| Posted: Tue May 26, 2020 6:33 am |
|
|
| Ttelmah wrote: | There is also a huge problem with the sizes. For example:
| Code: |
if(ID == 255)
{
ID = 0;
}
sprintf(idd,"%u",ID);
|
Id is allowed to be up to 255.
Printed in decimal, this needs three characters _plus another for the
terminator_. Your idd string is only 2 characters long. Result the last
couple of characters will destroy the start of 'sensor'.....
Given the need for null terminators, all the strings need to expand.
|
ID will always have a value between 10 and 30 at most, at the beginning of the program I do this:
| Code: | ID = read_eeprom(0);
if(ID == 255)
{
ID = 0;
} |
because I know that the pic antrs eeprom of any recording has its value in 255 decimal, so the first time the pis is start, it will read the eeprom with value 255, and at this moment it enters the if that makes it value 0, I knowing this I will go to the seta_ID function where I will create a new ID and save it at address 0, when there is a shutdown, and start again, address 0 will no longer have the value 255, so this if will no longer be necessary because the value will always be different from 255.
the ID value will never exceed 2 digits. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19260
|
|
| Posted: Tue May 26, 2020 6:45 am |
|
|
Problem is you use sprintf to put the number into the array. A two digit
number needs three characters when stored as a string. Result in fact is that
the first character of sensor will be destroyed (variables are normally
stored sequentially as declared).... |
|
|
rodrigocirilo
Joined: 14 Sep 2016
Posts: 20
|
|
| Posted: Tue May 26, 2020 6:51 am |
|
|
| Ttelmah wrote: | Problem is you use sprintf to put the number into the array. A two digit
number needs three characters when stored as a string. Result in fact is that
the first character of sensor will be destroyed (variables are normally
stored sequentially as declared).... |
Yes, but when I test the sending of OPEN or ABRE for example the complete string, example 15OPEN or 12ABRE, arrives on the other side of the serial, it doesn't fail, I don't know how else it could get in the way. You say that idd should be idd [3] instead of idd [2], I can do that too without problems. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19260
|
|
| Posted: Tue May 26, 2020 7:49 am |
|
|
The reason this works is you re-load this every time before sending,
so the overflow' doesn't cause disaster.
There are lots of similar issues with 'lack of control' of what the data
is doing.
For instance:
| Code: |
int i;
int counter;
counter++;
buffer[i++]=getc(); // aqui somente recebe o que vem do pc e monta a string
|
What is 'i' going to start as?. Undefined. It could result in values
written into memory anywhere.....
Similarly 'counter' is indeterminate.
These need to be declared at the start of the main routine (not in
the function block), and initialised counter=0 etc..
To use printf, the strings all need to be null terminated, otherwise
garbage could be printed. One character more for each string, and load the
last character with 0 as PCM_Programmer has pointed out.
You can initialise a string with:
strcpy(buffer, " "); //so long as buffer is 6 characters long. |
|
|
rodrigocirilo
Joined: 14 Sep 2016
Posts: 20
|
|
| Posted: Tue May 26, 2020 7:56 am |
|
|
| Ttelmah wrote: | The reason this works is you re-load this every time before sending,
so the overflow' doesn't cause disaster.
There are lots of similar issues with 'lack of control' of what the data
is doing.
For instance:
| Code: |
int i;
int counter;
counter++;
buffer[i++]=getc(); // aqui somente recebe o que vem do pc e monta a string
|
What is 'i' going to start as?. Undefined. It could result in values
written into memory anywhere.....
Similarly 'counter' is indeterminate.
These need to be declared at the start of the main routine (not in
the function block), and initialised counter=0 etc..
To use printf, the strings all need to be null terminated, otherwise
garbage could be printed. One character more for each string, and load the
last character with 0 as PCM_Programmer has pointed out.
You can initialise a string with:
strcpy(buffer, " "); //so long as buffer is 6 characters long. |
So I will declare i and counter at the beginning with a value of 0 and see what improves.
Instead of using printf I would ask to use puts ()? |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Tue May 26, 2020 8:40 am |
|
|
| rodrigocirilo wrote: |
Well, even though I didn't write the 0x00 at the end of the matrix,
it worked when I tested the send.
Can you show me what that would be like.
|
See the insertion of 0x00 below.
| Code: | mystring [0] = ' ';
mystring [1] = ' ';
mystring [2] = ' ';
mystring [3] = ' ';
mystring [4] = ' ';
mystring [5] = 0x00;
|
printf, and puts, and sprintf ALL look for the 0x00 to know when
to stop reading the array.
Your attitude of "it worked anyway" is wrong. This attitude leads to
bad coding and programs failing randomly.
Google this and read some websites about strings:
| Quote: | | strings in C tutorial |
|
|
|
rodrigocirilo
Joined: 14 Sep 2016
Posts: 20
|
|
| Posted: Tue May 26, 2020 8:45 am |
|
|
| PCM programmer wrote: | | rodrigocirilo wrote: |
Well, even though I didn't write the 0x00 at the end of the matrix,
it worked when I tested the send.
Can you show me what that would be like.
|
See the insertion of 0x00 below.
| Code: | mystring [0] = ' ';
mystring [1] = ' ';
mystring [2] = ' ';
mystring [3] = ' ';
mystring [4] = ' ';
mystring [5] = 0x00;
|
printf, and puts, and sprintf ALL look for the 0x00 to know when
to stop reading the array.
Your attitude of "it worked anyway" is wrong. This attitude leads to
bad coding and programs failing randomly.
Google this and read some websites about strings:
| Quote: | | strings in C tutorial |
|
I'm going to modify what you said, I'm sorry I didn't know that information. I will add 0x00 to all strings.
You managed to analyze why the variable changed its value without any command from me. |
|
|
rodrigocirilo
Joined: 14 Sep 2016
Posts: 20
|
|
| Posted: Tue May 26, 2020 10:47 am |
|
|
| I added 0x00 at the end of all strings and increased their sizes by 1 character, it worked, but the variable bug still persists. |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Tue May 26, 2020 11:54 am |
|
|
Do some experiments. Get creative. Try moving flag_emergencia
to the end of your variable list as shown below. See if it causes any
change in symptoms.
| Quote: |
int1 flag_sensor = false;
//int flag_emergencia = 0; // comment it out here
int dummy1; // Add a place holder for it
unsigned int flag_digito4ou6 = 0;
unsigned int contador=0;
unsigned int reset_contador = 0;
unsigned int ID = 0;
unsigned int a=0;
unsigned int x=0 ;
int dummy2; // Add a Spacer for more safety
int flag_emergencia = 0; // move the flag here
void bipa();
void envia();
void bipa_limpa(void);
void bip_ok(void);
void bip_erro(void);
void seta_ID(void); |
|
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
