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 Previous  1, 2
 
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

PostPosted: Thu Mar 08, 2018 10:41 pm     Reply with quote

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

View user's profile Send private message

PostPosted: Fri Mar 09, 2018 3:12 am     Reply with quote

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

View user's profile Send private message

PostPosted: Fri Mar 09, 2018 11:19 pm     Reply with quote

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

View user's profile Send private message

PostPosted: Sat Mar 10, 2018 3:22 am     Reply with quote

First a scream.... Very Happy

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

View user's profile Send private message

PostPosted: Sat Mar 10, 2018 3:55 am     Reply with quote

sorry for that hold_regs next time i will remember that
i try that code it showing this

https://drive.google.com/file/d/1XZd3F51ESkn6GRT04BLMJkzGE3_hXU-9/view?usp=sharing
Ttelmah



Joined: 11 Mar 2010
Posts: 13054

View user's profile Send private message

PostPosted: Sat Mar 10, 2018 5:26 am     Reply with quote

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

View user's profile Send private message

PostPosted: Sat Mar 10, 2018 5:58 am     Reply with quote

thank for helping me friend
its fine to use 8 bit, data will display accurate Laughing
Ttelmah



Joined: 11 Mar 2010
Posts: 13054

View user's profile Send private message

PostPosted: Sat Mar 10, 2018 6:06 am     Reply with quote

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. Smile
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 Previous  1, 2
Page 2 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