 |
 |
| View previous topic :: View next topic |
| Author |
Message |
borseyogesh1436
Joined: 30 Nov 2017
Posts: 29
|
|
 Posted: Thu Mar 08, 2018 10:41 pm |
 |
|
Thanks for reply friends.
Sorry for that i did not mention that FR & TOTAL value showing correct till 255 when the value is greater than that showing wrong values in both
forget that things
Now i am sending constant value like i mention in previous reply.
It will show all the values correct but in 8bit.
I think the problem is in reading holding function. It will store 8bit value
in hold_regs array.
I try suggested function by Ttelmah.
it will show error.
expecting lvalue such as a variable name or * expression
| Code: | unsigned int16 hold_regs[8];
void read_all_holding()
{
if(!(modbus_read_holding_registers(0x01,0x0000,0x08)))
{
int i;
for(i=1; i <(modbus_rx.len);i++)
{
(unsigned int8 *)(hold_regs)[i]=modbus_rx.data[i];
}
}
else
{
printf("<-**Exception %X**->\r\n\r\n", modbus_rx.error);}
delay_ms(1800);
}
}
|
In this function why use unsigned int8 ? i did not understand.
I change
| Code: | | (unsigned int16 *)hold_regs =modbus_rx.data[i]; |
It will not show error but its not showing values as given by slave. |
|
 |
Ttelmah
Joined: 11 Mar 2010
Posts: 19538
|
|
| Posted: Fri Mar 09, 2018 3:12 am |
|
|
Of course it won't. You are writing each byte into the same location.
The array form I showed should work (does on some compiler versions), but you can use this (which is universal):
| Code: |
*(((unsigned int8 *)(hold_regs))+i)=modbus_rx.data[i];
|
There is another form that avoids the complexities of pointers:
| Code: |
unsigned int16 hold_regs[8];
unsigned byte hold_bytes[16];
#byte hold_bytes=hold_regs
void read_all_holding()
{
if(!(modbus_read_holding_registers(0x01,0x0000,0x08)))
{
int i;
for(i=1; i <(modbus_rx.len);i++)
{
hold_bytes[i]=modbus_rx.data[i];
}
}
else
{
printf("<-**Exception %X**->\r\n\r\n", modbus_rx.error);}
delay_ms(1800);
}
}
|
This locates two variables into the same memory space. Just like a union, but without needing '.' notation.
So then the bytes are written into the 16 byte locations, but are also accessible as 8*16 bit values. |
|
|
borseyogesh1436
Joined: 30 Nov 2017
Posts: 29
|
|
| Posted: Fri Mar 09, 2018 11:19 pm |
|
|
Thanks for reply Ttelmah
Both things you suggested to me it will show wrong values. I think there
is msb and lsb are not stored in correct place.
I try with 8 bit and change the sequence of array and use with make32.
It will show all the value correct.
| Code: |
int32 FR,TOTAL,RANGE,FREQ;
char yazi[15];
unsigned int8 hold_regs[17];
if(!(modbus_read_holding_registers(0x01,0x0000,0x08)))
{
int i;
for(i=1; i <(modbus_rx.len);i++)
{
hold_regs[i]=modbus_rx.data[i];
}
}
else
{
hold_regs[3]=hold_regs[4]=hold_regs[1]=hold_regs[2]=0;
hold_regs[7]=hold_regs[8]=hold_regs[5]=hold_regs[6]=0;
hold_regs[16]=hold_regs[15]=hold_regs[14]=hold_regs[13]=0;
hold_regs[12]=hold_regs[11]=hold_regs[10]=hold_regs[9]=0;
}
FR = make32(hold_regs[3],hold_regs[4],hold_regs[1],hold_regs[2]);
sprintf(yazi,"%lu",FR);
glcd_text57(6,10,yazi,3,ON);
TOTAL = make32(hold_regs[7],hold_regs[8],hold_regs[5],hold_regs[6]);
sprintf(yazi,"%lu ",TOTAL);
glcd_text57(18,49,yazi,2,ON);
RANGE = make32(hold_regs[11],hold_regs[12],hold_regs[9],hold_regs[10]);
sprintf(yazi,"RNG-%lu ",RANGE);
glcd_text57(0,0,yazi,1,ON);
FREQ = make32(hold_regs[15],hold_regs[16],hold_regs[13],hold_regs[14]);
sprintf(yazi,"FRQ-%lu ",FREQ);
glcd_text57(63,0,yazi,1,ON); |
https://drive.google.com/file/d/1S3ejdivhLv--mXXps3-fviPg5tsyepq5/view?usp=sharing |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19538
|
|
| Posted: Sat Mar 10, 2018 3:22 am |
|
|
First a scream.... 
As shown, do not call your buffer 'hold_regs'. You are using a _byte_ buffer. As such _not_ the holding registers. 'Holding registers' has a specific meaning in Modbus, of the 16bit holding registers. Misusing a name like this is a sure way of driving yourself mad in the future, and also anyone else looking at the code.
This then would be the correct _holding register_ code to bring these in with the Modbus meaning:
| Code: |
unsigned int16 hold_regs[8];
unsigned byte hold_bytes[16];
#byte hold_bytes=hold_regs
void read_all_holding()
{
if(!(modbus_read_holding_registers(0x01,0x0000,0x08)))
{
int i;
for(i=1; i <(modbus_rx.len);i++)
{
hold_bytes[i^1]=modbus_rx.data[i]; //swap byte order into PIC
}
}
else
{
printf("<-**Exception %X**->\r\n\r\n", modbus_rx.error);}
delay_ms(1800);
}
}
|
|
|
|
borseyogesh1436
Joined: 30 Nov 2017
Posts: 29
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19538
|
|
| Posted: Sat Mar 10, 2018 5:26 am |
|
|
Understand, this is only creating the 16bit holding registers. If they are to be used for 323bit values you still have to re-assemble these.
All this does is reverse the order the bytes are assembled into the holding registers. If this is wrong then the original code would be right. |
|
|
borseyogesh1436
Joined: 30 Nov 2017
Posts: 29
|
|
| Posted: Sat Mar 10, 2018 5:58 am |
|
|
thank for helping me friend
its fine to use 8 bit, data will display accurate  |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19538
|
|
| Posted: Sat Mar 10, 2018 6:06 am |
|
|
Absolutely. For what you are doing now.
However if later you wanted to use the holding registers (or any other register set) in the format that Modbus uses them, it makes sense to be able to re-assemble them to the standard format.  |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
