I've spent some more time on classicladder modbus. I've tested against
an AVR running freemodbus which is the only device I have access to at
the moment. Communication is now very robust and fast at about 50
requests per second (and under 1% CPU utilization for the classicladder
process) with the following timing settings in the config:
MODBUS_MASTER_TIME_INTER_FRAME=0
MODBUS_MASTER_TIME_OUT_RECEIPT=20000
MODBUS_MASTER_TIME_AFTER_TRANSMIT=0
(in reality it works fine with much smaller TIME_OUT_RECEIPT, 50 or so
in the case of my device; modbus documentation indicates that 'the
duration of the WAIT and TREATMENT phases depends on the request
processing time needed for the slave application.' However, setting
this value fairly high doesn't have a negative impact on throughput
in a properly working system)
I also fixed a number of other problems I encountered with classicladder
modbus. Most notable are making --modmaster --nogui work, and fixing a
possible crash when only one byte is received from the slave.
Summary of my changes:
classicladder: modbus: Fix crash if LgtResponse==1
classicladder: modbus: fix response size calculation
classicladder: modbus: Use correct response timeout
classicladder: modbus: work with --nogui
classicladder: modbus: always transmit before trying to receive
classicladder: modbus: use non-blocking reads
classicladder: modbus: inter-byte delays are short
src/hal/classicladder/classicladder.c | 7 +++++
src/hal/classicladder/protocol_modbus_master.c | 32 ++++++++++++-----------
src/hal/classicladder/serial_common.h | 2 +-
src/hal/classicladder/serial_linux.c | 13 +++++----
src/hal/classicladder/socket_modbus_master.c | 2 +-
5 files changed, 33 insertions(+), 23 deletions(-)
Jeff
>From cf1cf370c92549419d105368bb06d39a865aa89d Mon Sep 17 00:00:00 2001
From: Jeff Epler <[email protected]>
Date: Sun, 20 Jun 2010 08:30:00 -0400
Subject: [PATCH 1/7] classicladder: modbus: Fix crash if LgtResponse==1
When LgtResponse was 1, CRC16 would be passed a length argument
of (unsigned short)-1, which would cause it to go past the storage
allocated for Response.
The modbus crc is designed so that after reading a packet and its
crc, the new crc is 0. Using this property, calculate the CRC on
LgtResponse bytes and check that result, instead of checking the
CRC on all-but-two bytes against the last two bytes. This eliminates
the need for a special case on a 1-byte response.
---
src/hal/classicladder/protocol_modbus_master.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/hal/classicladder/protocol_modbus_master.c
b/src/hal/classicladder/protocol_modbus_master.c
index b56d4af..e0e5f52 100755
--- a/src/hal/classicladder/protocol_modbus_master.c
+++ b/src/hal/classicladder/protocol_modbus_master.c
@@ -589,10 +589,10 @@ char TreatModbusMasterResponse( unsigned char * Response,
int LgtResponse )
// Modbus/RTU on serial used ?
if ( ModbusSerialPortNameUsed[ 0 ]!='\0' )
{
- unsigned short CalcCRC = CRC16( &Response[ 0 ],
LgtResponse-2 ) ;
+ unsigned short CalcCRC = CRC16( &Response[ 0 ],
LgtResponse ) ;
OffsetHeader = 1; // slave address
// verify CRC
- if( CalcCRC==( (Response[ LgtResponse-2
]<<8)|Response[ LgtResponse-1 ] ) )
+ if( CalcCRC==0 )
{
LgtResponse = LgtResponse-2;
// verify number of slave which has
responded
--
1.7.0.4
>From e197324d6eab45b99f82e2b04047e63f2bfdeb7d Mon Sep 17 00:00:00 2001
From: Jeff Epler <[email protected]>
Date: Sun, 20 Jun 2010 08:31:20 -0400
Subject: [PATCH 2/7] classicladder: modbus: fix response size calculation
the response length calculations were re-done based on online
modbus references.
---
src/hal/classicladder/protocol_modbus_master.c | 28 ++++++++++++-----------
1 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/src/hal/classicladder/protocol_modbus_master.c
b/src/hal/classicladder/protocol_modbus_master.c
index e0e5f52..5cea3d8 100755
--- a/src/hal/classicladder/protocol_modbus_master.c
+++ b/src/hal/classicladder/protocol_modbus_master.c
@@ -376,39 +376,41 @@ int TreatPureModbusResponse( unsigned char * RespFrame,
int SizeFrame )
/* Give number of bytes of the response we should receive for the current
request */
int GetModbusResponseLenghtToReceive( void )
{
- int LgtResp = 0, NbrRealBytes;
+ int LgtResp=0, NbrRealBytes;
if ( CurrentReq!=-1 )
{
int NbrEles = ModbusMasterReq[ CurrentReq ].NbrModbusElements;
- LgtResp++;//for function code
switch( CurrentFuncCode )
{
case MODBUS_FC_READ_INPUTS:
case MODBUS_FC_READ_COILS:
+ // 1 byte for function code
// 1 byte for count of data bytes returned
// 1 byte for 8 coils if number of coils is
not evenly divisable by 8 add another byte
NbrRealBytes = (NbrEles+7)/8;
- LgtResp++;
- LgtResp = LgtResp + NbrRealBytes;
+ LgtResp = 2+NbrRealBytes;
break;
case MODBUS_FC_READ_INPUT_REGS:
case MODBUS_FC_READ_HOLD_REGS:
+ // 1 byte for function code
+ // 1 byte for count of data bytes returned
// 2 bytes per register for data returned and
1 byte for number of registers (max 125)
- LgtResp = LgtResp + (NbrEles*2)+1;
+ LgtResp = 2+2*NbrEles;
break;
- case MODBUS_FC_FORCE_COIL:
+ case MODBUS_FC_FORCE_COIL:
case MODBUS_FC_FORCE_COILS:
case MODBUS_FC_WRITE_REG:
- // 2 bytes for address 2 for data
- LgtResp = LgtResp + LgtResp +4;
- break;
case MODBUS_FC_WRITE_REGS:
- // 2 bytes for address start and 2 bytes for
number of registers
- LgtResp = LgtResp + LgtResp +4;
+ // 1 byte for function code
+ // 2 bytes for starting address
+ // 2 bytes for number of addresses
+ LgtResp = 5;
break;
- case MODBUS_FC_DIAGNOSTICS://modbus function 8
+ case MODBUS_FC_DIAGNOSTICS:
+ // assume hardcoded sub code: echo (0x0)
+ // 2 byte for function code
// 2 bytes for sub code 2 for data
- LgtResp = LgtResp + LgtResp +4;
+ LgtResp = 5;
break;
default:
printf("INFO CLASSICLADDER- MODBUS
function code not reconized");
--
1.7.0.4
>From e650514217bf3d874c7618e5f2fa60b0546cc3be Mon Sep 17 00:00:00 2001
From: Jeff Epler <[email protected]>
Date: Sun, 20 Jun 2010 10:39:05 -0400
Subject: [PATCH 3/7] classicladder: modbus: Use correct response timeout
Because VTIME is coarse, make the timeout in milliseconds
available in SerialReceive so it can be used directly.
tv_usec values above 999999 are not permitted; set tv_sec to a
positive value when appropriate
---
src/hal/classicladder/serial_common.h | 2 +-
src/hal/classicladder/serial_linux.c | 6 +++---
src/hal/classicladder/socket_modbus_master.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/hal/classicladder/serial_common.h
b/src/hal/classicladder/serial_common.h
index 56c7ac8..535a7ac 100755
--- a/src/hal/classicladder/serial_common.h
+++ b/src/hal/classicladder/serial_common.h
@@ -3,6 +3,6 @@ char SerialOpen( char * SerialPortName, int Speed );
void SerialClose( );
void SerialSend( char *Buff, int BuffLength );
void SerialSetResponseSize( int Size, int TimeOutResp );
-int SerialReceive( char * Buff, int MaxBuffLength );
+int SerialReceive( char * Buff, int MaxBuffLength, int TimeOutResp );
void SerialFlush( void );
diff --git a/src/hal/classicladder/serial_linux.c
b/src/hal/classicladder/serial_linux.c
index 62a036f..65bca3f 100755
--- a/src/hal/classicladder/serial_linux.c
+++ b/src/hal/classicladder/serial_linux.c
@@ -203,7 +203,7 @@ void SerialSetResponseSize( int Size, int TimeOutResp )
}
}
-int SerialReceive( char * Buff, int MaxBuffLength )//, int TimeOutResp )
+int SerialReceive( char * Buff, int MaxBuffLength, int TimeOutResp )
{
int NbrCarsReceived = 0;
if ( PortIsOpened )
@@ -216,8 +216,8 @@ struct timeval tv;
FD_ZERO( &myset);
// add descrip to survey and set time-out wanted !
FD_SET( fd, &myset );
-tv.tv_sec = 0; //seconds
-tv.tv_usec = newtio.c_cc[VTIME]*100 *1000; //micro-seconds
+tv.tv_sec = TimeOutResp / 1000; //seconds
+tv.tv_usec = (TimeOutResp % 1000) * 1000; //micro-seconds
if ( ModbusDebugLevel>=3 )
printf("select() for serial reading...\n");
recep_descrip = select( 16, &myset, NULL, NULL, &tv );
diff --git a/src/hal/classicladder/socket_modbus_master.c
b/src/hal/classicladder/socket_modbus_master.c
index 44bed8a..3ee49f0 100755
--- a/src/hal/classicladder/socket_modbus_master.c
+++ b/src/hal/classicladder/socket_modbus_master.c
@@ -356,7 +356,7 @@ void SocketModbusMasterLoop( void )
if ( ModbusSerialPortNameUsed[ 0 ]=='\0' )
ResponseSize =
WaitRespSocketModbusMaster( ResponseFrame, 800, ModbusTimeOutReceipt );
else
- ResponseSize = SerialReceive(
ResponseFrame, 800 );
+ ResponseSize = SerialReceive(
ResponseFrame, 800, ModbusTimeOutReceipt );
NbrFrames++;
if ( ResponseSize==0 )
printf("ERROR CLASSICLADDER- MODBUS NO
RESPONSE (Errs=%d/%d) !?\n", ++CptErrors, NbrFrames);
--
1.7.0.4
>From 9327177554a96b574b5750bcc7b940eaf79198fc Mon Sep 17 00:00:00 2001
From: Jeff Epler <[email protected]>
Date: Sun, 20 Jun 2010 10:40:19 -0400
Subject: [PATCH 4/7] classicladder: modbus: work with --nogui
Without this, classicladder --nogui --modmaster would exit
immediately and not do any modbus communication.
There is a remaining problem that when using --nogui --modmaster the gui
can't be started later (because a component with the classicladder name
already exists) but this is still preferable to the old status quo.
---
src/hal/classicladder/classicladder.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/hal/classicladder/classicladder.c
b/src/hal/classicladder/classicladder.c
index de0f13d..6a54a88 100644
--- a/src/hal/classicladder/classicladder.c
+++ b/src/hal/classicladder/classicladder.c
@@ -248,6 +248,13 @@ int main( int argc, char *argv[] )
ProjectLoadedOk = LoadProjectFiles(
InfosGene->CurrentProjectFileName );
if (pathswitch){ strcpy(
InfosGene->CurrentProjectFileName, NewPath ); }
InfosGene->LadderState = STATE_RUN;
+ if (modslave || modmaster) {
+ rtapi_print("INFO CLASSICLADDER- No ladder
GUI requested-Modbus runs till HAL closes.\n");
+ hal_ready(compId);
+ if(modslave) InitSocketServer( 0/*UseUdpMode*/,
ModbusServerPort/*PortNbr*/);
+ if (modmaster) PrepareModbusMaster( );
+ while(1) sleep(1);
+ }
ClassicLadder_FreeAll(TRUE);
hal_ready(compId);
hal_exit(compId);
--
1.7.0.4
>From eef9d285b2e926ac6e7f7c40fc914de438492bfd Mon Sep 17 00:00:00 2001
From: Jeff Epler <[email protected]>
Date: Sun, 20 Jun 2010 10:41:48 -0400
Subject: [PATCH 5/7] classicladder: modbus: always transmit before trying to
receive
the receive timeout should always start after the request
has been entirely transmitted
---
src/hal/classicladder/serial_linux.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/hal/classicladder/serial_linux.c
b/src/hal/classicladder/serial_linux.c
index 65bca3f..6fc4bdc 100755
--- a/src/hal/classicladder/serial_linux.c
+++ b/src/hal/classicladder/serial_linux.c
@@ -181,10 +181,10 @@ void SerialSend( char *Buff, int BuffLength )
write(fd,Buff,BuffLength);
if ( ModbusDebugLevel>=2 )
printf("Writing done!\n");
+ // wait until everything has been transmitted
+ tcdrain( fd );
if ( ModbusSerialUseRtsToSend )
{
- // wait until everything has been transmitted
- tcdrain( fd );
SerialSetRTS( 0 );
}
}
--
1.7.0.4
>From 960121e2a8fbcff995f1660c196bce7c9b8b8303 Mon Sep 17 00:00:00 2001
From: Jeff Epler <[email protected]>
Date: Sun, 20 Jun 2010 10:42:53 -0400
Subject: [PATCH 6/7] classicladder: modbus: use non-blocking reads
termios VMIN/VTIME don't work properly when O_NONBLOCK
is enabled. Setting O_NDELAY in open is required to
avoid blocking at open() when DTR is not asserted, but
it must be cleared or read() behaves as for O_NONBLOCK.
---
src/hal/classicladder/serial_linux.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/hal/classicladder/serial_linux.c
b/src/hal/classicladder/serial_linux.c
index 6fc4bdc..848b216 100755
--- a/src/hal/classicladder/serial_linux.c
+++ b/src/hal/classicladder/serial_linux.c
@@ -63,6 +63,7 @@ char SerialOpen( char * SerialPortName, int Speed )
{
int BaudRate = -1;
int ScanBaudRate = 0;
+ fcntl(fd, F_SETFL, O_RDWR | O_NOCTTY ); /* perform blocking
reads */
while( SerialSpeed[ ScanBaudRate ]!=Speed && SerialSpeed[
ScanBaudRate ]>=0 )
{
ScanBaudRate++;
--
1.7.0.4
>From d768a13452050c5073bb966f4323a91d99d3b496 Mon Sep 17 00:00:00 2001
From: Jeff Epler <[email protected]>
Date: Sun, 20 Jun 2010 10:45:18 -0400
Subject: [PATCH 7/7] classicladder: modbus: inter-byte delays are short
the modbus standards indicate that the time between two bytes may be no
more than 1.5 byte times (put another way, the time from one byte to
the next may be no more than 2.5 byte times).
The smallest VTIME is .1 second, and at the slowest supported rate
(300 baud), .1 seconds is still greater than 2.5 byte times. Thus,
the hardcoded value VTIME=1 is the best choice.
---
src/hal/classicladder/serial_linux.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/hal/classicladder/serial_linux.c
b/src/hal/classicladder/serial_linux.c
index 848b216..86f4435 100755
--- a/src/hal/classicladder/serial_linux.c
+++ b/src/hal/classicladder/serial_linux.c
@@ -196,7 +196,7 @@ void SerialSetResponseSize( int Size, int TimeOutResp )
if ( PortIsOpened )
{
newtio.c_cc[VMIN] = Size; //Nbr chars we should receive;
- newtio.c_cc[VTIME] = TimeOutResp/100; // TimeOut in 0.1s
+ newtio.c_cc[VTIME] = 1;
// tcflush(fd, TCIFLUSH);
if ( ModbusDebugLevel>=2 )
printf("Serial config...\n");
--
1.7.0.4
------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
lucky parental unit. See the prize list and enter to win:
http://p.sf.net/sfu/thinkgeek-promo
_______________________________________________
Emc-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/emc-developers
