 |
 |
| View previous topic :: View next topic |
| Author |
Message |
borseyogesh1436
Joined: 30 Nov 2017
Posts: 29
|
| [SOLVED] pic18f46k22 modbus master data error |
 Posted: Tue Mar 06, 2018 4:39 am |
 |
|
hi friends,
I am using pic18f46k22 as master and slave for modbus using CCS & PCH 16 bit v 5.015.
Slave working fine tested with modbus poll but the master showing two
value correct that is for FR & TOTAL and two value wrong that is FREQ and RANGE,
Range showing '0' and FREq showing '16' but
slave sending RANGE '500' and FREQ between '0 to 1000'.
slave send data like this
| Code: |
intval.whole=FR;//f_PICtoIEEE(FR);
hold_regs[0]=intval.words[0]; //LSW
hold_regs[1]=intval.words[1]; //MSW
intval.whole=TOTAL;//f_PICtoIEEE(TOTAL);
hold_regs[2]=intval.words[0]; //LSW
hold_regs[3]=intval.words[1]; //MSW
intval.whole=Erng;//f_PICtoIEEE(frequency);
hold_regs[4]=intval.words[0]; //LSW
hold_regs[5]=intval.words[1]; //MSW
intval.whole=frequency;//f_PICtoIEEE(Erng);
hold_regs[6]=intval.words[0]; //LSW
hold_regs[7]=intval.words[1]; //MSW
|
and master program
| Code: |
#include <main.h>
#define FAST_GLCD
#define MODBUS_TYPE MODBUS_TYPE_MASTER
#define MODBUS_SERIAL_INT_SOURCE MODBUS_INT_RDA
#define MODBUS_SERIAL_BAUD 9600
#define MODBUS_SERAIL_RX_BUFFER_SIZE 64
#define MODBUS_PARITY "EVEN"
#define MODBUS_SERIAL_RX_PIN PIN_c6
#define MODBUS_SERIAL_TX_PIN PIN_c7
#define MODBUS_SERIAL_ENABLE_PIN PIN_A3
#define MODBUS_SERIAL_RX_ENABLE PIN_A3
#include "modbus.c"
#include <h.c>
#include <g.c>
char yazi[15];
int16 hold_regs[];
void read_all_holding()
{
if(!(modbus_read_holding_registers(0x01,0x00,8))) // should I use
{
int i;
printf("Data: ");
for(i=0; i < 8;i++)
hold_regs[i]=modbus_rx.data[i];
}
else
{printf("<-**Exception %X**->\r\n\r\n", modbus_rx.error);}
delay_ms(1000);
}
char read[]="reading holding";
typedef union { unsigned int16 words[2]; unsigned int32 whole; } word16;
word16 intval;
float FR,TOTAL,RANGE,FREQ,ciz;
void main(void)
{
modbus_init();
glcd_init(on);
glcd_image(0);
glcd_update();
delay_ms(5000);
glcd_fillScreen(0);
while(1)
{
read_all_holding();
intval.words[0]=hold_regs[2]; //LSW
intval.words[1]=hold_regs[3]; //MSW
FR=intval.whole;
sprintf(yazi,"%1.0f ",FR);
glcd_text57(6,10,yazi,3,ON);
ciz=(float)(FR/RANGE)*127;
glcd_rect(0,36,127,45,no,on);
glcd_rect(2,38,ciz,43,yes,on); // flow bar
intval.words[0]=hold_regs[6]; //LSW
intval.words[1]=hold_regs[7]; //MSW
TOTAL=intval.whole;
sprintf(yazi,"%1.0f ",TOTAL);
glcd_text57(18,49,yazi,2,ON);
intval.words[0]=hold_regs[4]; //LSW
intval.words[1]=hold_regs[5]; //MSW
RANGE=intval.whole;
sprintf(yazi,"RNG-%1.0f ",RANGE);
glcd_text57(1,1,yazi,1,ON);
intval.words[0]=hold_regs[0]; //LSW
intval.words[1]=hold_regs[1]; //MSW
FREQ=intval.whole;
sprintf(yazi,"FRQ-%1.0f ",FREQ);
glcd_text57(50,1,yazi,1,ON);
glcd_text57(10,2,read,1,ON);
glcd_update();
glcd_fillScreen(0);
}
} |
Whats wrong with master code ? pls help me.
Last edited by borseyogesh1436 on Sat Mar 10, 2018 5:59 am; edited 1 time in total |
|
 |
temtronic
Joined: 01 Jul 2010
Posts: 9224
Location: Greensville,Ontario
|
|
| Posted: Tue Mar 06, 2018 5:56 am |
|
|
OK, I don't use modbus but...
you've got 4 variables, 2 are OK, 2 are not.
Providing the master code to receive, parse and display the data for all four is similar,then I'd 'hard code' data in the slave and try again
Hard code means to put known values for 'freq' and 'range', say 0x1234 and 0x5678. Something unique NOT 0x00 0x1111. This way you can see what may be going wrong.
I suspect it's the slave code not the master that has a problem.
jay |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19503
|
|
| Posted: Tue Mar 06, 2018 7:48 am |
|
|
Agreed wholeheartedly.
There are also worrying things about the register order. If you look the slave puts TOTAL into registers 2/3, but the value displayed as 'TOTAL' in the master is from registers 6/7. The same is true for each of the other values..... |
|
|
RF_Developer
Joined: 07 Feb 2011
Posts: 839
|
| Re: pic18f46k22 modbus master data error |
| Posted: Tue Mar 06, 2018 10:19 am |
|
|
Okay, you have problems at both slave and master. This is because you are trying to send float data using integer unions. This is not going to work.
Sending float data is always a problem, even today with IEEE754 as the overwhelmingly common standard. There's still no standard for sending floats over byte or other integer orientated comms links, including Modbus. Much better to send only integers.
So, what is the problem? Your union, intval of type word16, is, as the name suggests, set up for integers only. It allows conversion of 32 bit integers to pairs of 16 bit integers to suit Modbus, which is based on 16 bit data. It can't work with floats. When you assign a float you don't get what you are thinking it'll give: it messes up the value. At best you wont get any decimal places, at worst it may be total gibberish.
If you really must send floats, and not integers, then you need to change the union:
| Code: |
typedef union
{
unsigned int16 words[2];
unsigned int32 whole;
float float_value;
} word32;
word32 intval;
...
intval.float_value = f_PICtoIEEE(FR); // Best to send IEEE754 format.
hold_regs[0]=intval.words[0]; //LSW
hold_regs[1]=intval.words[1]; //MSW
intval.float_value= f_PICtoIEEE(TOTAL);
hold_regs[2]=intval.words[0]; //LSW
hold_regs[3]=intval.words[1]; //MSW
...
|
and master program
| Code: |
...
char yazi[15];
int16 hold_regs[];
void read_all_holding()
{
if(!(modbus_read_holding_registers(0x01,0x00,8))) // should I use
{
int i;
printf("Data: ");
for(i=0; i < 8;i++)
hold_regs[i]=modbus_rx.data[i]; // these will not be meaningful with float data.
}
else
{printf("<-**Exception %X**->\r\n\r\n", modbus_rx.error);}
delay_ms(1000);
}
char read[]="reading holding";
typedef union
{
unsigned int16 words[2];
unsigned int32 whole;
float float_value;
} word32;
word32 intval;
float FR,TOTAL,RANGE,FREQ,ciz;
void main(void)
{
modbus_init();
glcd_init(on);
glcd_image(0);
glcd_update();
delay_ms(5000);
glcd_fillScreen(0);
while(1)
{
read_all_holding();
intval.words[0]=hold_regs[2]; //LSW
intval.words[1]=hold_regs[3]; //MSW
FR=intval.float_value;
sprintf(yazi,"%1.0f ",FR);
glcd_text57(6,10,yazi,3,ON);
...
}
} |
|
|
|
borseyogesh1436
Joined: 30 Nov 2017
Posts: 29
|
|
| Posted: Wed Mar 07, 2018 12:23 am |
|
|
thanks for reply friends
i want to send and receive unsigned int32
i try both things as suggested but not working
float value send and receive showing '0' in all the values and i put known values for 'freq' and 'range' that will showing also 0 and 16
master send data
01 03 00 00 00 08 44 0C
slave send data
01 03 10 00 B2 00 00 00 0E 00 00 01 F4 00 00 00 16 00 00 77 03
FR=178
TOTAL =14
RANGE=500
FREQ=22
but master showing
FR=178
TOTAL =14
RANGE=0
FREQ=16
both master and slave sending accurate data to each other but master not displaying correct data which it will received. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19503
|
|
| Posted: Wed Mar 07, 2018 1:37 am |
|
|
Of course it won't....
If you are sending in IEEE, then you need to use the reciprocal function at the master end to turn it back into PIC format.
In fact RF_developer is wrong, since the IEEE conversion functions already change the values into int32, and back. However your original code did not show conversion to/from IEEE format (so what he then said was right...).
You are using 'half' what is required. You either need to use the IEEE conversion functions at each end, and send int32 values, or not use them and send float values as he tells you.
So sequence either is:
f_PICtoIEEE -> Gives INT32 that you then send using two int16's
int16's -> assemble to int32 then f_IEEEtoPIC
or
use the unions as he shows to just send the PIC float. |
|
|
borseyogesh1436
Joined: 30 Nov 2017
Posts: 29
|
|
| Posted: Thu Mar 08, 2018 12:34 am |
|
|
hi friends
https://drive.google.com/file/d/1s9T5cW9_u6CNKWCpbAzxDjpoRLzcwd_n/view?usp=sharing
master display data correctly when reading each 8bit
as
| Code: | sprintf(yazi,"%2X%2X",hold_regs[1],hold_regs[2]);
glcd_text57(1,0,yazi,2,ON);
sprintf(yazi,"%2X%2X",hold_regs[3],hold_regs[4]);
glcd_text57(1,16,yazi,2,ON);
sprintf(yazi,"%2X%2X",hold_regs[5],hold_regs[6]);
glcd_text57(1,31,yazi,2,ON);
sprintf(yazi,"%2X%2X",hold_regs[7],hold_regs[8]);
glcd_text57(1,46,yazi,2,ON);
sprintf(yazi,"%2X%2X",hold_regs[9],hold_regs[10]);
glcd_text57(50,0,yazi,2,ON);
sprintf(yazi,"%2X%2X",hold_regs[11],hold_regs[12]);
glcd_text57(50,16,yazi,2,ON);
sprintf(yazi,"%2X%2X",hold_regs[13],hold_regs[14]);
glcd_text57(50,31,yazi,2,ON);
sprintf(yazi,"%2X%2X",hold_regs[15],hold_regs[16]);
glcd_text57(50,46,yazi,2,ON); |
but using union when combine all 8 bit in to 32 it will show
00000032 actual value is 76543210
code as
| Code: |
int32 FR;
typedef union {int32 whole; int8 words[4];} word8;
word8 intval;
intval.words[0]=hold_regs[1];
intval.words[1]=hold_regs[2];
intval.words[2]=hold_regs[3];
intval.words[3]=hold_regs[4];
FR=intval.whole;
sprintf(yazi,"FR-%8X ",FR);
glcd_text57(1,0,yazi,1,ON); |
and also try with make32 it will also display 00000032 actual value is 76543210
| Code: | | FR=make32(hold_regs[4],hold_regs[3],hold_regs[2],hold_regs[1]); |
slave transmitting constant value like
4001=0x3210
4002=0x7654
4003=0xBA98
4004=0xFEDC
4005=0x0123
4006=0x4567
4007=0x89AB
4008=0xCDEF |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19503
|
|
| Posted: Thu Mar 08, 2018 3:25 am |
|
|
You really need to stop and start thinking.
The holding registers are 16bit values. You are writing them into 8bit variables.
Make32 takes either 4*8bit or 2*16bit variables. You are giving it 4*16 bit values, which it doesn't know how to cope with.
Then a 'word' implies a 16bit value. You are using it for 8bit values.
| Code: |
int32 FR;
typedef union {int32 whole; int16 words[2];} word16;
word16 intval;
intval.words[0]=hold_regs[1];
intval.words[1]=hold_regs[2]; //two 16bit values
FR=intval.whole;
sprintf(yazi,"FR-%8X ",FR);
glcd_text57(1,0,yazi,1,ON);
|
Now you also need to look at addresses. In your original post, you show the slave code writing to register 0, but are using register 1. Which is right?. |
|
|
borseyogesh1436
Joined: 30 Nov 2017
Posts: 29
|
|
| Posted: Thu Mar 08, 2018 4:29 am |
|
|
Thanks for reply Ttelmah sorry for my silly mistakes.
I know that the holding registers are 16bit values,
but this read_all_holding function gives me that 8 bit value.
When i use unsigned int16 hold_regs[] that time it will save only 8 bit
value and next holding resistor having half of first value. That's why i am
using unsigned int8 hold_regs[]. I know its wrong but its showing actual value that slave is sending to master.
Is there anything wrong with this function, void read_all_holding() ?
Its holding only 8 bit value, after changing unsigned int16 hold_regs[]
| Code: | unsigned int8 hold_regs[];
void read_all_holding()
{
if(!(modbus_read_holding_registers(0x01,0x0000,0x08))) // should I use
{
int i;
printf("Data: ");
for(i=1; i <(modbus_rx.len);i++)
{
hold_regs[i]=modbus_rx.data[i];
}
}
else
{printf("<-**Exception %X**->\r\n\r\n", modbus_rx.error);}
delay_ms(1800);
} |
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19503
|
|
| Posted: Thu Mar 08, 2018 8:26 am |
|
|
It is silly to call a variable 'holding_regs', which is 8bit, when these are 16bit values. You can cheat:
| Code: |
unsigned int16 hold_regs[8];
void read_all_holding()
{
if(!(modbus_read_holding_registers(0x01,0x0000,0x08))) // should I use
{
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);
}
}
|
This now writes the data into the 16bit holding registers directly.
You could even use a union at this point. |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Thu Mar 08, 2018 8:40 am |
|
|
Ttelmah, you fixed it, but you didn't tell him about a major flaw in
his program. This one:
That's how he wrote it in his earlier posts.
He never defined any storage space for the array. This will create an
uninitialized pointer, with no RAM allocated to it. So any attempt to write
to the array will likely trash other ram used by the compiler. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19503
|
|
| Posted: Thu Mar 08, 2018 9:28 am |
|
|
Yes. Not spotted.
Will cause complete garbage. Probably the other half of what is wrong....  |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9224
Location: Greensville,Ontario
|
|
| Posted: Thu Mar 08, 2018 9:31 am |
|
|
OK..what I don't understand, is that he tranfers 4 numbers and only 2 are 'bad'. Seems to me either all 4 should be good or all 4 should be bad, providing the code is the same for all 4.
Just curious...
Jay |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Thu Mar 08, 2018 9:46 am |
|
|
Temtronic,
If you put the diagnostic line shown below at the start of his routine, it will
print the value of the pointer.
| Code: |
void read_all_holding()
{
printf("hold_regs = %lx \r", hold_regs); |
I did this, then I ran his program in MPLAB vs. 8.92 simulator, and got:
In other words, it is not initialized. It doesn't point to any legitimate array.
Here is the first part of the .SYM file, which shows how the compiler
has allocated RAM:
| Code: |
000 @SCRATCH
001 @SCRATCH
001 _RETURN_
002 @SCRATCH
003 @SCRATCH
004 @INTERRUPT_AREA
005 @INTERRUPT_AREA
006 @INTERRUPT_AREA
007 @INTERRUPT_AREA
008 @INTERRUPT_AREA |
Since his pointer is pointing to 0000, he is over-writing an area of RAM
that is used by the compiler. This will cause erratic operation of his
program and incorrect data. |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9224
Location: Greensville,Ontario
|
|
| Posted: Thu Mar 08, 2018 10:45 am |
|
|
that I understand.. it was his line about...
the master showing two
value correct that is for FR & TOTAL and two value wrong that is FREQ and RANGE,
that has me confused...
then again I just found out it's Spring ahead, fall back time this weekend
oh well... |
|
|
|
|
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
