CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com

[SOLVED] pic18f46k22 modbus master data error
Goto page 1, 2  Next
 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
borseyogesh1436



Joined: 30 Nov 2017
Posts: 29

View user's profile Send private message

[SOLVED] pic18f46k22 modbus master data error
PostPosted: Tue Mar 06, 2018 4:39 am     Reply with quote

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: 9081
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Tue Mar 06, 2018 5:56 am     Reply with quote

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: 19195

View user's profile Send private message

PostPosted: Tue Mar 06, 2018 7:48 am     Reply with quote

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

View user's profile Send private message

Re: pic18f46k22 modbus master data error
PostPosted: Tue Mar 06, 2018 10:19 am     Reply with quote

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

View user's profile Send private message

PostPosted: Wed Mar 07, 2018 12:23 am     Reply with quote

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: 19195

View user's profile Send private message

PostPosted: Wed Mar 07, 2018 1:37 am     Reply with quote

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

View user's profile Send private message

PostPosted: Thu Mar 08, 2018 12:34 am     Reply with quote

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: 19195

View user's profile Send private message

PostPosted: Thu Mar 08, 2018 3:25 am     Reply with quote

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

View user's profile Send private message

PostPosted: Thu Mar 08, 2018 4:29 am     Reply with quote

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: 19195

View user's profile Send private message

PostPosted: Thu Mar 08, 2018 8:26 am     Reply with quote

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

View user's profile Send private message

PostPosted: Thu Mar 08, 2018 8:40 am     Reply with quote

Ttelmah, you fixed it, but you didn't tell him about a major flaw in
his program. This one:
Code:
int16 hold_regs[];

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: 19195

View user's profile Send private message

PostPosted: Thu Mar 08, 2018 9:28 am     Reply with quote

Yes. Not spotted.
Will cause complete garbage. Probably the other half of what is wrong.... Sad
temtronic



Joined: 01 Jul 2010
Posts: 9081
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Thu Mar 08, 2018 9:31 am     Reply with quote

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

View user's profile Send private message

PostPosted: Thu Mar 08, 2018 9:46 am     Reply with quote

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:
Quote:
hold_regs = 0000

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: 9081
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Thu Mar 08, 2018 10:45 am     Reply with quote

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...
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Goto page 1, 2  Next
Page 1 of 2

 
Jump to:  
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