stephane / libmodbus

A Modbus library for Linux, Mac OS, FreeBSD and Windows
http://libmodbus.org
GNU Lesser General Public License v2.1
3.45k stars 1.75k forks source link

Win32 problems #187

Closed vgrin closed 10 years ago

vgrin commented 10 years ago

Hi, I have found some problem in Win32 implementation:

  1. The function

int modbus_tcp_pi_listen(modbus_t *ctx, int nb_connection)

requires the same _modbus_tcp_init_win32 call as modbus_tcp_listen, _modbus_tcp_pi_connect and _modbus_tcp_connect functions

modbus-tcp.c line 521

int modbus_tcp_pi_listen(modbus_t ctx, int nb_connection) { int rc; struct addrinfo ai_list; struct addrinfo ai_ptr; struct addrinfo ai_hints; const char node; const char service; int new_s; modbus_tcp_pi_t ctx_tcp_pi;

if (ctx == NULL) {
    errno = EINVAL;
    return -1;
}

ctx_tcp_pi = ctx->backend_data;

// insert this

ifdef OS_WIN32

if (_modbus_tcp_init_win32() == -1) {
    return -1;
}

endif

// insert this

…. }

  1. The code in the test unit-test-client.c line 258

rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS, 0, tab_rp_registers); printf("3/5 modbus_read_registers (0): "); if (rc != 0) { printf("FAILED (nb points %d)\n", rc); goto close; } printf("OK\n"); looks like incorrect because after the call wit nb=0 the rc is -1 as result of an error

if (nb < 1 || MODBUS_MAX_READ_REGISTERS < nb) in modbus.c line 770 and an exception MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE from the server. It has to be changed to:

if (rc != -1) { printf("FAILED (nb points %d)\n", rc); goto close; } printf("OK\n"); Probably it is not Win32 only.

  1. The Win32 implementation of modbus is a DLL. This means that the test program or any other programs have separate errno from the DLL and communication about errors datails between EXE and DLL is not possible: EXE reads error value from anther slot than DLL uses for writing. Only in case of using the modbus as a static LIB the test passes. Any idea how to use modbus as DLL and accessible errno?

Best regards, vgrin!

vgrin commented 10 years ago

The same problem with nb=0 takes place in random-test-client.c

ERROR Illegal data value ERROR modbus_read_bits Address = 99, nb = 0 ... ERROR Illegal data value ERROR modbus_read_registers (-1) Address = 99, nb = 0 ... ERROR Illegal data value ERROR modbus_read_and_write_registers (-1) Address = 99, nb = 0 Test: 3 FAILS

Either for (addr = ADDRESS_START; addr <= ADDRESS_END; addr++)

should be changed to

for (addr = ADDRESS_START; addr < ADDRESS_END; addr++)

or any read operations should be skipped for nb == 0? The write operations with zero size are allowed, except _FC_WRITE_AND_READ_REGISTERS. Is it by design?

vgrin commented 10 years ago

A small fix of the bandwidth-client.c for Win32:

/ ... /

include

include

include

include

include

if !defined(_MSC_VER)

include <sys/time.h>

endif

include

include

define G_MSEC_PER_SEC 1000

uint32_t gettime_ms(void) {

if defined(_MSC_VER)

return GetTickCount();

else

struct timeval tv;
gettimeofday (&tv, NULL);
return (uint32_t) tv.tv_sec * 1000 + tv.tv_usec / 1000;

endif

}

....

vgrin commented 10 years ago

A typing mistake in the bandwidth-client.c line 180:

nb_points = MODBUS_MAX_RW_WRITE_REGISTERS;

to

nb_points = MODBUS_MAX_WR_WRITE_REGISTERS;

stephane commented 10 years ago

@vgrin did look to other github issues about Win32 before posting? Could you create a separate issue for each point (starting from now so don't report again in seperate issues the current comments)?

stephane commented 10 years ago

I very difficult to answer clearly to your comments (one issue must be related to only one subject).

1 - I've added the change in modbus_tcp_pi_listen in a commit in my PC but before I push it, could you confirm you've tested it? 2 - the tests have been rewritten a month ago so could you report again against the current master branch of libmodbus? 3 - In "small fix of the bandwidth-client.c for Win32", the code is not properly formatted for github, please use github flavored markdown documentation for your next postings, it seems to be an improvement not a fix, right? 4 - MODBUS_MAX_RW_WRITE_REGISTERS not in master anymore

Thank you for your reports.

vgrin commented 10 years ago

Hello Stephane, I did look, but I have not found these issues. Sorry, if I have duplicated something. I am new in github. Thank you for understanding.

  1. About modbus_tcp_pi_listen – of course I have tested and it works. BTWL any other reported issues have been fixed in my PC and they are working as well.
  2. Sorry, I didn’t know. Where should I look for current master branch ?
  3. It is not an improvement – there no neither <sys/time.h> nor gettimeofday under Win32
stephane commented 10 years ago
  1. OK pushed
  2. git clone https://github.com/stephane/libmodbus.git or download zip of master
  3. the current master code don't use gettimeofday for win32 but
SYSTEMTIME st;
GetLocalTime(&st);
tv.tv_sec = st.wSecond;
tv.tv_usec = st.wMilliseconds * 1000;
```, so I still think it's an improvement not a fix. Code pushed too, could you test benchmark client of master branch?
vgrin commented 10 years ago

I have sent you e-mail with changes. Have a nice weekend.

vgrin commented 10 years ago

About ever passed “Timeout of 3ms between bytes“ test – please change 500 microseconds to 5 milliseconds File “tests\unit-test-server.c” line 204 from usleep(500); to usleep(5000);

PointOnePA commented 10 years ago

Sorry, I'm new to GIT Hub and using it for the first time, so I don't know it if is okay to post a comment on a closed issue.

I'm playing with libmodbus for Win32 (Visual Studio 2010) and I'll help on the unit tests once I get it running, but I thought I would mention a TYPO in modbus-tcp.c. line 557

line 557 "if (modbustcp_init_win32() == -1) {" but line 67 defines static in t _modbus_tcp_init_win32(void)

Note the two missing underscores indicated with asterisks: _modbus_tcp_init_win32

Once I added the two underscores on line 557: if (_modbus_tcp_init_win32() == -1) { the library compiles successfully in Visual Studio 2010, creating modbus.dll

FYI

vgrin commented 10 years ago

@PointOnePA yes, I have sent this fix to Stephane as well With all my changes all test are running.

conect commented 9 years ago

I'm using linux and just cloned the current master but neverthelesse I get: ERROR Illegal data value ERROR modbus_read_and_write_registers (-1) Address = 99, nb = 0 Test: 3 FAILS

on executing ./random-test-client