|
|
View previous topic :: View next topic |
Author |
Message |
lsteele
Joined: 02 Jan 2007 Posts: 18
|
Modbus crc errors... |
Posted: Thu Jan 29, 2009 5:42 pm |
|
|
Hello,
I'm trying to create a simple modbus device based on ex_modbus_slave.c, but I'm running into some problems. I'm hoping that someone more expert than myself will recognise the symptoms and have some ideas.
My 'device' is a pic 18f4550 running a slightly modified version of the ex_modbus_slave.c example code. It's connected to my pc via a serial connection (using the pic's hardware uart) at 57600 baud. Meanwhile I have a usb serial link using the usb_cdc.h library spitting data to hyperterminal so I can see everything's working, also at 57600 baud. On the PC I have running mach3, a servo/stepper control program for cnc machines.
At the moment I have a script running within the mach3 software that attempts to set 6 holding register values on my pic around 20 times a second. Initially all seems to be well - as the values in the mach3 software change I can see the values change on the Pic in hyperterminal. After some minutes (it's not consistent) I'll notice the rate of change of the values (i.e. the rate at which the holding registers are being updated) in the hyperterminal window slow from a fairly rapid rate to around 2hz. At the same time mach3 reports a crc error. Soon after this mach3 crashes. If when I notice the crc error I close the serial connection in mach3 then reopen it, I can see the rate at which the values are being updated in the hyperterminal window speed up again, and the crc error clears.
Now, if the problem is with mach3 I guess I'm on my own, but can anyone see something that could be creating this problem on the pic? I was wondering if printf'ing data to hyperterminal could be cause a delay which might cause problems?
I've included my code below. The only significant change (I think) is where I printf the values (soon after the beginning of main() ).
Hopefully this is ringing bells for someone. Any help is gratefully received.
Luke
Excerpt from the code containing my modifications:
Code: |
#include "18f4550.h"
#fuses NOPBADEN, NOPROTECT, NOBROWNOUT, BORV20, NOWDT, NOLVP, VREGEN, NOPUT, XTPLL, PLL1, USBDIV, CPUDIV1
#use delay(clock=48000000)
#define MODBUS_TYPE MODBUS_TYPE_SLAVE
#define MODBUS_SERIAL_RX_BUFFER_SIZE 128
#define MODBUS_SERIAL_BAUD 57600 //9600
#ifndef USE_WITH_PC
#define MODBUS_SERIAL_INT_SOURCE MODBUS_INT_EXT
#define MODBUS_SERIAL_TX_PIN PIN_B1 // Data transmit pin
#define MODBUS_SERIAL_RX_PIN PIN_B0 // Data receive pin
//The following should be defined for RS485 communication
//#define MODBUS_SERIAL_ENABLE_PIN 0 // Controls DE pin for RS485
//#define MODBUS_SERIAL_RX_ENABLE 0 // Controls RE pin for RS485
#else
#define MODBUS_SERIAL_INT_SOURCE MODBUS_INT_RDA
#endif
#include "usb_cdc.h"
#include "modbus.c"
#define MODBUS_ADDRESS 0x01
/*This function may come in handy for you since MODBUS uses MSB first.*/
int8 swap_bits(int8 c)
{
return ((c&1)?128:0)|((c&2)?64:0)|((c&4)?32:0)|((c&8)?16:0)|((c&16)?8:0)
|((c&32)?4:0)|((c&64)?2:0)|((c&128)?1:0);
}
unsigned int16 swap_endian(unsigned long toswap) {
unsigned int16 swapped;
*((char *)&swapped+1)=*((char *)&toswap);
*((char *)&swapped)=*((char *)&toswap+1);
return swapped;
}
#define BEAT1PIN pin_a3
void main()
{
int8 coils = 0b00000101;
int8 inputs = 0b00001001;
int16 hold_regs[] = {0x8800,0x7700,0x6600,0x5500,0x4400,0x3300,0x2200,0x1100};
int16 input_regs[] = {0x1100,0x2200,0x3300,0x4400,0x5500,0x6600,0x7700,0x8800};
int16 event_count = 0;
modbus_init();
usb_init();
while(TRUE)
{
while(!modbus_kbhit()) {
usb_task();
if(usb_enumerated()) {
printf(usb_cdc_putc, "\n\r%ld.%ld, %ld.%ld, %ld.%ld", swap_endian(hold_regs[0]), swap_endian(hold_regs[1]), swap_endian(hold_regs[2]), swap_endian(hold_regs[3]), swap_endian(hold_regs[4]), swap_endian(hold_regs[5]));
}
//check address against our address, 0 is broadcast
if((modbus_rx.address == MODBUS_ADDRESS) || modbus_rx.address == 0)
{
switch(modbus_rx.func)
{
|
|
|
|
Ttelmah Guest
|
|
Posted: Fri Jan 30, 2009 3:28 am |
|
|
I'd suggest you start by 'proving' where the problem lies.
Something like a VBS program, rather than Mach, that just sends dummy values at the expected rate. If this shows the same problem, you have ruled out Mach, and can concentrate on your code.
As a comment though, your 'swap_endian' function, is very heavy on machine time, and with the number of times it is used, is going to represent a lot of computer time. Pointers are quite an inefficient way of doing this sort of thing in the PIC (have to load the table address registers, then do a byte access through the indirect access). Much more efficient to use either a union, or the CCS make8/make16 functions. So something like:
Code: |
unsigned int16 swap_endian(unsigned long toswap) {
unsigned int16 swapped;
swapped=make16(make8(toswap,0),make8(toswap,1));
return swapped;
}
|
This takes just 8 machine cycles, versus 50 for the original code...
It might make enough difference to change things.
Best Wishes |
|
|
lsteele
Joined: 02 Jan 2007 Posts: 18
|
|
Posted: Sat Jan 31, 2009 7:52 am |
|
|
Hi Ttelmah,
Thanks for the ideas. I'm going to do as you suggest and see if I can replicate the fault with a simple host running on the computer in place of mach3. By VBS did you mean visual basic? - In fact I've noticed that Modbus Poll seems to have some kind of Visual Basic library, so I'm going to see if I can use that.
Incidentally is it plausible that a long delay where I've got my printf could cause the sorts of problems I'm seeing?
Luke |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Sat Jan 31, 2009 10:16 am |
|
|
Quote: | As a comment though, your 'swap_endian' function, is very heavy on machine time. |
The problem with this code is, that CCS C actually uses pointers cause it's apparently unable to understand this kind of type casting, that's a ususal means in C programming to my opinion. Obviously, one shouldn't make any assumptions on the cleverless of a compiler but check the produced code.
Your code could be further reduced by omitting the auxilary variable
Code: | return make16(make8(toswap,0),make8(toswap,1)); |
Also in this point, CCS C is operating line by line without any optimization. An inline function or C macro is appropriate to remove also the function call overhead.
Regarding delay effects in general, cause MODBUS RTU has the requirement of continuous transmission of packets, it may be affected by delays in program execution. |
|
|
|
|
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
|