From 746d3b40d49be76058cee3d531cd60e9eb80ab5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frode=20Tenneb=C3=B8?= Date: Thu, 31 Mar 2016 16:32:23 +0200 Subject: [PATCH 01/41] New function modbus_reply_callback() The modbus_reply_callback() function is modelled iclosely on modbus_reply(), but should be used when more complex actions are required for handling Modbus functions that what is possible with the current mapping scheme. --- doc/Makefile.am | 1 + doc/libmodbus.txt | 1 + doc/modbus_reply_callback.txt | 105 ++++++++++++ src/modbus.c | 296 ++++++++++++++++++++++++++++++++++ src/modbus.h | 29 +++- 5 files changed, 431 insertions(+), 1 deletion(-) create mode 100644 doc/modbus_reply_callback.txt diff --git a/doc/Makefile.am b/doc/Makefile.am index 61ab5d5fb..3e28b2f73 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -26,6 +26,7 @@ TXT3 = \ modbus_read_registers.txt \ modbus_receive_confirmation.txt \ modbus_receive.txt \ + modbus_reply_callback.txt \ modbus_reply_exception.txt \ modbus_reply.txt \ modbus_report_slave_id.txt \ diff --git a/doc/libmodbus.txt b/doc/libmodbus.txt index 500b70738..9035d351a 100644 --- a/doc/libmodbus.txt +++ b/doc/libmodbus.txt @@ -228,6 +228,7 @@ Receive:: Reply:: linkmb:modbus_reply[3] + linkmb:modbus_reply_callback[3] linkmb:modbus_reply_exception[3] diff --git a/doc/modbus_reply_callback.txt b/doc/modbus_reply_callback.txt new file mode 100644 index 000000000..547354302 --- /dev/null +++ b/doc/modbus_reply_callback.txt @@ -0,0 +1,105 @@ +modbus_reply_callback(3) +======================== + +NAME +---- +modbus_reply_callback - invoke a callback and send a reponse to the received request + + +SYNOPSIS +-------- +*int modbus_reply_callback(modbus_t *'ctx', const uint8_t *'req', int 'req_length', modbus_callbacks_t *'mb_callbacks', void *'user_data'); + + +DESCRIPTION +----------- +The *modbus_reply_callback()* function is modelled on and takes much the same +parameters as *modbus_reply()*. It is designed for more complex situations +where actions are needed to be performed instead of reading or writing to +a Modbus mapping. + +Based on the request *modbus_reply_callback()* shall invoke a callback and +send a response to the received request. The request _req_ given +as argument is analyzed, a callback is invoked, and a response is then built +and sent by using the information of the modbus context _ctx_. + +For read functions, the callback will need to supply the response, returning +the length of that response. For pure write functions (i.e. not +_FC_WRITE_AND_READ_REGISTERS), the response is automatically constructed. + +The callbacks are supplied in the callback mapping _mb_callbacks_ according +to the type of the request. If a callback is not provided for a given +function, *modbus_reply_callback()* constructs the exception +MODBUS_EXCEPTION_ILLEGAL_FUNCTION accordingly. + +It is the callback's responsibility to return the Modbus error code +*negated* (except for the case where the operation is _FC_WRITE_SINGLE_COIL +and the value is neither OFF nor ON) in case of errors. In this case an +exception response will be sent. + +Note: If the return value is => 0 it is assumed that the call was successful +both for read and write callbacks. For read callbacks the value is also used +as the response length supplied by the callback function. + +An optional parameter _user_data_ can be suppied which will be passed to +the callback function, typically for context purpose. + +The read and write callbacks have the following prototypes: + +int (*modbus_read_callback)(uint16_t addr, uint16_t nb, uint8_t *rsp, void *user_data); + +Each read callback function takes the address (_addr_), number of registers +(_nb_), address to response message (_rsp_)(, and pointer to optional user data +(_user_data_). The response is pre-filled with the MBAP header AND FCode +(function code), 8 bytes in total. The callback is responsible for filling +in the rest of the response PDU according to Modbus Application Protocol' +V1.1b3, returning the size of the PDU. + +int (*modbus_write_callback)(uint16_t addr, uint16_t nb, const uint8_t *req, void *user_data); + +Each write callback function takes the address (_addr_), number of registers +(_nb_), address to the request to be written (_req_), and pointer to optional +user data (_user_data_). + +This function is designed for Modbus server. + + +RETURN VALUE +------------ +The function shall return the length of the response sent if +successful. Otherwise it shall return -1 and set errno. + + +ERRORS +------ +*EMBMDATA*:: +Sending has failed + +See also the errors returned by the syscall used to send the response (eg. send +or write). + +EXAMPLE +------- +[source,c] +------------------- +static struct modbus_callbacks_t callbacks; +int my_read_coil_implementation(uint16_t addr, uint16_t nb, uint8_t *rsp, void *user_data); +... + +callbacks.read_coils = &my_read_coil_implementation; +... + +modbus_reply_callback(context, &request, request_lenght, &callbacks, NULL); + + +SEE ALSO +-------- +linkmb:modbus_reply_exception[3] +linkmb:modbus_reply[3] +linkmb:libmodbus[7] + + +AUTHORS +------- +The libmodbus documentation was written by Stéphane Raimbault + diff --git a/src/modbus.c b/src/modbus.c index 1a5a778b6..37de05c27 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1085,6 +1085,302 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, return (slave == MODBUS_BROADCAST_ADDRESS) ? 0 : send_msg(ctx, rsp, rsp_length); } +/* This function is modelled on and takes the same parameters as modbus_reply() + above. Based on the request, modbus_reply_callback() will invoke either a + read or a write callback function. In addition, this function takes a + pointer to optional user data which is passed along to the callback function. + + For read functions, the callback will need to supply the response, returning + the length of that response. For pure write (i.e. not + _FC_WRITE_AND_READ_REGISTERS) functions, the response is automatically + constructed. + + If a callback is not provided for a given function, modbus_reply_callback() + constructs the exception MODBUS_EXCEPTION_ILLEGAL_FUNCTION accordingly. + Except for _FC_WRITE_SINGLE_COIL where (value != 0xFF00 || value != 0x0) + it is the callback's responsibility to return the MODBUS error code NEGATED! + NB: If the return value is >= 0 it is assumed that the call was successful, + and the value is used as the response length for read callbacks. + + Usage: + + static struct modbus_callbacks_t callbacks; + callbacks.read_coils = &my_read_coil_implementation; + modbus_reply_callback(context, &request, request_lenght, &callbacks, NULL); +*/ +int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, + int req_length, modbus_callbacks_t *mb_callbacks, + void *user_data) +{ + int offset = ctx->backend->header_length; + int slave = req[offset - 1]; + int function = req[offset]; + uint16_t address = (req[offset + 1] << 8) + req[offset + 2]; + uint8_t rsp[MAX_MESSAGE_LENGTH]; + int rsp_length = 0; + sft_t sft; + int rv; + + sft.slave = slave; + sft.function = function; + sft.t_id = ctx->backend->prepare_response_tid(req, &req_length); + + switch (function) { + case MODBUS_FC_READ_COILS: + if (mb_callbacks->read_coils == NULL) { + if (ctx->debug) { + fprintf(stderr, "Callback not defined for _FC_READ_COILS.\n"); + } + rsp_length = response_exception( + ctx, &sft, + MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + } else { + uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; + + rsp_length = ctx->backend->build_response_basis(&sft, rsp); + rv = mb_callbacks->read_coils(address, nb, &rsp[rsp_length], user_data); + + if (rv < 0) { + rsp_length = response_exception( + ctx, &sft, + -rv, rsp); + } else { + rsp_length += rv; + } + } + break; + case MODBUS_FC_READ_DISCRETE_INPUTS: + if (mb_callbacks->read_discrete_inputs == NULL) { + if (ctx->debug) { + fprintf(stderr, "Callback not defined for _FC_READ_DISCRETE_INPUTS.\n"); + } + rsp_length = response_exception( + ctx, &sft, + MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + } else { + uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; + + rsp_length = ctx->backend->build_response_basis(&sft, rsp); + rv = mb_callbacks->read_discrete_inputs(address, nb, &rsp[rsp_length], user_data); + if (rv < 0) { + rsp_length = response_exception( + ctx, &sft, + -rv, rsp); + } else { + rsp_length += rv; + } + } + break; + case MODBUS_FC_READ_HOLDING_REGISTERS: + if (mb_callbacks->read_holding_registers == NULL) { + if (ctx->debug) { + fprintf(stderr, "Callback not defined for _FC_READ_HOLDING_REGISTERS.\n"); + } + rsp_length = response_exception( + ctx, &sft, + MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + } else { + uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; + + rsp_length = ctx->backend->build_response_basis(&sft, rsp); + rv = mb_callbacks->read_holding_registers(address, nb, &rsp[rsp_length], user_data); + + if (rv < 0) { + rsp_length = response_exception( + ctx, &sft, + -rv, rsp); + } else { + rsp_length += rv; + } + } + break; + case MODBUS_FC_READ_INPUT_REGISTERS: + if (mb_callbacks->read_input_registers == NULL) { + if (ctx->debug) { + fprintf(stderr, "Callback not defined for _FC_READ_INPUT_REGISTERS.\n"); + } + rsp_length = response_exception( + ctx, &sft, + MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + } else { + uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; + + rsp_length = ctx->backend->build_response_basis(&sft, rsp); + rv = mb_callbacks->read_input_registers(address, nb, &rsp[rsp_length], user_data); + + if (rv < 0) { + rsp_length = response_exception( + ctx, &sft, + -rv, rsp); + } else { + rsp_length += rv; + } + } + break; + case MODBUS_FC_WRITE_SINGLE_COIL: + if (mb_callbacks->write_single_coil == NULL) { + if (ctx->debug) { + fprintf(stderr, "Callback not defined for _FC_WRITE_SINGLE_COIL.\n"); + } + rsp_length = response_exception( + ctx, &sft, + MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + } else { + uint16_t value = (req[offset + 3] << 8) + req[offset + 4]; + + if (value == 0xFF00 || value == 0x0) { + rv = mb_callbacks->write_single_coil(address, 1, &req[offset + 3], user_data); + if (rv < 0) { + rsp_length = response_exception( + ctx, &sft, + -rv, rsp); + } else { + memcpy(rsp, req, req_length); + rsp_length = req_length; + } + } else { + if (ctx->debug) { + fprintf(stderr, + "Illegal data value %0X in _FC_WRITE_SINGLE_COIL request at address %0X.\n", + value, address); + } + rsp_length = response_exception( + ctx, &sft, + MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); + } + } + break; + case MODBUS_FC_WRITE_SINGLE_REGISTER: + if (mb_callbacks->write_single_register == NULL) { + if (ctx->debug) { + fprintf(stderr, "Callback not defined for _FC_WRITE_SINGLE_REGISTER.\n"); + } + rsp_length = response_exception( + ctx, &sft, + MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + } else { + rv = mb_callbacks->write_single_register(address, 1, &req[offset + 3], user_data); + + if (rv < 0) { + rsp_length = response_exception( + ctx, &sft, + -rv, rsp); + } else { + memcpy(rsp, req, req_length); + rsp_length = req_length; + } + } + break; + case MODBUS_FC_WRITE_MULTIPLE_COILS: + if (mb_callbacks->write_multiple_coils == NULL) { + if (ctx->debug) { + fprintf(stderr, "Callback not defined for _FC_WRITE_MULTIPLE_COILS.\n"); + } + rsp_length = response_exception( + ctx, &sft, + MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + } else { + uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; + rv = mb_callbacks->write_multiple_coils(address, nb, &req[offset+6], user_data); + + if (rv < 0) { + rsp_length = response_exception( + ctx, &sft, + -rv, rsp); + } else { + rsp_length = ctx->backend->build_response_basis(&sft, rsp); + /* 4 to copy the bit address (2) and the quantity of bits */ + memcpy(rsp + rsp_length, req + rsp_length, 4); + rsp_length += 4; + } + } + break; + case MODBUS_FC_WRITE_MULTIPLE_REGISTERS: + if (mb_callbacks->write_multiple_registers == NULL) { + if (ctx->debug) { + fprintf(stderr, "Callback not defined for _FC_WRITE_MULTIPLE_REGISTERS.\n"); + } + rsp_length = response_exception( + ctx, &sft, + MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + } else { + uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; + rv = mb_callbacks->write_multiple_registers(address, nb, &req[offset+6], user_data); + + if (rv < 0) { + rsp_length = response_exception( + ctx, &sft, + -rv, rsp); + } else { + rsp_length = ctx->backend->build_response_basis(&sft, rsp); + /* 4 to copy the bit address (2) and the quantity of bits */ + memcpy(rsp + rsp_length, req + rsp_length, 4); + rsp_length += 4; + } + } + break; + case MODBUS_FC_REPORT_SLAVE_ID: { + int str_len; + int byte_count_pos; + + rsp_length = ctx->backend->build_response_basis(&sft, rsp); + /* Skip byte count for now */ + byte_count_pos = rsp_length++; + rsp[rsp_length++] = _REPORT_SLAVE_ID; + /* Run indicator status to ON */ + rsp[rsp_length++] = 0xFF; + /* LMB + length of LIBMODBUS_VERSION_STRING */ + str_len = 3 + strlen(LIBMODBUS_VERSION_STRING); + memcpy(rsp + rsp_length, "LMB" LIBMODBUS_VERSION_STRING, str_len); + rsp_length += str_len; + rsp[byte_count_pos] = rsp_length - byte_count_pos - 1; + break; + } + case MODBUS_FC_WRITE_AND_READ_REGISTERS: + if (mb_callbacks->read_holding_registers == NULL || + mb_callbacks->write_multiple_registers == NULL) { + if (ctx->debug) { + fprintf(stderr, "Callbacks not defined for _FC_WRITE_AND_READ_REGISTERS.\n"); + } + rsp_length = response_exception( + ctx, &sft, + MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + } else { + uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; + uint16_t address_write = (req[offset + 5] << 8) + req[offset + 6]; + uint16_t nb_write = (req[offset + 7] << 8) + req[offset + 8]; + + /* Write first... + (10 is the offset to the start of the values to write) */ + rv = mb_callbacks->write_multiple_registers(address_write, nb_write, &req[offset+10], user_data); + if (rv < 0) { + rsp_length = response_exception( + ctx, &sft, + -rv, rsp); + } else { + /* ...then read the response. */ + rsp_length = ctx->backend->build_response_basis(&sft, rsp); + rv = mb_callbacks->read_holding_registers(address, nb, &rsp[rsp_length], user_data); + if (rv < 0) { + rsp_length = response_exception( + ctx, &sft, + -rv, rsp); + } else { + rsp_length += rv; + } + } + } + break; + default: + rsp_length = response_exception(ctx, &sft, + MODBUS_EXCEPTION_ILLEGAL_FUNCTION, + rsp); + break; + } + + return send_msg(ctx, rsp, rsp_length); +} + int modbus_reply_exception(modbus_t *ctx, const uint8_t *req, unsigned int exception_code) { diff --git a/src/modbus.h b/src/modbus.h index 47a69d172..13be38f61 100644 --- a/src/modbus.h +++ b/src/modbus.h @@ -123,7 +123,7 @@ enum { MODBUS_EXCEPTION_SLAVE_OR_SERVER_BUSY, MODBUS_EXCEPTION_NEGATIVE_ACKNOWLEDGE, MODBUS_EXCEPTION_MEMORY_PARITY, - MODBUS_EXCEPTION_NOT_DEFINED, + MODBUS_EXCEPTION_NOT_DEFINED, /* Do not use! (with callbacks) */ MODBUS_EXCEPTION_GATEWAY_PATH, MODBUS_EXCEPTION_GATEWAY_TARGET, MODBUS_EXCEPTION_MAX @@ -176,6 +176,29 @@ typedef enum MODBUS_ERROR_RECOVERY_PROTOCOL = (1<<2) } modbus_error_recovery_mode; +/* Each read callback function takes the address, number of registers, address + to response message and pointer to optional user data. The response is + pre-filled with the MBAP header AND FCode (function code), 8 bytes in total. + The callback is responsible for filling in the rest of the response PDU + according to Modbus Application Protocol V1.1b3, returning the size of the + PDU. */ +typedef int (*modbus_read_callback)(uint16_t addr, uint16_t nb, uint8_t *rsp, void *user_data); + +/* Each write callback function takes the address, number of registers, + address to the request to be written and pointer to optional user data. */ +typedef int (*modbus_write_callback)(uint16_t addr, uint16_t nb, const uint8_t *req, void *user_data); + +typedef struct { + modbus_read_callback read_coils; + modbus_read_callback read_discrete_inputs; + modbus_read_callback read_holding_registers; /* Also used for _FC_WRITE_AND_READ_REGISTERS */ + modbus_read_callback read_input_registers; + modbus_write_callback write_single_coil; + modbus_write_callback write_single_register; + modbus_write_callback write_multiple_coils; + modbus_write_callback write_multiple_registers; /* Also used for _FC_WRITE_AND_READ_REGISTERS */ +} modbus_callbacks_t; + MODBUS_API int modbus_set_slave(modbus_t* ctx, int slave); MODBUS_API int modbus_set_error_recovery(modbus_t *ctx, modbus_error_recovery_mode error_recovery); MODBUS_API int modbus_set_socket(modbus_t *ctx, int s); @@ -232,6 +255,10 @@ MODBUS_API int modbus_reply(modbus_t *ctx, const uint8_t *req, MODBUS_API int modbus_reply_exception(modbus_t *ctx, const uint8_t *req, unsigned int exception_code); +MODBUS_API int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, + int req_length, modbus_callbacks_t *mb_callbacks, + void *user_data); + /** * UTILS FUNCTIONS **/ From 817e51acc682ffe9d421d9f908f823d805509837 Mon Sep 17 00:00:00 2001 From: Yegor Yefremov Date: Wed, 30 Mar 2016 11:28:52 +0200 Subject: [PATCH 02/41] Change msinttypes URL to GitHub repository msinttypes moved to GitHub, so change URL accordingly. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 11b9dc9f7..28455eb7a 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ The library provides a *libmodbus.pc* file to use with `pkg-config` to ease your program compilation and linking. If you want to compile with Microsoft Visual Studio, you need to install - to fill the absence of stdint.h. + to fill the absence of stdint.h. To compile under Windows, install [MinGW](http://www.mingw.org/) and MSYS then select the common packages (gcc, automake, libtool, etc). The directory From 0214a9a71c0441b1526315ec7dc6087c3b43b482 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 27 Apr 2016 10:53:52 +0200 Subject: [PATCH 03/41] Add Jakob to AUTHORS with Contributor License Agreement --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index faba8dd3a..188dcb81a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -5,3 +5,4 @@ oldfaber Hannu Vuolasaho - CLA Michael Heimpold - CLA Jimmy Bergström - CLA +Jakob Bysewski - CLA \ No newline at end of file From 61854b2a66a83d01d701b3f828cfee7e52b69712 Mon Sep 17 00:00:00 2001 From: Yegor Yefremov Date: Mon, 25 Apr 2016 10:06:27 +0200 Subject: [PATCH 04/41] Add an option to disable tests compilation Signed-off-by: Yegor Yefremov --- Makefile.am | 6 +++++- configure.ac | 8 ++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index 158e04004..a0a165e18 100644 --- a/Makefile.am +++ b/Makefile.am @@ -9,4 +9,8 @@ CLEANFILES += libmodbus.pc dist_doc_DATA = MIGRATION README.md -SUBDIRS = src tests doc +SUBDIRS = src doc + +if BUILD_TESTS +SUBDIRS += tests +endif diff --git a/configure.ac b/configure.ac index 2e14ec9df..ddb95327d 100644 --- a/configure.ac +++ b/configure.ac @@ -141,6 +141,13 @@ my_CFLAGS="-Wall \ -Wformat-security" AC_SUBST([my_CFLAGS]) +# Build options +AC_ARG_ENABLE(tests, + AS_HELP_STRING([--disable-tests], + [Build tests (default: yes)]),, + [enable_tests=yes]) +AM_CONDITIONAL(BUILD_TESTS, [test $enable_tests != no]) + AC_CONFIG_HEADERS([config.h tests/unit-test.h]) AC_CONFIG_FILES([ Makefile @@ -167,4 +174,5 @@ AC_MSG_RESULT([ ldflags: ${LDFLAGS} documentation: ${ac_libmodbus_build_doc} + tests: ${enable_tests} ]) From 9478c33aeb61ddb560016281f8c88b5a493497ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Mon, 9 May 2016 09:46:58 +0200 Subject: [PATCH 05/41] Fix address range in random-test-client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Raimbault --- tests/random-test-client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/random-test-client.c b/tests/random-test-client.c index 69c9b0a11..2ebbded5b 100644 --- a/tests/random-test-client.c +++ b/tests/random-test-client.c @@ -86,7 +86,7 @@ int main(void) nb_loop = nb_fail = 0; while (nb_loop++ < LOOP) { - for (addr = ADDRESS_START; addr <= ADDRESS_END; addr++) { + for (addr = ADDRESS_START; addr < ADDRESS_END; addr++) { int i; /* Random numbers (short) */ From 7d61d48b33e2f2a46302ea7310bf15b3a1d1a6d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Tue, 10 May 2016 13:34:22 +0200 Subject: [PATCH 06/41] Change API modbus_mapping_(offsets_new) to (new_start_address) Related to 52ab1bbea760ed8eaca184f7d875a2f52a116d0f. The arguments have been changed (see documentation). https://groups.google.com/d/msg/libmodbus/aXO8nBzW4Ew/uVGTDmvvBAAJ --- doc/Makefile.am | 2 +- doc/modbus_mapping_new.txt | 6 +- doc/modbus_mapping_new_start_address.txt | 81 ++++++++++++++ doc/modbus_mapping_offset_new.txt | 70 ------------ src/modbus.c | 133 +++++++++++++---------- src/modbus.h | 20 ++-- 6 files changed, 170 insertions(+), 142 deletions(-) create mode 100644 doc/modbus_mapping_new_start_address.txt delete mode 100644 doc/modbus_mapping_offset_new.txt diff --git a/doc/Makefile.am b/doc/Makefile.am index 3e28b2f73..93b5a759d 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -15,7 +15,7 @@ TXT3 = \ modbus_get_socket.txt \ modbus_mapping_free.txt \ modbus_mapping_new.txt \ - modbus_mapping_offset_new.txt \ + modbus_mapping_new_start_address.txt \ modbus_mask_write_register.txt \ modbus_new_rtu.txt \ modbus_new_tcp_pi.txt \ diff --git a/doc/modbus_mapping_new.txt b/doc/modbus_mapping_new.txt index 293131b33..71cde1d63 100644 --- a/doc/modbus_mapping_new.txt +++ b/doc/modbus_mapping_new.txt @@ -18,8 +18,9 @@ The *modbus_mapping_new()* function shall allocate four arrays to store bits, input bits, registers and inputs registers. The pointers are stored in modbus_mapping_t structure. All values of the arrays are initialized to zero. -This function is equivalent to a call of the _modbus_mapping_offset_new()_ function -with all offsets set to zero. +This function is equivalent to a call of the +linkmb:modbus_mapping_new_start_address[3] function with all start addresses to +`0`. If it isn't necessary to allocate an array for a specific type of data, you can pass the zero value in argument, the associated pointer will be NULL. @@ -59,6 +60,7 @@ if (mb_mapping == NULL) { SEE ALSO -------- linkmb:modbus_mapping_free[3] +linkmb:modbus_mapping_new_start_address[3] AUTHORS diff --git a/doc/modbus_mapping_new_start_address.txt b/doc/modbus_mapping_new_start_address.txt new file mode 100644 index 000000000..4fa196ae5 --- /dev/null +++ b/doc/modbus_mapping_new_start_address.txt @@ -0,0 +1,81 @@ +modbus_mapping_new_start_address(3) +=================================== + + +NAME +---- +modbus_mapping_new_start_address - allocate four arrays of bits and registers accessible from their starting addresses + + +SYNOPSIS +-------- +*modbus_mapping_t* modbus_mapping_new_start_address(int 'start_bits', int 'nb_bits', + int 'start_input_bits', int 'nb_input_bits', + int 'start_registers', int 'nb_registers', + int 'start_input_registers', int 'nb_input_registers');* + + +DESCRIPTION +----------- +The _modbus_mapping_new_start_address()_ function shall allocate four arrays to +store bits, input bits, registers and inputs registers. The pointers are stored +in modbus_mapping_t structure. All values of the arrays are initialized to zero. + +The different starting adresses make it possible to place the mapping at any +address in each address space. This way, you can give access to values stored +at high adresses without allocating memory from the address zero, for eg. to +make available registers from 10000 to 10009, you can use: + +[source,c] +------------------- +mb_mapping = modbus_mapping_offset_start_address(0, 0, 0, 0, 10000, 10, 0, 0); +------------------- + +With this code, only 10 registers (`uint16_t`) are allocated. + +If it isn't necessary to allocate an array for a specific type of data, you can +pass the zero value in argument, the associated pointer will be NULL. + +This function is convenient to handle requests in a Modbus server/slave. + + +RETURN VALUE +------------ +The _modbus_mapping_offset_new()_ function shall return the new allocated structure if +successful. Otherwise it shall return NULL and set errno. + + +ERRORS +------ +ENOMEM:: +Not enough memory + + +EXAMPLE +------- +[source,c] +------------------- +/* The first value of each array is accessible at the defined address. + The end address is ADDRESS + NB - 1. */ +mb_mapping = modbus_mapping_offset_start_address(BITS_ADDRESS, BITS_NB, + INPUT_BITS_ADDRESS, INPUT_BITS_NB, + REGISTERS_ADDRESS, REGISTERS_NB, + INPUT_REGISTERS_ADDRESS, INPUT_REGISTERS_NB); +if (mb_mapping == NULL) { + fprintf(stderr, "Failed to allocate the mapping: %s\n", + modbus_strerror(errno)); + modbus_free(ctx); + return -1; +} +------------------- + +SEE ALSO +-------- +linkmb:modbus_mapping_new[3] +linkmb:modbus_mapping_free[3] + + +AUTHORS +------- +The libmodbus documentation was written by Stéphane Raimbault + diff --git a/doc/modbus_mapping_offset_new.txt b/doc/modbus_mapping_offset_new.txt deleted file mode 100644 index d860664ec..000000000 --- a/doc/modbus_mapping_offset_new.txt +++ /dev/null @@ -1,70 +0,0 @@ -modbus_mapping_offset_new(3) -============================ - - -NAME ----- -modbus_mapping_offset_new - allocate four arrays of bits and registers - - -SYNOPSIS --------- -*modbus_mapping_t* modbus_mapping_new(int 'nb_bits', int 'offset_bits', - int 'nb_input_bits', int 'offset_input_bits', - int 'nb_registers', int 'offset_registers', - int 'nb_input_registers', int 'offset_input_registers');* - - -DESCRIPTION ------------ -The _modbus_mapping_offset_new()_ function shall allocate four arrays to store bits, -input bits, registers and inputs registers. The pointers are stored in -modbus_mapping_t structure. All values of the arrays are initialized to zero. - -The different offsets make it possible to place the mapping at any address in -each address space. - -If it isn't necessary to allocate an array for a specific type of data, you can -pass the zero value in argument, the associated pointer will be NULL. - -This function is convenient to handle requests in a Modbus server/slave. - - -RETURN VALUE ------------- -The _modbus_mapping_offset_new()_ function shall return the new allocated structure if -successful. Otherwise it shall return NULL and set errno. - - -ERRORS ------- -ENOMEM:: -Not enough memory - - -EXAMPLE -------- -[source,c] -------------------- -/* The first value of each array is accessible at address 4. */ -mb_mapping = modbus_mapping_offset_new(BITS_ADDRESS + BITS_NB, 4, - INPUT_BITS_ADDRESS + INPUT_BITS_NB, 4, - REGISTERS_ADDRESS + REGISTERS_NB, 4, - INPUT_REGISTERS_ADDRESS + INPUT_REGISTERS_NB, 4); -if (mb_mapping == NULL) { - fprintf(stderr, "Failed to allocate the mapping: %s\n", - modbus_strerror(errno)); - modbus_free(ctx); - return -1; -} -------------------- - -SEE ALSO --------- -linkmb:modbus_mapping_free[3] - - -AUTHORS -------- -The libmodbus documentation was written by Stéphane Raimbault - diff --git a/src/modbus.c b/src/modbus.c index 37de05c27..58aa1a434 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -630,8 +630,8 @@ static int check_confirmation(modbus_t *ctx, uint8_t *req, return rc; } -static int response_io_status(int address, int nb, - uint8_t *tab_io_status, +static int response_io_status(uint8_t *tab_io_status, + int address, int nb, uint8_t *rsp, int offset) { int shift = 0; @@ -639,7 +639,7 @@ static int response_io_status(int address, int nb, int one_byte = 0; int i; - for (i = address; i < address+nb; i++) { + for (i = address; i < address + nb; i++) { one_byte |= tab_io_status[i] << shift; if (shift == 7) { /* Byte is full */ @@ -706,7 +706,9 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, switch (function) { case MODBUS_FC_READ_COILS: { int nb = (req[offset + 3] << 8) + req[offset + 4]; - int addr = address - mb_mapping->offset_bits; + /* The mapping can be shifted to reduce memory consumption and it + doesn't always start at address zero. */ + int mapping_address = address - mb_mapping->start_bits; if (nb < 1 || MODBUS_MAX_READ_BITS < nb) { if (ctx->debug) { @@ -719,10 +721,11 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); - } else if (address < mb_mapping->offset_bits || (addr + nb) > mb_mapping->nb_bits) { + } else if (mapping_address < 0 || + (mapping_address + nb) > mb_mapping->nb_bits) { if (ctx->debug) { fprintf(stderr, "Illegal data address 0x%0X in read_bits\n", - address < mb_mapping->offset_bits ? address : address + nb); + mapping_address < 0 ? address : address + nb); } rsp_length = response_exception( ctx, &sft, @@ -730,8 +733,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, } else { rsp_length = ctx->backend->build_response_basis(&sft, rsp); rsp[rsp_length++] = (nb / 8) + ((nb % 8) ? 1 : 0); - rsp_length = response_io_status(addr, nb, - mb_mapping->tab_bits, + rsp_length = response_io_status(mb_mapping->tab_bits, + mapping_address, nb, rsp, rsp_length); } } @@ -740,7 +743,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, /* Similar to coil status (but too many arguments to use a * function) */ int nb = (req[offset + 3] << 8) + req[offset + 4]; - int addr = address - mb_mapping->offset_input_bits; + int mapping_address = address - mb_mapping->start_input_bits; if (nb < 1 || MODBUS_MAX_READ_BITS < nb) { if (ctx->debug) { @@ -753,10 +756,11 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); - } else if (address < mb_mapping->offset_input_bits || (addr + nb) > mb_mapping->nb_input_bits) { + } else if (mapping_address < 0 || + (mapping_address + nb) > mb_mapping->nb_input_bits) { if (ctx->debug) { fprintf(stderr, "Illegal data address 0x%0X in read_input_bits\n", - address < mb_mapping->offset_input_bits ? address : address + nb); + mapping_address < 0 ? address : address + nb); } rsp_length = response_exception( ctx, &sft, @@ -764,15 +768,15 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, } else { rsp_length = ctx->backend->build_response_basis(&sft, rsp); rsp[rsp_length++] = (nb / 8) + ((nb % 8) ? 1 : 0); - rsp_length = response_io_status(addr, nb, - mb_mapping->tab_input_bits, + rsp_length = response_io_status(mb_mapping->tab_input_bits, + mapping_address, nb, rsp, rsp_length); } } break; case MODBUS_FC_READ_HOLDING_REGISTERS: { int nb = (req[offset + 3] << 8) + req[offset + 4]; - int addr = address - mb_mapping->offset_registers; + int mapping_address = address - mb_mapping->start_registers; if (nb < 1 || MODBUS_MAX_READ_REGISTERS < nb) { if (ctx->debug) { @@ -785,10 +789,11 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); - } else if (address < mb_mapping->offset_registers || (addr + nb) > mb_mapping->nb_registers) { + } else if (mapping_address < 0 || + (mapping_address + nb) > mb_mapping->nb_registers) { if (ctx->debug) { fprintf(stderr, "Illegal data address 0x%0X in read_registers\n", - address < mb_mapping->offset_registers ? address : address + nb); + mapping_address < 0 ? address : address + nb); } rsp_length = response_exception( ctx, &sft, @@ -798,7 +803,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, rsp_length = ctx->backend->build_response_basis(&sft, rsp); rsp[rsp_length++] = nb << 1; - for (i = addr; i < addr + nb; i++) { + for (i = mapping_address; i < mapping_address + nb; i++) { rsp[rsp_length++] = mb_mapping->tab_registers[i] >> 8; rsp[rsp_length++] = mb_mapping->tab_registers[i] & 0xFF; } @@ -809,7 +814,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, /* Similar to holding registers (but too many arguments to use a * function) */ int nb = (req[offset + 3] << 8) + req[offset + 4]; - int addr = address - mb_mapping->offset_input_registers; + int mapping_address = address - mb_mapping->start_input_registers; if (nb < 1 || MODBUS_MAX_READ_REGISTERS < nb) { if (ctx->debug) { @@ -822,10 +827,11 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); - } else if (address < mb_mapping->offset_input_registers || (addr + nb) > mb_mapping->nb_input_registers) { + } else if (mapping_address < 0 || + (mapping_address + nb) > mb_mapping->nb_input_registers) { if (ctx->debug) { fprintf(stderr, "Illegal data address 0x%0X in read_input_registers\n", - address < mb_mapping->offset_input_registers ? address : address + nb); + mapping_address < 0 ? address : address + nb); } rsp_length = response_exception( ctx, &sft, @@ -835,7 +841,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, rsp_length = ctx->backend->build_response_basis(&sft, rsp); rsp[rsp_length++] = nb << 1; - for (i = addr; i < addr + nb; i++) { + for (i = mapping_address; i < mapping_address + nb; i++) { rsp[rsp_length++] = mb_mapping->tab_input_registers[i] >> 8; rsp[rsp_length++] = mb_mapping->tab_input_registers[i] & 0xFF; } @@ -843,9 +849,9 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, } break; case MODBUS_FC_WRITE_SINGLE_COIL: { - int addr = address - mb_mapping->offset_bits; + int mapping_address = address - mb_mapping->start_bits; - if (address < mb_mapping->offset_bits || addr >= mb_mapping->nb_bits) { + if (mapping_address < 0 || mapping_address >= mb_mapping->nb_bits) { if (ctx->debug) { fprintf(stderr, "Illegal data address 0x%0X in write_bit\n", @@ -858,7 +864,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, int data = (req[offset + 3] << 8) + req[offset + 4]; if (data == 0xFF00 || data == 0x0) { - mb_mapping->tab_bits[addr] = (data) ? ON : OFF; + mb_mapping->tab_bits[mapping_address] = data ? ON : OFF; memcpy(rsp, req, req_length); rsp_length = req_length; } else { @@ -875,9 +881,9 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, } break; case MODBUS_FC_WRITE_SINGLE_REGISTER: { - int addr = address - mb_mapping->offset_registers; + int mapping_address = address - mb_mapping->start_registers; - if (address < mb_mapping->offset_registers || addr >= mb_mapping->nb_registers) { + if (mapping_address < 0 || mapping_address >= mb_mapping->nb_registers) { if (ctx->debug) { fprintf(stderr, "Illegal data address 0x%0X in write_register\n", address); @@ -888,7 +894,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, } else { int data = (req[offset + 3] << 8) + req[offset + 4]; - mb_mapping->tab_registers[addr] = data; + mb_mapping->tab_registers[mapping_address] = data; memcpy(rsp, req, req_length); rsp_length = req_length; } @@ -896,7 +902,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, break; case MODBUS_FC_WRITE_MULTIPLE_COILS: { int nb = (req[offset + 3] << 8) + req[offset + 4]; - int addr = address - mb_mapping->offset_bits; + int mapping_address = address - mb_mapping->start_bits; if (nb < 1 || MODBUS_MAX_WRITE_BITS < nb) { if (ctx->debug) { @@ -912,17 +918,19 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); - } else if (address < mb_mapping->offset_bits || (addr + nb) > mb_mapping->nb_bits) { + } else if (mapping_address < 0 || + (mapping_address + nb) > mb_mapping->nb_bits) { if (ctx->debug) { fprintf(stderr, "Illegal data address 0x%0X in write_bits\n", - address < mb_mapping->offset_bits ? address : address + nb); + mapping_address < 0 ? address : address + nb); } rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); } else { /* 6 = byte count */ - modbus_set_bits_from_bytes(mb_mapping->tab_bits, addr, nb, &req[offset + 6]); + modbus_set_bits_from_bytes(mb_mapping->tab_bits, mapping_address, nb, + &req[offset + 6]); rsp_length = ctx->backend->build_response_basis(&sft, rsp); /* 4 to copy the bit address (2) and the quantity of bits */ @@ -933,7 +941,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, break; case MODBUS_FC_WRITE_MULTIPLE_REGISTERS: { int nb = (req[offset + 3] << 8) + req[offset + 4]; - int addr = address - mb_mapping->offset_registers; + int mapping_address = address - mb_mapping->start_registers; if (nb < 1 || MODBUS_MAX_WRITE_REGISTERS < nb) { if (ctx->debug) { @@ -949,17 +957,18 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); - } else if (address < mb_mapping->offset_registers || (addr + nb) > mb_mapping->nb_registers) { + } else if (mapping_address < 0 || + (mapping_address + nb) > mb_mapping->nb_registers) { if (ctx->debug) { fprintf(stderr, "Illegal data address 0x%0X in write_registers\n", - address < mb_mapping->offset_registers ? address : address + nb); + mapping_address < 0 ? address : address + nb); } rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); } else { int i, j; - for (i = addr, j = 6; i < addr + nb; i++, j += 2) { + for (i = mapping_address, j = 6; i < mapping_address + nb; i++, j += 2) { /* 6 and 7 = first value */ mb_mapping->tab_registers[i] = (req[offset + j] << 8) + req[offset + j + 1]; @@ -997,9 +1006,9 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, return -1; break; case MODBUS_FC_MASK_WRITE_REGISTER: { - int addr = address - mb_mapping->offset_registers; + int mapping_address = address - mb_mapping->start_registers; - if (address < mb_mapping->offset_registers || addr >= mb_mapping->nb_registers) { + if (mapping_address < 0 || mapping_address >= mb_mapping->nb_registers) { if (ctx->debug) { fprintf(stderr, "Illegal data address 0x%0X in write_register\n", address); @@ -1008,12 +1017,12 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); } else { - uint16_t data = mb_mapping->tab_registers[addr]; + uint16_t data = mb_mapping->tab_registers[mapping_address]; uint16_t and = (req[offset + 3] << 8) + req[offset + 4]; uint16_t or = (req[offset + 5] << 8) + req[offset + 6]; data = (data & and) | (or & (~and)); - mb_mapping->tab_registers[addr] = data; + mb_mapping->tab_registers[mapping_address] = data; memcpy(rsp, req, req_length); rsp_length = req_length; } @@ -1024,8 +1033,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, uint16_t address_write = (req[offset + 5] << 8) + req[offset + 6]; int nb_write = (req[offset + 7] << 8) + req[offset + 8]; int nb_write_bytes = req[offset + 9]; - int addr = address - mb_mapping->offset_registers; - int addr_write = address_write - mb_mapping->offset_registers; + int mapping_address = address - mb_mapping->start_registers; + int mapping_address_write = address_write - mb_mapping->start_registers; if (nb_write < 1 || MODBUS_MAX_WR_WRITE_REGISTERS < nb_write || nb < 1 || MODBUS_MAX_WR_READ_REGISTERS < nb || @@ -1041,15 +1050,15 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); - } else if (address < mb_mapping->offset_registers || - (addr + nb) > mb_mapping->nb_registers || - address_write < mb_mapping->offset_registers || - (addr_write + nb_write) > mb_mapping->nb_registers) { + } else if (mapping_address < 0 || + (mapping_address + nb) > mb_mapping->nb_registers || + mapping_address < 0 || + (mapping_address_write + nb_write) > mb_mapping->nb_registers) { if (ctx->debug) { fprintf(stderr, "Illegal data read address 0x%0X or write address 0x%0X write_and_read_registers\n", - address < mb_mapping->offset_registers ? address : address + nb, - address_write < mb_mapping->offset_registers ? address_write : address_write + nb_write); + mapping_address < 0 ? address : address + nb, + mapping_address_write < 0 ? address_write : address_write + nb_write); } rsp_length = response_exception(ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); @@ -1060,13 +1069,14 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, /* Write first. 10 and 11 are the offset of the first values to write */ - for (i = addr_write, j = 10; i < addr_write + nb_write; i++, j += 2) { + for (i = mapping_address_write, j = 10; + i < mapping_address_write + nb_write; i++, j += 2) { mb_mapping->tab_registers[i] = (req[offset + j] << 8) + req[offset + j + 1]; } /* and read the data for the response */ - for (i = addr; i < addr + nb; i++) { + for (i = mapping_address; i < mapping_address + nb; i++) { rsp[rsp_length++] = mb_mapping->tab_registers[i] >> 8; rsp[rsp_length++] = mb_mapping->tab_registers[i] & 0xFF; } @@ -2103,12 +2113,14 @@ int modbus_set_debug(modbus_t *ctx, int flag) /* Allocates 4 arrays to store bits, input bits, registers and inputs registers. The pointers are stored in modbus_mapping structure. - The modbus_mapping_offset_new() function shall return the new allocated structure if - successful. Otherwise it shall return NULL and set errno to ENOMEM. */ -modbus_mapping_t* modbus_mapping_offset_new(int nb_bits, int offset_bits, - int nb_input_bits, int offset_input_bits, - int nb_registers, int offset_registers, - int nb_input_registers, int offset_input_registers) + The modbus_mapping_new_ranges() function shall return the new allocated + structure if successful. Otherwise it shall return NULL and set errno to + ENOMEM. */ +modbus_mapping_t* modbus_mapping_new_start_address( + unsigned int start_bits, unsigned int nb_bits, + unsigned int start_input_bits, unsigned int nb_input_bits, + unsigned int start_registers, unsigned int nb_registers, + unsigned int start_input_registers, unsigned int nb_input_registers) { modbus_mapping_t *mb_mapping; @@ -2119,7 +2131,7 @@ modbus_mapping_t* modbus_mapping_offset_new(int nb_bits, int offset_bits, /* 0X */ mb_mapping->nb_bits = nb_bits; - mb_mapping->offset_bits = offset_bits; + mb_mapping->start_bits = start_bits; if (nb_bits == 0) { mb_mapping->tab_bits = NULL; } else { @@ -2135,7 +2147,7 @@ modbus_mapping_t* modbus_mapping_offset_new(int nb_bits, int offset_bits, /* 1X */ mb_mapping->nb_input_bits = nb_input_bits; - mb_mapping->offset_input_bits = offset_input_bits; + mb_mapping->start_input_bits = start_input_bits; if (nb_input_bits == 0) { mb_mapping->tab_input_bits = NULL; } else { @@ -2151,7 +2163,7 @@ modbus_mapping_t* modbus_mapping_offset_new(int nb_bits, int offset_bits, /* 4X */ mb_mapping->nb_registers = nb_registers; - mb_mapping->offset_registers = offset_registers; + mb_mapping->start_registers = start_registers; if (nb_registers == 0) { mb_mapping->tab_registers = NULL; } else { @@ -2168,7 +2180,7 @@ modbus_mapping_t* modbus_mapping_offset_new(int nb_bits, int offset_bits, /* 3X */ mb_mapping->nb_input_registers = nb_input_registers; - mb_mapping->offset_input_registers = offset_input_registers; + mb_mapping->start_input_registers = start_input_registers; if (nb_input_registers == 0) { mb_mapping->tab_input_registers = NULL; } else { @@ -2191,7 +2203,8 @@ modbus_mapping_t* modbus_mapping_offset_new(int nb_bits, int offset_bits, modbus_mapping_t* modbus_mapping_new(int nb_bits, int nb_input_bits, int nb_registers, int nb_input_registers) { - return modbus_mapping_offset_new(nb_bits, 0, nb_input_bits, 0, nb_registers, 0, nb_input_registers, 0); + return modbus_mapping_new_start_address( + 0, nb_bits, 0, nb_input_bits, 0, nb_registers, 0, nb_input_registers); } /* Frees the 4 arrays */ diff --git a/src/modbus.h b/src/modbus.h index 13be38f61..886f37a50 100644 --- a/src/modbus.h +++ b/src/modbus.h @@ -156,13 +156,13 @@ typedef struct _modbus modbus_t; typedef struct { int nb_bits; - int offset_bits; + int start_bits; int nb_input_bits; - int offset_input_bits; + int start_input_bits; int nb_input_registers; - int offset_input_registers; + int start_input_registers; int nb_registers; - int offset_registers; + int start_registers; uint8_t *tab_bits; uint8_t *tab_input_bits; uint16_t *tab_input_registers; @@ -236,12 +236,14 @@ MODBUS_API int modbus_write_and_read_registers(modbus_t *ctx, int write_addr, in uint16_t *dest); MODBUS_API int modbus_report_slave_id(modbus_t *ctx, int max_dest, uint8_t *dest); -MODBUS_API modbus_mapping_t* modbus_mapping_offset_new(int nb_bits, int offset_bits, - int nb_input_bits, int offset_input_bits, - int nb_registers, int offset_registers, - int nb_input_registers, int offset_input_registers); +MODBUS_API modbus_mapping_t* modbus_mapping_new_start_address( + unsigned int start_bits, unsigned int nb_bits, + unsigned int start_input_bits, unsigned int nb_input_bits, + unsigned int start_registers, unsigned int nb_registers, + unsigned int start_input_registers, unsigned int nb_input_registers); + MODBUS_API modbus_mapping_t* modbus_mapping_new(int nb_bits, int nb_input_bits, - int nb_registers, int nb_input_registers); + int nb_registers, int nb_input_registers); MODBUS_API void modbus_mapping_free(modbus_mapping_t *mb_mapping); MODBUS_API int modbus_send_raw_request(modbus_t *ctx, uint8_t *raw_req, int raw_req_length); From fcdfd6315e6f710b038877eb31c251985b39edd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Tue, 10 May 2016 15:41:04 +0200 Subject: [PATCH 07/41] Use new modbus_mapping_new_start_address in unit tests --- tests/unit-test-client.c | 23 ++++++++++---------- tests/unit-test-server.c | 46 ++++++++-------------------------------- tests/unit-test.h.in | 17 ++++++++------- 3 files changed, 29 insertions(+), 57 deletions(-) diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index cfb2c188d..911605c78 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -146,8 +146,7 @@ int main(int argc, char *argv[]) uint8_t tab_value[UT_BITS_NB]; modbus_set_bits_from_bytes(tab_value, 0, UT_BITS_NB, UT_BITS_TAB); - rc = modbus_write_bits(ctx, UT_BITS_ADDRESS, - UT_BITS_NB, tab_value); + rc = modbus_write_bits(ctx, UT_BITS_ADDRESS, UT_BITS_NB, tab_value); printf("1/2 modbus_write_bits: "); ASSERT_TRUE(rc == UT_BITS_NB, ""); } @@ -325,7 +324,7 @@ int main(int argc, char *argv[]) ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS, - UT_REGISTERS_NB + 1, tab_rp_registers); + UT_REGISTERS_NB_MAX + 1, tab_rp_registers); printf("* modbus_read_registers: "); ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); @@ -344,12 +343,12 @@ int main(int argc, char *argv[]) printf("* modbus_write_coils: "); ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); - rc = modbus_write_register(ctx, UT_REGISTERS_ADDRESS + UT_REGISTERS_NB, + rc = modbus_write_register(ctx, UT_REGISTERS_ADDRESS + UT_REGISTERS_NB_MAX, tab_rp_registers[0]); printf("* modbus_write_register: "); ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); - rc = modbus_write_registers(ctx, UT_REGISTERS_ADDRESS + UT_REGISTERS_NB, + rc = modbus_write_registers(ctx, UT_REGISTERS_ADDRESS + UT_REGISTERS_NB_MAX, UT_REGISTERS_NB, tab_rp_registers); printf("* modbus_write_registers: "); ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); @@ -639,8 +638,10 @@ int test_server(modbus_t *ctx, int use_backend) uint8_t read_raw_req[] = { /* slave */ (use_backend == RTU) ? SERVER_ID : 0xFF, - /* function, addr 1, 5 values */ - MODBUS_FC_READ_HOLDING_REGISTERS, 0x00, 0x01, 0x0, 0x05 + /* function, address, 5 values */ + MODBUS_FC_READ_HOLDING_REGISTERS, + UT_REGISTERS_ADDRESS >> 8, UT_REGISTERS_ADDRESS & 0xFF, + 0x0, 0x05 }; /* Write and read registers request */ const int RW_RAW_REQ_LEN = 13; @@ -650,7 +651,7 @@ int test_server(modbus_t *ctx, int use_backend) /* function, addr to read, nb to read */ MODBUS_FC_WRITE_AND_READ_REGISTERS, /* Read */ - 0, 0, + UT_REGISTERS_ADDRESS >> 8, UT_REGISTERS_ADDRESS & 0xFF, (MODBUS_MAX_WR_READ_REGISTERS + 1) >> 8, (MODBUS_MAX_WR_READ_REGISTERS + 1) & 0xFF, /* Write */ @@ -668,8 +669,7 @@ int test_server(modbus_t *ctx, int use_backend) /* function will be set in the loop */ MODBUS_FC_WRITE_MULTIPLE_REGISTERS, /* Address */ - UT_REGISTERS_ADDRESS >> 8, - UT_REGISTERS_ADDRESS & 0xFF, + UT_REGISTERS_ADDRESS >> 8, UT_REGISTERS_ADDRESS & 0xFF, /* 3 values, 6 bytes */ 0x00, 0x03, 0x06, /* Dummy data to write */ @@ -711,8 +711,7 @@ int test_server(modbus_t *ctx, int use_backend) ASSERT_TRUE(rc == (backend_length + 12), "FAILED (%d)\n", rc); /* Try to read more values than a response could hold for all data - * types. - */ + types. */ for (i=0; i<4; i++) { rc = send_crafted_request(ctx, tab_read_function[i], read_raw_req, READ_RAW_REQ_LEN, diff --git a/tests/unit-test-server.c b/tests/unit-test-server.c index ebcfdb9c1..fe2fc7625 100644 --- a/tests/unit-test-server.c +++ b/tests/unit-test-server.c @@ -71,11 +71,11 @@ int main(int argc, char*argv[]) modbus_set_debug(ctx, TRUE); - mb_mapping = modbus_mapping_new( - UT_BITS_ADDRESS + UT_BITS_NB, - UT_INPUT_BITS_ADDRESS + UT_INPUT_BITS_NB, - UT_REGISTERS_ADDRESS + UT_REGISTERS_NB, - UT_INPUT_REGISTERS_ADDRESS + UT_INPUT_REGISTERS_NB); + mb_mapping = modbus_mapping_new_start_address( + UT_BITS_ADDRESS, UT_BITS_NB, + UT_INPUT_BITS_ADDRESS, UT_INPUT_BITS_NB, + UT_REGISTERS_ADDRESS, UT_REGISTERS_NB_MAX, + UT_INPUT_REGISTERS_ADDRESS, UT_INPUT_REGISTERS_NB); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); @@ -83,44 +83,16 @@ int main(int argc, char*argv[]) return -1; } - /* Unit tests of modbus_mapping_new (tests would not be sufficient if two - nb_* were identical) */ - if (mb_mapping->nb_bits != UT_BITS_ADDRESS + UT_BITS_NB) { - printf("Invalid nb bits (%d != %d)\n", UT_BITS_ADDRESS + UT_BITS_NB, mb_mapping->nb_bits); - modbus_free(ctx); - return -1; - } - - if (mb_mapping->nb_input_bits != UT_INPUT_BITS_ADDRESS + UT_INPUT_BITS_NB) { - printf("Invalid nb input bits: %d\n", UT_INPUT_BITS_ADDRESS + UT_INPUT_BITS_NB); - modbus_free(ctx); - return -1; - } - - if (mb_mapping->nb_registers != UT_REGISTERS_ADDRESS + UT_REGISTERS_NB) { - printf("Invalid nb registers: %d\n", UT_REGISTERS_ADDRESS + UT_REGISTERS_NB); - modbus_free(ctx); - return -1; - } - - if (mb_mapping->nb_input_registers != UT_INPUT_REGISTERS_ADDRESS + UT_INPUT_REGISTERS_NB) { - printf("Invalid nb input registers: %d\n", UT_INPUT_REGISTERS_ADDRESS + UT_INPUT_REGISTERS_NB); - modbus_free(ctx); - return -1; - } - /* Examples from PI_MODBUS_300.pdf. Only the read-only input values are assigned. */ - /** INPUT STATUS **/ - modbus_set_bits_from_bytes(mb_mapping->tab_input_bits, - UT_INPUT_BITS_ADDRESS, UT_INPUT_BITS_NB, + /* Initialize input values that's can be only done server side. */ + modbus_set_bits_from_bytes(mb_mapping->tab_input_bits, 0, UT_INPUT_BITS_NB, UT_INPUT_BITS_TAB); - /** INPUT REGISTERS **/ + /* Initialize values of INPUT REGISTERS */ for (i=0; i < UT_INPUT_REGISTERS_NB; i++) { - mb_mapping->tab_input_registers[UT_INPUT_REGISTERS_ADDRESS+i] = - UT_INPUT_REGISTERS_TAB[i];; + mb_mapping->tab_input_registers[i] = UT_INPUT_REGISTERS_TAB[i];; } if (use_backend == TCP) { diff --git a/tests/unit-test.h.in b/tests/unit-test.h.in index b753ad31b..dca826f46 100644 --- a/tests/unit-test.h.in +++ b/tests/unit-test.h.in @@ -25,7 +25,6 @@ #define SERVER_ID 17 #define INVALID_SERVER_ID 18 -/* Server allocates address + nb */ const uint16_t UT_BITS_ADDRESS = 0x130; const uint16_t UT_BITS_NB = 0x25; const uint8_t UT_BITS_TAB[] = { 0xCD, 0x6B, 0xB2, 0x0E, 0x1B }; @@ -34,18 +33,20 @@ const uint16_t UT_INPUT_BITS_ADDRESS = 0x1C4; const uint16_t UT_INPUT_BITS_NB = 0x16; const uint8_t UT_INPUT_BITS_TAB[] = { 0xAC, 0xDB, 0x35 }; -const uint16_t UT_REGISTERS_ADDRESS = 0x16B; +const uint16_t UT_REGISTERS_ADDRESS = 0x160; +const uint16_t UT_REGISTERS_NB = 0x3; +const uint16_t UT_REGISTERS_NB_MAX = 0x20; +const uint16_t UT_REGISTERS_TAB[] = { 0x022B, 0x0001, 0x0064 }; + /* Raise a manual exception when this address is used for the first byte */ -const uint16_t UT_REGISTERS_ADDRESS_SPECIAL = 0x6C; +const uint16_t UT_REGISTERS_ADDRESS_SPECIAL = 0x170; /* The response of the server will contains an invalid TID or slave */ -const uint16_t UT_REGISTERS_ADDRESS_INVALID_TID_OR_SLAVE = 0x6D; +const uint16_t UT_REGISTERS_ADDRESS_INVALID_TID_OR_SLAVE = 0x171; /* The server will wait for 1 second before replying to test timeout */ -const uint16_t UT_REGISTERS_ADDRESS_SLEEP_500_MS = 0x6E; +const uint16_t UT_REGISTERS_ADDRESS_SLEEP_500_MS = 0x172; /* The server will wait for 5 ms before sending each byte */ -const uint16_t UT_REGISTERS_ADDRESS_BYTE_SLEEP_5_MS = 0x6F; +const uint16_t UT_REGISTERS_ADDRESS_BYTE_SLEEP_5_MS = 0x173; -const uint16_t UT_REGISTERS_NB = 0x3; -const uint16_t UT_REGISTERS_TAB[] = { 0x022B, 0x0001, 0x0064 }; /* If the following value is used, a bad response is sent. It's better to test with a lower value than UT_REGISTERS_NB_POINTS to try to raise a segfault. */ From b5f77b77507035ac466f4927adb1cc6841f0aad1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Tue, 10 May 2016 16:22:52 +0200 Subject: [PATCH 08/41] Add more tests for new modbus_mapping_new_start_address --- tests/unit-test-client.c | 75 ++++++++++++++++++++++++++++++++++------ 1 file changed, 64 insertions(+), 11 deletions(-) diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index 911605c78..c790b31e3 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -311,46 +311,99 @@ int main(int argc, char *argv[]) /** ILLEGAL DATA ADDRESS **/ printf("\nTEST ILLEGAL DATA ADDRESS:\n"); - /* The mapping begins at 0 and ends at address + nb_points so - * the addresses are not valid. */ + /* The mapping begins at the defined addresses and ends at address + + * nb_points so these addresses are not valid. */ + + rc = modbus_read_bits(ctx, 0, 1, tab_rp_bits); + printf("* modbus_read_bits (0): "); + ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); rc = modbus_read_bits(ctx, UT_BITS_ADDRESS, UT_BITS_NB + 1, tab_rp_bits); - printf("* modbus_read_bits: "); + printf("* modbus_read_bits (max): "); + ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); + + rc = modbus_read_input_bits(ctx, 0, 1, tab_rp_bits); + printf("* modbus_read_input_bits (0): "); ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); rc = modbus_read_input_bits(ctx, UT_INPUT_BITS_ADDRESS, UT_INPUT_BITS_NB + 1, tab_rp_bits); - printf("* modbus_read_input_bits: "); + printf("* modbus_read_input_bits (max): "); + ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); + + rc = modbus_read_registers(ctx, 0, 1, tab_rp_registers); + printf("* modbus_read_registers (0): "); ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS, UT_REGISTERS_NB_MAX + 1, tab_rp_registers); - printf("* modbus_read_registers: "); + printf("* modbus_read_registers (max): "); + ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); + + rc = modbus_read_input_registers(ctx, 0, 1, tab_rp_registers); + printf("* modbus_read_input_registers (0): "); ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); rc = modbus_read_input_registers(ctx, UT_INPUT_REGISTERS_ADDRESS, UT_INPUT_REGISTERS_NB + 1, tab_rp_registers); - printf("* modbus_read_input_registers: "); + printf("* modbus_read_input_registers (max): "); + ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); + + rc = modbus_write_bit(ctx, 0, ON); + printf("* modbus_write_bit (0): "); ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); rc = modbus_write_bit(ctx, UT_BITS_ADDRESS + UT_BITS_NB, ON); - printf("* modbus_write_bit: "); + printf("* modbus_write_bit (max): "); + ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); + + rc = modbus_write_bits(ctx, 0, 1, tab_rp_bits); + printf("* modbus_write_coils (0): "); ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); rc = modbus_write_bits(ctx, UT_BITS_ADDRESS + UT_BITS_NB, UT_BITS_NB, tab_rp_bits); - printf("* modbus_write_coils: "); + printf("* modbus_write_coils (max): "); + ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); + + rc = modbus_write_register(ctx, 0, tab_rp_registers[0]); + printf("* modbus_write_register (0): "); ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); rc = modbus_write_register(ctx, UT_REGISTERS_ADDRESS + UT_REGISTERS_NB_MAX, tab_rp_registers[0]); - printf("* modbus_write_register: "); + printf("* modbus_write_register (max): "); + ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); + + rc = modbus_write_registers(ctx, 0, 1, tab_rp_registers); + printf("* modbus_write_registers (0): "); ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); rc = modbus_write_registers(ctx, UT_REGISTERS_ADDRESS + UT_REGISTERS_NB_MAX, - UT_REGISTERS_NB, tab_rp_registers); - printf("* modbus_write_registers: "); + UT_REGISTERS_NB, tab_rp_registers); + printf("* modbus_write_registers (max): "); + ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); + + rc = modbus_mask_write_register(ctx, 0, 0xF2, 0x25); + printf("* modbus_mask_write_registers (0): "); + ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); + + rc = modbus_mask_write_register(ctx, UT_REGISTERS_ADDRESS + UT_REGISTERS_NB_MAX, + 0xF2, 0x25); + printf("* modbus_mask_write_registers (max): "); + ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); + + rc = modbus_write_and_read_registers(ctx, 0, 1, tab_rp_registers, 0, 1, tab_rp_registers); + printf("* modbus_write_and_read_registers (0): "); + ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); + + rc = modbus_write_and_read_registers(ctx, + UT_REGISTERS_ADDRESS + UT_REGISTERS_NB_MAX, + UT_REGISTERS_NB, tab_rp_registers, + UT_REGISTERS_ADDRESS + UT_REGISTERS_NB_MAX, + UT_REGISTERS_NB, tab_rp_registers); + printf("* modbus_write_and_read_registers (max): "); ASSERT_TRUE(rc == -1 && errno == EMBXILADD, ""); /** TOO MANY DATA **/ From 0892deddbd2011dc4a8097da74f644a8a18b2bd4 Mon Sep 17 00:00:00 2001 From: Shoichi Sakane Date: Sat, 30 Apr 2016 19:37:21 +0900 Subject: [PATCH 09/41] Fix "wildcard address" in TCP IPv6 --- src/modbus-tcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modbus-tcp.c b/src/modbus-tcp.c index d19b36a64..b6d42b284 100644 --- a/src/modbus-tcp.c +++ b/src/modbus-tcp.c @@ -854,7 +854,7 @@ modbus_t* modbus_new_tcp_pi(const char *node, const char *service) if (node == NULL) { /* The node argument can be empty to indicate any hosts */ - ctx_tcp_pi->node[0] = '0'; + ctx_tcp_pi->node[0] = 0; } else { dest_size = sizeof(char) * _MODBUS_TCP_PI_NODE_LENGTH; ret_size = strlcpy(ctx_tcp_pi->node, node, dest_size); From de496beecacfa76c9afedf1692073939298cc77e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 11 May 2016 11:51:27 +0200 Subject: [PATCH 10/41] Improve documentation of modbus_report_slave_id Inspired by https://github.com/remakeelectric/libmodbus/commit/56b2526f5140a7e7efd0a59d5f2e5fb200f5a632 --- doc/modbus_report_slave_id.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/modbus_report_slave_id.txt b/doc/modbus_report_slave_id.txt index e72717717..5307d5c19 100644 --- a/doc/modbus_report_slave_id.txt +++ b/doc/modbus_report_slave_id.txt @@ -26,7 +26,8 @@ The response stored in _dest_ contains: * additional data specific to each controller. For example, libmodbus returns the version of the library as a string. -The function write at most _max_dest_ bytes from the response to _dest_. +The function writes at most _max_dest_ bytes from the response to _dest_ so +you must take to allocate enough memory to store the results. RETURN VALUE ------------ From 31100e319c2339d36ea871d1b9145825519a3d17 Mon Sep 17 00:00:00 2001 From: Jakob Bysewski Date: Sun, 13 Mar 2016 12:53:37 +0100 Subject: [PATCH 11/41] Add bswap macro to compile on OSX --- src/modbus-data.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/modbus-data.c b/src/modbus-data.c index 4b549191d..e69cf1c09 100644 --- a/src/modbus-data.c +++ b/src/modbus-data.c @@ -27,6 +27,13 @@ # include #endif +#if defined(__APPLE__) + #include + #define bswap_16 OSSwapInt16 + #define bswap_32 OSSwapInt32 + #define bswap_64 OSSwapInt64 +#endif + #if defined(__GNUC__) # define GCC_VERSION (__GNUC__ * 100 + __GNUC_MINOR__ * 10) # if GCC_VERSION >= 430 From 471cffffcb4b3bffd7b0c4dc632e843ad2a34174 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 11 May 2016 12:15:21 +0200 Subject: [PATCH 12/41] Minor --- src/modbus-data.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modbus-data.c b/src/modbus-data.c index e69cf1c09..bbcbab870 100644 --- a/src/modbus-data.c +++ b/src/modbus-data.c @@ -31,7 +31,7 @@ #include #define bswap_16 OSSwapInt16 #define bswap_32 OSSwapInt32 - #define bswap_64 OSSwapInt64 + #define bswap_64 OSSwapInt64 #endif #if defined(__GNUC__) From b2d6069c5e1cd22b3657e32b848e763271683bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 11 May 2016 12:31:02 +0200 Subject: [PATCH 13/41] Update NEWS file --- NEWS | 20 ++++++++++++++++---- configure.ac | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index f2475aba0..c7cc62cb5 100644 --- a/NEWS +++ b/NEWS @@ -1,8 +1,20 @@ -libmodbus 3.1.3 (XXXX-XX-XX) +libmodbus 3.1.3 (2016-05-11) ============================ -- Introduce offsets in modbus mappings. -- Add some more macros for data manipulation. +- New bswap macros for Max OSX by Jakob Bysewski. +- Fix "wildcard address" in TCP IPv6 by Shoichi Sakane. +- Introduce offsets in modbus mappings with modbus_mapping_new_start_address. + Thanks to Michael Heimpold and Stéphane Raimbault. +- Fix address range in random-test-client. + Thanks to Martin Galvan. +- Add an option to disable tests compilation by Yegor Yefremov. +- Define MSG_DONTWAIT to MSG_NONBLOCK on AIX (#294). + Thanks to Fabrice Cantos. +- Fix building when byteswap.h is not defined by Tomasz Mon. +- Add some more macros for data manipulation and documentation. +- Remove duplicate install of modbus.h (closes #290). + Thanks to Daniel Sutcliffe. +- Move MIGRATION and README.md to dist_doc_DATA target. - Change order of few functions in modbus RTU code. - Add entries for modbus_rtu_[get|set]_delay in documentation index. - Implemented runtime configurable RTS delay by Jimmy Bergström. @@ -12,7 +24,7 @@ libmodbus 3.1.3 (XXXX-XX-XX) - Added ILLEGAL_DATA_ADDRESS tests for modbus_write_register[|s]. Thanks to Andrey Skvortsov. - Update documentation of modbus_rtu_set_rts -- fix rts signal switch time by Hiromasa Ihara. +- Fix rts signal switch time by Hiromasa Ihara. - Improve new_rtu and set_slave documentation (related to #276). - Fix late check of ctx in modbus_reply[|_exception] (closes #269). - Wait the server for 1 second before running tests (help Travis). diff --git a/configure.ac b/configure.ac index ddb95327d..e8df6a74b 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # m4_define([libmodbus_version_major], [3]) m4_define([libmodbus_version_minor], [1]) -m4_define([libmodbus_version_micro], [2]) +m4_define([libmodbus_version_micro], [3]) m4_define([libmodbus_release_status], [m4_if(m4_eval(libmodbus_version_minor % 2), [1], [snapshot], [release])]) From adb117720b98e4cd22bb0972018f5ffe95778c1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 11 May 2016 14:13:06 +0200 Subject: [PATCH 14/41] Slight change to modbus_report_slave_id doc. Thanks to Karl Palsson. --- doc/modbus_report_slave_id.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/modbus_report_slave_id.txt b/doc/modbus_report_slave_id.txt index 5307d5c19..6aedec679 100644 --- a/doc/modbus_report_slave_id.txt +++ b/doc/modbus_report_slave_id.txt @@ -27,7 +27,7 @@ The response stored in _dest_ contains: the version of the library as a string. The function writes at most _max_dest_ bytes from the response to _dest_ so -you must take to allocate enough memory to store the results. +you must ensure that _dest_ is large enough. RETURN VALUE ------------ From 72ce9a6192cde7d332e31d1eb71ca96a24771fc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 11 May 2016 16:24:33 +0200 Subject: [PATCH 15/41] Add links to new modbus_*_float_* functions in index --- doc/libmodbus.txt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/doc/libmodbus.txt b/doc/libmodbus.txt index 9035d351a..32ac72909 100644 --- a/doc/libmodbus.txt +++ b/doc/libmodbus.txt @@ -162,10 +162,17 @@ Handling of bits and bytes:: linkmb:modbus_get_byte_from_bits[3] Set or get float numbers:: - linkmb:modbus_get_float[3] - linkmb:modbus_set_float[3] + linkmb:modbus_get_float_abcd[3] + linkmb:modbus_set_float_abcd[3] + linkmb:modbus_get_float_badc[3] + linkmb:modbus_set_float_badc[3] + linkmb:modbus_get_float_cdab[3] + linkmb:modbus_set_float_cdab[3] linkmb:modbus_get_float_dcba[3] linkmb:modbus_set_float_dcba[3] + linkmb:modbus_get_float[3] (deprecated) + linkmb:modbus_set_float[3] (deprecated) + Connection From 35761b836ccf648ca59dc704651f80d7c0b552e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 11 May 2016 16:41:42 +0200 Subject: [PATCH 16/41] Move setting of option inside the relevant conditional group --- src/modbus-tcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modbus-tcp.c b/src/modbus-tcp.c index b6d42b284..15a8f0092 100644 --- a/src/modbus-tcp.c +++ b/src/modbus-tcp.c @@ -225,7 +225,6 @@ static int _modbus_tcp_set_ipv4_options(int s) /* If the OS does not offer SOCK_NONBLOCK, fall back to setting FIONBIO to * make sockets non-blocking */ /* Do not care about the return value, this is optional */ - option = 1; #if !defined(SOCK_NONBLOCK) && defined(FIONBIO) #ifdef OS_WIN32 { @@ -234,6 +233,7 @@ static int _modbus_tcp_set_ipv4_options(int s) ioctlsocket(s, FIONBIO, &loption); } #else + option = 1; ioctl(s, FIONBIO, &option); #endif #endif From 493bce377838c1a455b7c788eef7c265f2af5beb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 11 May 2016 17:18:18 +0200 Subject: [PATCH 17/41] Add ./configure.scan to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 1068a4745..c23170421 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ Makefile.in /build-aux /config.* /configure +/configure.scan /depcomp /install-sh /libtool From 991ec1bebddf9bf3f069812dca481c7b538b082d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Tue, 17 May 2016 22:07:53 +0200 Subject: [PATCH 18/41] Add unit-tests.sh to tarball --- tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 4d469efd7..184d3e6c8 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,4 +1,4 @@ -EXTRA_DIST = README +EXTRA_DIST = README unit-tests.sh noinst_PROGRAMS = \ bandwidth-server-one \ From fd35dab80a46f9add1e735e952760cca2179ef86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Tue, 17 May 2016 22:17:20 +0200 Subject: [PATCH 19/41] Fix small leak (64 bytes in TCP) in unit-test-client --- tests/unit-test-client.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index c790b31e3..a99ba20a5 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -649,11 +649,15 @@ int main(int argc, char *argv[]) printf("* modbus_read_registers at special address: "); ASSERT_TRUE(rc == -1 && errno == EMBXSBUSY, ""); - /** SERVER **/ + /** Run a few tests to challenge the server code **/ if (test_server(ctx, use_backend) == -1) { goto close; } + modbus_close(ctx); + modbus_free(ctx); + ctx = NULL; + /* Test init functions */ printf("\nTEST INVALID INITIALIZATION:\n"); ctx = modbus_new_rtu(NULL, 1, 'A', 0, 0); From c1cda7974fcd0d980dd7af25b07a9f3e76505e0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Tue, 17 May 2016 23:16:13 +0200 Subject: [PATCH 20/41] Github's contributing and issue template files --- CONTRIBUTING.md | 29 +++++++++++++++++++++++++++++ ISSUE_TEMPLATE.md | 13 +++++++++++++ README.md | 25 ++++++------------------- 3 files changed, 48 insertions(+), 19 deletions(-) create mode 100644 CONTRIBUTING.md create mode 100644 ISSUE_TEMPLATE.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..346619325 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,29 @@ +How Do I Submit A Good Bug Report? +---------------------------------- + +Please, don't send direct emails to Stéphane Raimbault unless you want +commercial support. + +Take care to read the documentation at http://libmodbus.org/documentation/. + +- *Be sure it's a bug before creating an issue*, in doubt, post a message on + https://groups.google.com/forum/#!forum/libmodbus or send an email to + libmodbus@googlegroups.com + +- *Use a clear and decriptive title* for the issue to identify + +- *Which version of libmodbus are you using?* you can obtain this information +from your package manager or by running `pkg-config --modversion libmodbus`. +You can provide the sha1 of the commit if you have fetched the code with `git`. + +- *Which operating system are you using?* + +- *Describe the exact steps which reproduce the problem* in as many details as +possible. For example, the software/equipement which runs the Modbus server, how +the clients are connected (TCP, RTU, ASCII) and the source code you are using. + +- *Enable the debug mode*, libmodbus provides a function to display the content +of the Modbus messages and it's very convenient to analyze issues +(http://libmodbus.org/docs/latest/modbus_set_debug.html). + +Good bug reports provide right and quick fixes! diff --git a/ISSUE_TEMPLATE.md b/ISSUE_TEMPLATE.md new file mode 100644 index 000000000..19d08a292 --- /dev/null +++ b/ISSUE_TEMPLATE.md @@ -0,0 +1,13 @@ +### libmodbus version + +### Operating system + +### Description of the Modbus network (server, client, links, etc) + +### Expected behavior + +### Actual behavior + +### Steps to reproduce the behavior (commands or source code) + +### libmodbus output with debug mode enabled diff --git a/README.md b/README.md index 28455eb7a..d41e5f7d9 100644 --- a/README.md +++ b/README.md @@ -59,13 +59,14 @@ automake libtool`. Documentation ------------- +The documentation is available [online](http://libmodbus.org/documentation) or +as manual pages after installation. + The documentation is based on [AsciiDoc](http://www.methods.co.nz/asciidoc/). Only man pages are built by default with `make` command, you can run `make htmldoc` in *docs* directory to generate HTML files. -The documentation is also available [online](http://libmodbus.org/documentation). - Testing ------- @@ -83,21 +84,7 @@ By default, all TCP unit tests will be executed (see --help for options). It's also possible to run the unit tests with `make check`. -Report a Bug ------------- - -Before reporting a bug, take care to read the documentation (RTFM!) and to -provide enough information: - -1. libmodbus version -2. OS/environment/architecture -3. libmodbus backend (TCP, RTU, IPv6) -3. Modbus messages when running in debug mode (`man modbus_set_debug`) - -To report your problem, you can: - -* fill a bug report on the issue tracker . -* or send an email to the libmodbus mailing list [libmodbus@googlegroups.com](https://groups.google.com/forum/#!forum/libmodbus). +To report a bug or to contribute +-------------------------------- -If your prefer live talk when your're looking for help or to offer contribution, -there is also a channel called #libmodbus on Freenode. +See [CONTRIBUTING](CONTRIBUTING.md) document. \ No newline at end of file From 5bef3cf2aa8c94494e476b3ef7fc06e494da0b6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 18 May 2016 00:10:05 +0200 Subject: [PATCH 21/41] Rewrite and rename README as README.md in tests/ --- tests/Makefile.am | 2 +- tests/README | 42 ------------------------------------------ tests/README.md | 27 +++++++++++++++++++++++++++ 3 files changed, 28 insertions(+), 43 deletions(-) delete mode 100644 tests/README create mode 100644 tests/README.md diff --git a/tests/Makefile.am b/tests/Makefile.am index 184d3e6c8..38fa21c09 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,4 +1,4 @@ -EXTRA_DIST = README unit-tests.sh +EXTRA_DIST = README.md unit-tests.sh noinst_PROGRAMS = \ bandwidth-server-one \ diff --git a/tests/README b/tests/README deleted file mode 100644 index 04d192bf4..000000000 --- a/tests/README +++ /dev/null @@ -1,42 +0,0 @@ -License -------- -Test programs of this directory are provided under BSD license (see associated -LICENSE file). - -Compilation ------------ -After installation, you can use pkg-config to compile these tests. -For example, to compile random-test-server run: - -gcc random-test-server.c -o random-test-server `pkg-config --libs --cflags libmodbus` - -random-test-server ------------------ -It's necessary to launch this server before run random-test-client. By -default, it receives and responses to Modbus query on the localhost -and port 1502. - -random-test-client ------------------- -This programm sends many different queries to a large range of -addresses and values to test the communication between the client and -the server. - -unit-test-server -unit-test-client ----------------- -By default, this program sends some queries with the values defined in -unit-test.h and checks the responses. These programs are useful to -test the protocol implementation. - -bandwidth-server-one -bandwidth-server-many-up -bandwidth-client ------------------------ -It returns some very useful informations about the performance of -transfert rate between the server and the client. - -- bandwidth-server-one: it can handles only one connection with a client. -- bandwidth-server-many-up: it opens a connection each time a new client asks - for, but the number of connection is limited. The same server process handles - all the connections. diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 000000000..810dc8c70 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,27 @@ +# License +Test programs of this directory are provided under BSD license (see associated +LICENSE file). + +# Compilation +After installation, you can use pkg-config to compile these tests. +For example, to compile random-test-server run: + +gcc random-test-server.c -o random-test-server `pkg-config --libs --cflags libmodbus` + +- `random-test-server` is necessary to launch a server before running +random-test-client. By default, it receives and replies to Modbus query on the +localhost and port 1502. + +- `random-test-client` sends many different queries to a large range of +addresses and values to test the communication between the client and the +server. + +- `unit-test-server` and `unit-test-client` run a full unit test suite. These +programs are essential to test the Modbus protocol implementation and libmodbus +behavior. + +- `bandwidth-server-one`, `bandwidth-server-many-up` and `bandwidth-client` + return very useful information about the performance of transfert rate between + the server and the client. `bandwidth-server-one` can only handles one + connection at once with a client whereas `bandwidth-server-many-up` opens a + connection for each new clients (with a limit). From 7094e397e11ea5725839317be040b8185a7e1cca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 18 May 2016 15:43:30 +0200 Subject: [PATCH 22/41] Fix CID 69140 - Bad bit shift operation (coverity) in tests --- tests/bandwidth-server-many-up.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/bandwidth-server-many-up.c b/tests/bandwidth-server-many-up.c index 84b59f0b3..0f00f3a2f 100644 --- a/tests/bandwidth-server-many-up.c +++ b/tests/bandwidth-server-many-up.c @@ -62,6 +62,11 @@ int main(void) } server_socket = modbus_tcp_listen(ctx, NB_CONNECTION); + if (server_socket == -1) { + fprintf(stderr, "Unable to listen TCP connection\n"); + modbus_free(ctx); + return -1; + } signal(SIGINT, close_sigint); From b92c709aa03f624b6bd1437a9229d58268fe29d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 18 May 2016 16:16:58 +0200 Subject: [PATCH 23/41] CID 69142 - Unchecked return value in unit-test-server --- tests/unit-test-server.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit-test-server.c b/tests/unit-test-server.c index fe2fc7625..ae18c4bf2 100644 --- a/tests/unit-test-server.c +++ b/tests/unit-test-server.c @@ -167,7 +167,10 @@ int main(int argc, char*argv[]) for (i=0; i < req_length; i++) { printf("(%.2X)", req[i]); usleep(5000); - send(w_s, (const char*)(req + i), 1, MSG_NOSIGNAL); + rc = send(w_s, (const char*)(req + i), 1, MSG_NOSIGNAL); + if (rc == -1) { + break; + } } continue; } From d0fdd8b4c56011f8aa1a6109c0d954f593f5e57d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 18 May 2016 16:30:22 +0200 Subject: [PATCH 24/41] CID 69145 - Argument cannot be negative in unit-test-server --- tests/unit-test-server.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/unit-test-server.c b/tests/unit-test-server.c index ae18c4bf2..1f3ded5f5 100644 --- a/tests/unit-test-server.c +++ b/tests/unit-test-server.c @@ -161,6 +161,10 @@ int main(int argc, char*argv[]) uint8_t req[] = "\x00\x1C\x00\x00\x00\x05\xFF\x03\x02\x00\x00"; int req_length = 11; int w_s = modbus_get_socket(ctx); + if (ws_s == -1) { + fprintf(stderr, "Unable to get a valid socket in special test\n"); + continue; + } /* Copy TID */ req[1] = query[1]; From e5bf0b38b2091a3e433d0014f5629b550e4e7079 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Wed, 18 May 2016 17:48:58 +0200 Subject: [PATCH 25/41] Fix typo in 3053bd0adb --- tests/unit-test-server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit-test-server.c b/tests/unit-test-server.c index 1f3ded5f5..7002b10c6 100644 --- a/tests/unit-test-server.c +++ b/tests/unit-test-server.c @@ -161,7 +161,7 @@ int main(int argc, char*argv[]) uint8_t req[] = "\x00\x1C\x00\x00\x00\x05\xFF\x03\x02\x00\x00"; int req_length = 11; int w_s = modbus_get_socket(ctx); - if (ws_s == -1) { + if (w_s == -1) { fprintf(stderr, "Unable to get a valid socket in special test\n"); continue; } From 5b60cdbde1115093ea3b926f93519c08ea7052b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Fri, 20 May 2016 12:01:13 +0200 Subject: [PATCH 26/41] DRY in modbus_reply by improving response_exception() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thanks to Rüdiger Ranft for the idea. --- src/modbus.c | 216 +++++++++++++++++++-------------------------------- 1 file changed, 78 insertions(+), 138 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index 58aa1a434..0a01424ee 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -658,14 +659,30 @@ static int response_io_status(uint8_t *tab_io_status, /* Build the exception response */ static int response_exception(modbus_t *ctx, sft_t *sft, - int exception_code, uint8_t *rsp) + int exception_code, uint8_t *rsp, + unsigned int to_flush, + const char* template, ...) { int rsp_length; + /* Print debug message */ + if (ctx->debug) { + va_list ap; + + va_start(ap, template); + vfprintf(stderr, template, ap); + va_end(ap); + } + + /* Flush if required */ + if (to_flush) { + _sleep_response_timeout(ctx); + modbus_flush(ctx); + } + + /* Build exception response */ sft->function = sft->function + 0x80; rsp_length = ctx->backend->build_response_basis(sft, rsp); - - /* Positive exception code */ rsp[rsp_length++] = exception_code; return rsp_length; @@ -711,25 +728,17 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, int mapping_address = address - mb_mapping->start_bits; if (nb < 1 || MODBUS_MAX_READ_BITS < nb) { - if (ctx->debug) { - fprintf(stderr, - "Illegal nb of values %d in read_bits (max %d)\n", - nb, MODBUS_MAX_READ_BITS); - } - _sleep_response_timeout(ctx); - modbus_flush(ctx); rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, + "Illegal nb of values %d in read_bits (max %d)\n", + nb, MODBUS_MAX_READ_BITS); } else if (mapping_address < 0 || (mapping_address + nb) > mb_mapping->nb_bits) { - if (ctx->debug) { - fprintf(stderr, "Illegal data address 0x%0X in read_bits\n", - mapping_address < 0 ? address : address + nb); - } rsp_length = response_exception( ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); + MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, + "Illegal data address 0x%0X in read_bits\n", + mapping_address < 0 ? address : address + nb); } else { rsp_length = ctx->backend->build_response_basis(&sft, rsp); rsp[rsp_length++] = (nb / 8) + ((nb % 8) ? 1 : 0); @@ -746,25 +755,17 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, int mapping_address = address - mb_mapping->start_input_bits; if (nb < 1 || MODBUS_MAX_READ_BITS < nb) { - if (ctx->debug) { - fprintf(stderr, - "Illegal nb of values %d in read_input_bits (max %d)\n", - nb, MODBUS_MAX_READ_BITS); - } - _sleep_response_timeout(ctx); - modbus_flush(ctx); rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, + "Illegal nb of values %d in read_input_bits (max %d)\n", + nb, MODBUS_MAX_READ_BITS); } else if (mapping_address < 0 || (mapping_address + nb) > mb_mapping->nb_input_bits) { - if (ctx->debug) { - fprintf(stderr, "Illegal data address 0x%0X in read_input_bits\n", - mapping_address < 0 ? address : address + nb); - } rsp_length = response_exception( ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); + MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, + "Illegal data address 0x%0X in read_input_bits\n", + mapping_address < 0 ? address : address + nb); } else { rsp_length = ctx->backend->build_response_basis(&sft, rsp); rsp[rsp_length++] = (nb / 8) + ((nb % 8) ? 1 : 0); @@ -779,25 +780,16 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, int mapping_address = address - mb_mapping->start_registers; if (nb < 1 || MODBUS_MAX_READ_REGISTERS < nb) { - if (ctx->debug) { - fprintf(stderr, - "Illegal nb of values %d in read_holding_registers (max %d)\n", - nb, MODBUS_MAX_READ_REGISTERS); - } - _sleep_response_timeout(ctx); - modbus_flush(ctx); rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, + "Illegal nb of values %d in read_holding_registers (max %d)\n", + nb, MODBUS_MAX_READ_REGISTERS); } else if (mapping_address < 0 || (mapping_address + nb) > mb_mapping->nb_registers) { - if (ctx->debug) { - fprintf(stderr, "Illegal data address 0x%0X in read_registers\n", - mapping_address < 0 ? address : address + nb); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, + "Illegal data address 0x%0X in read_registers\n", + mapping_address < 0 ? address : address + nb); } else { int i; @@ -817,25 +809,16 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, int mapping_address = address - mb_mapping->start_input_registers; if (nb < 1 || MODBUS_MAX_READ_REGISTERS < nb) { - if (ctx->debug) { - fprintf(stderr, - "Illegal number of values %d in read_input_registers (max %d)\n", - nb, MODBUS_MAX_READ_REGISTERS); - } - _sleep_response_timeout(ctx); - modbus_flush(ctx); rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, + "Illegal number of values %d in read_input_registers (max %d)\n", + nb, MODBUS_MAX_READ_REGISTERS); } else if (mapping_address < 0 || (mapping_address + nb) > mb_mapping->nb_input_registers) { - if (ctx->debug) { - fprintf(stderr, "Illegal data address 0x%0X in read_input_registers\n", - mapping_address < 0 ? address : address + nb); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, + "Illegal data address 0x%0X in read_input_registers\n", + mapping_address < 0 ? address : address + nb); } else { int i; @@ -852,14 +835,10 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, int mapping_address = address - mb_mapping->start_bits; if (mapping_address < 0 || mapping_address >= mb_mapping->nb_bits) { - if (ctx->debug) { - fprintf(stderr, - "Illegal data address 0x%0X in write_bit\n", - address); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, + "Illegal data address 0x%0X in write_bit\n", + address); } else { int data = (req[offset + 3] << 8) + req[offset + 4]; @@ -868,14 +847,11 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, memcpy(rsp, req, req_length); rsp_length = req_length; } else { - if (ctx->debug) { - fprintf(stderr, - "Illegal data value 0x%0X in write_bit request at address %0X\n", - data, address); - } rsp_length = response_exception( ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); + MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, FALSE, + "Illegal data value 0x%0X in write_bit request at address %0X\n", + data, address); } } } @@ -884,13 +860,11 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, int mapping_address = address - mb_mapping->start_registers; if (mapping_address < 0 || mapping_address >= mb_mapping->nb_registers) { - if (ctx->debug) { - fprintf(stderr, "Illegal data address 0x%0X in write_register\n", - address); - } rsp_length = response_exception( ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); + MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, + "Illegal data address 0x%0X in write_register\n", + address); } else { int data = (req[offset + 3] << 8) + req[offset + 4]; @@ -905,28 +879,20 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, int mapping_address = address - mb_mapping->start_bits; if (nb < 1 || MODBUS_MAX_WRITE_BITS < nb) { - if (ctx->debug) { - fprintf(stderr, - "Illegal number of values %d in write_bits (max %d)\n", - nb, MODBUS_MAX_WRITE_BITS); - } /* May be the indication has been truncated on reading because of * invalid address (eg. nb is 0 but the request contains values to * write) so it's necessary to flush. */ - _sleep_response_timeout(ctx); - modbus_flush(ctx); rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, + "Illegal number of values %d in write_bits (max %d)\n", + nb, MODBUS_MAX_WRITE_BITS); } else if (mapping_address < 0 || (mapping_address + nb) > mb_mapping->nb_bits) { - if (ctx->debug) { - fprintf(stderr, "Illegal data address 0x%0X in write_bits\n", - mapping_address < 0 ? address : address + nb); - } rsp_length = response_exception( ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); + MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, + "Illegal data address 0x%0X in write_bits\n", + mapping_address < 0 ? address : address + nb); } else { /* 6 = byte count */ modbus_set_bits_from_bytes(mb_mapping->tab_bits, mapping_address, nb, @@ -944,28 +910,16 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, int mapping_address = address - mb_mapping->start_registers; if (nb < 1 || MODBUS_MAX_WRITE_REGISTERS < nb) { - if (ctx->debug) { - fprintf(stderr, - "Illegal number of values %d in write_registers (max %d)\n", - nb, MODBUS_MAX_WRITE_REGISTERS); - } - /* May be the indication has been truncated on reading because of - * invalid address (eg. nb is 0 but the request contains values to - * write) so it's necessary to flush. */ - _sleep_response_timeout(ctx); - modbus_flush(ctx); rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, + "Illegal number of values %d in write_registers (max %d)\n", + nb, MODBUS_MAX_WRITE_REGISTERS); } else if (mapping_address < 0 || (mapping_address + nb) > mb_mapping->nb_registers) { - if (ctx->debug) { - fprintf(stderr, "Illegal data address 0x%0X in write_registers\n", - mapping_address < 0 ? address : address + nb); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, + "Illegal data address 0x%0X in write_registers\n", + mapping_address < 0 ? address : address + nb); } else { int i, j; for (i = mapping_address, j = 6; i < mapping_address + nb; i++, j += 2) { @@ -1009,13 +963,10 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, int mapping_address = address - mb_mapping->start_registers; if (mapping_address < 0 || mapping_address >= mb_mapping->nb_registers) { - if (ctx->debug) { - fprintf(stderr, "Illegal data address 0x%0X in write_register\n", - address); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, + "Illegal data address 0x%0X in write_register\n", + address); } else { uint16_t data = mb_mapping->tab_registers[mapping_address]; uint16_t and = (req[offset + 3] << 8) + req[offset + 4]; @@ -1039,29 +990,19 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, if (nb_write < 1 || MODBUS_MAX_WR_WRITE_REGISTERS < nb_write || nb < 1 || MODBUS_MAX_WR_READ_REGISTERS < nb || nb_write_bytes != nb_write * 2) { - if (ctx->debug) { - fprintf(stderr, - "Illegal nb of values (W%d, R%d) in write_and_read_registers (max W%d, R%d)\n", - nb_write, nb, - MODBUS_MAX_WR_WRITE_REGISTERS, MODBUS_MAX_WR_READ_REGISTERS); - } - _sleep_response_timeout(ctx); - modbus_flush(ctx); rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, + "Illegal nb of values (W%d, R%d) in write_and_read_registers (max W%d, R%d)\n", + nb_write, nb, MODBUS_MAX_WR_WRITE_REGISTERS, MODBUS_MAX_WR_READ_REGISTERS); } else if (mapping_address < 0 || (mapping_address + nb) > mb_mapping->nb_registers || mapping_address < 0 || (mapping_address_write + nb_write) > mb_mapping->nb_registers) { - if (ctx->debug) { - fprintf(stderr, - "Illegal data read address 0x%0X or write address 0x%0X write_and_read_registers\n", - mapping_address < 0 ? address : address + nb, - mapping_address_write < 0 ? address_write : address_write + nb_write); - } - rsp_length = response_exception(ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp); + rsp_length = response_exception( + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, + "Illegal data read address 0x%0X or write address 0x%0X write_and_read_registers\n", + mapping_address < 0 ? address : address + nb, + mapping_address_write < 0 ? address_write : address_write + nb_write); } else { int i, j; rsp_length = ctx->backend->build_response_basis(&sft, rsp); @@ -1085,9 +1026,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, break; default: - rsp_length = response_exception(ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_FUNCTION, - rsp); + rsp_length = response_exception( + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, FALSE, ""); break; } From e1c86dd85446093097d87e35adf28c1b20f139c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Fri, 20 May 2016 12:32:13 +0200 Subject: [PATCH 27/41] Add debug message on unknown function and new unit test --- src/modbus.c | 3 ++- tests/unit-test-client.c | 28 ++++++++++++++++++---------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index 0a01424ee..d863775f0 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1027,7 +1027,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, default: rsp_length = response_exception( - ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, FALSE, ""); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, FALSE, + "Unknown Modbus function code: 0x%0X\n", function); break; } diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index a99ba20a5..168af61fa 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -457,7 +457,7 @@ int main(int argc, char *argv[]) uint8_t rsp[MODBUS_RTU_MAX_ADU_LENGTH]; /* No response in RTU mode */ - printf("1-A/3 No response from slave %d: ", INVALID_SERVER_ID); + printf("1-A/4 No response from slave %d: ", INVALID_SERVER_ID); ASSERT_TRUE(rc == -1 && errno == ETIMEDOUT, ""); /* The slave raises a timeout on a confirmation to ignore because if an @@ -472,7 +472,7 @@ int main(int argc, char *argv[]) modbus_send_raw_request(ctx, raw_rep, RAW_REP_LENGTH * sizeof(uint8_t)); rc = modbus_receive_confirmation(ctx, rsp); - printf("1-B/3 No response from slave %d on indication/confirmation messages: ", + printf("1-B/4 No response from slave %d on indication/confirmation messages: ", INVALID_SERVER_ID); ASSERT_TRUE(rc == -1 && errno == ETIMEDOUT, ""); @@ -480,12 +480,12 @@ int main(int argc, char *argv[]) modbus_send_raw_request(ctx, raw_invalid_req, RAW_REQ_LENGTH * sizeof(uint8_t)); rc = modbus_receive_confirmation(ctx, rsp); - printf("1-C/3 No response from slave %d with invalid request: ", + printf("1-C/4 No response from slave %d with invalid request: ", INVALID_SERVER_ID); ASSERT_TRUE(rc == -1 && errno == ETIMEDOUT, ""); } else { /* Response in TCP mode */ - printf("1/3 Response from slave %d: ", INVALID_SERVER_ID); + printf("1/4 Response from slave %d: ", INVALID_SERVER_ID); ASSERT_TRUE(rc == UT_REGISTERS_NB, ""); } @@ -494,17 +494,25 @@ int main(int argc, char *argv[]) rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS, UT_REGISTERS_NB, tab_rp_registers); - printf("2/3 No reply after a broadcast query: "); + printf("2/4 No reply after a broadcast query: "); ASSERT_TRUE(rc == -1 && errno == ETIMEDOUT, ""); /* Restore slave */ - if (use_backend == RTU) { - modbus_set_slave(ctx, SERVER_ID); - } else { - modbus_set_slave(ctx, MODBUS_TCP_SLAVE); + modbus_set_slave(ctx, use_backend == RTU ? SERVER_ID : MODBUS_TCP_SLAVE); + + { + const int RAW_REQ_LENGTH = 6; + uint8_t raw_req[] = { use_backend == RTU ? SERVER_ID : MODBUS_TCP_SLAVE, 0x42, 0x00, 0x00, 0x00, 0x00 }; + uint8_t rsp[MODBUS_MAX_ADU_LENGTH]; + + rc = modbus_send_raw_request(ctx, raw_req, RAW_REQ_LENGTH * sizeof(uint8_t)); + ASSERT_TRUE(rc != -1, "Unable to send raw request with invalid function code"); + rc = modbus_receive_confirmation(ctx, rsp); + printf("3/4 Raise an exception on unknown function code: "); + ASSERT_TRUE(rc == -1, ""); } - printf("3/3 Response with an invalid TID or slave: "); + printf("4/4 Response with an invalid TID or slave: "); rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS_INVALID_TID_OR_SLAVE, 1, tab_rp_registers); ASSERT_TRUE(rc == -1, ""); From 5b03897b7a65a7ecec48828e64f36b43e0236a87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Fri, 20 May 2016 12:36:49 +0200 Subject: [PATCH 28/41] Fix handling of invalid function code (closes #315) Thanks to paperwork --- src/modbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modbus.c b/src/modbus.c index d863775f0..265058358 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1027,7 +1027,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, default: rsp_length = response_exception( - ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, FALSE, + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, TRUE, "Unknown Modbus function code: 0x%0X\n", function); break; } From dfbc6bf77d5e3162e097ea102633b713bf6b6617 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Fri, 20 May 2016 12:52:00 +0200 Subject: [PATCH 29/41] Fix wrong function name in debug message --- src/modbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modbus.c b/src/modbus.c index 265058358..bd96ec7b5 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -782,7 +782,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, if (nb < 1 || MODBUS_MAX_READ_REGISTERS < nb) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, - "Illegal nb of values %d in read_holding_registers (max %d)\n", + "Illegal nb of values %d in read_registers (max %d)\n", nb, MODBUS_MAX_READ_REGISTERS); } else if (mapping_address < 0 || (mapping_address + nb) > mb_mapping->nb_registers) { From fcd8830af40b396f337f2db6e4c8ee8e3ea2888c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Fri, 20 May 2016 12:53:27 +0200 Subject: [PATCH 30/41] Rename raw_rep to raw_rsp in unit-test-client --- tests/unit-test-client.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index 168af61fa..727cc192b 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -452,8 +452,8 @@ int main(int argc, char *argv[]) uint8_t raw_req[] = { INVALID_SERVER_ID, 0x03, 0x00, 0x01, 0x01, 0x01 }; /* Too many points */ uint8_t raw_invalid_req[] = { INVALID_SERVER_ID, 0x03, 0x00, 0x01, 0xFF, 0xFF }; - const int RAW_REP_LENGTH = 7; - uint8_t raw_rep[] = { INVALID_SERVER_ID, 0x03, 0x04, 0, 0, 0, 0 }; + const int RAW_RSP_LENGTH = 7; + uint8_t raw_rsp[] = { INVALID_SERVER_ID, 0x03, 0x04, 0, 0, 0, 0 }; uint8_t rsp[MODBUS_RTU_MAX_ADU_LENGTH]; /* No response in RTU mode */ @@ -469,7 +469,7 @@ int main(int argc, char *argv[]) * slave will see the indication message then the confirmation, and it must * ignore both. */ modbus_send_raw_request(ctx, raw_req, RAW_REQ_LENGTH * sizeof(uint8_t)); - modbus_send_raw_request(ctx, raw_rep, RAW_REP_LENGTH * sizeof(uint8_t)); + modbus_send_raw_request(ctx, raw_rsp, RAW_RSP_LENGTH * sizeof(uint8_t)); rc = modbus_receive_confirmation(ctx, rsp); printf("1-B/4 No response from slave %d on indication/confirmation messages: ", From e769541feb1d64b52dcfd8f3dfd6e590821f82a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Fri, 20 May 2016 14:17:37 +0200 Subject: [PATCH 31/41] Another round of DRY in modbus_reply() --- src/modbus.c | 105 +++++++++++++++------------------------------------ 1 file changed, 30 insertions(+), 75 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index bd96ec7b5..6fd9e31d0 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -721,112 +721,67 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, /* Data are flushed on illegal number of values errors. */ switch (function) { - case MODBUS_FC_READ_COILS: { + case MODBUS_FC_READ_COILS: + case MODBUS_FC_READ_DISCRETE_INPUTS: { + unsigned int is_input = (function == MODBUS_FC_READ_DISCRETE_INPUTS); + int start_bits = is_input ? mb_mapping->start_input_bits : mb_mapping->start_bits; + int nb_bits = is_input ? mb_mapping->nb_input_bits : mb_mapping->nb_bits; + uint8_t *tab_bits = is_input ? mb_mapping->tab_input_bits : mb_mapping->tab_bits; + const char * const name = is_input ? "read_input_bits" : "read_bits"; int nb = (req[offset + 3] << 8) + req[offset + 4]; /* The mapping can be shifted to reduce memory consumption and it doesn't always start at address zero. */ - int mapping_address = address - mb_mapping->start_bits; + int mapping_address = address - start_bits; if (nb < 1 || MODBUS_MAX_READ_BITS < nb) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, - "Illegal nb of values %d in read_bits (max %d)\n", - nb, MODBUS_MAX_READ_BITS); - } else if (mapping_address < 0 || - (mapping_address + nb) > mb_mapping->nb_bits) { + "Illegal nb of values %d in %s (max %d)\n", + nb, name, MODBUS_MAX_READ_BITS); + } else if (mapping_address < 0 || (mapping_address + nb) > nb_bits) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, - "Illegal data address 0x%0X in read_bits\n", - mapping_address < 0 ? address : address + nb); + "Illegal data address 0x%0X in %s\n", + mapping_address < 0 ? address : address + nb, name); } else { rsp_length = ctx->backend->build_response_basis(&sft, rsp); rsp[rsp_length++] = (nb / 8) + ((nb % 8) ? 1 : 0); - rsp_length = response_io_status(mb_mapping->tab_bits, - mapping_address, nb, + rsp_length = response_io_status(tab_bits, mapping_address, nb, rsp, rsp_length); } } break; - case MODBUS_FC_READ_DISCRETE_INPUTS: { - /* Similar to coil status (but too many arguments to use a - * function) */ - int nb = (req[offset + 3] << 8) + req[offset + 4]; - int mapping_address = address - mb_mapping->start_input_bits; - - if (nb < 1 || MODBUS_MAX_READ_BITS < nb) { - rsp_length = response_exception( - ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, - "Illegal nb of values %d in read_input_bits (max %d)\n", - nb, MODBUS_MAX_READ_BITS); - } else if (mapping_address < 0 || - (mapping_address + nb) > mb_mapping->nb_input_bits) { - rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, - "Illegal data address 0x%0X in read_input_bits\n", - mapping_address < 0 ? address : address + nb); - } else { - rsp_length = ctx->backend->build_response_basis(&sft, rsp); - rsp[rsp_length++] = (nb / 8) + ((nb % 8) ? 1 : 0); - rsp_length = response_io_status(mb_mapping->tab_input_bits, - mapping_address, nb, - rsp, rsp_length); - } - } - break; - case MODBUS_FC_READ_HOLDING_REGISTERS: { - int nb = (req[offset + 3] << 8) + req[offset + 4]; - int mapping_address = address - mb_mapping->start_registers; - - if (nb < 1 || MODBUS_MAX_READ_REGISTERS < nb) { - rsp_length = response_exception( - ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, - "Illegal nb of values %d in read_registers (max %d)\n", - nb, MODBUS_MAX_READ_REGISTERS); - } else if (mapping_address < 0 || - (mapping_address + nb) > mb_mapping->nb_registers) { - rsp_length = response_exception( - ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, - "Illegal data address 0x%0X in read_registers\n", - mapping_address < 0 ? address : address + nb); - } else { - int i; - - rsp_length = ctx->backend->build_response_basis(&sft, rsp); - rsp[rsp_length++] = nb << 1; - for (i = mapping_address; i < mapping_address + nb; i++) { - rsp[rsp_length++] = mb_mapping->tab_registers[i] >> 8; - rsp[rsp_length++] = mb_mapping->tab_registers[i] & 0xFF; - } - } - } - break; + case MODBUS_FC_READ_HOLDING_REGISTERS: case MODBUS_FC_READ_INPUT_REGISTERS: { - /* Similar to holding registers (but too many arguments to use a - * function) */ + unsigned int is_input = (function == MODBUS_FC_READ_INPUT_REGISTERS); + int start_registers = is_input ? mb_mapping->start_input_registers : mb_mapping->start_registers; + int nb_registers = is_input ? mb_mapping->nb_input_registers : mb_mapping->nb_registers; + uint16_t *tab_registers = is_input ? mb_mapping->tab_input_registers : mb_mapping->tab_registers; + const char * const name = is_input ? "read_input_registers" : "read_registers"; int nb = (req[offset + 3] << 8) + req[offset + 4]; - int mapping_address = address - mb_mapping->start_input_registers; + /* The mapping can be shifted to reduce memory consumption and it + doesn't always start at address zero. */ + int mapping_address = address - start_registers; if (nb < 1 || MODBUS_MAX_READ_REGISTERS < nb) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, - "Illegal number of values %d in read_input_registers (max %d)\n", - nb, MODBUS_MAX_READ_REGISTERS); - } else if (mapping_address < 0 || - (mapping_address + nb) > mb_mapping->nb_input_registers) { + "Illegal nb of values %d in %s (max %d)\n", + nb, name, MODBUS_MAX_READ_REGISTERS); + } else if (mapping_address < 0 || (mapping_address + nb) > nb_registers) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, - "Illegal data address 0x%0X in read_input_registers\n", - mapping_address < 0 ? address : address + nb); + "Illegal data address 0x%0X in %s\n", + mapping_address < 0 ? address : address + nb, name); } else { int i; rsp_length = ctx->backend->build_response_basis(&sft, rsp); rsp[rsp_length++] = nb << 1; for (i = mapping_address; i < mapping_address + nb; i++) { - rsp[rsp_length++] = mb_mapping->tab_input_registers[i] >> 8; - rsp[rsp_length++] = mb_mapping->tab_input_registers[i] & 0xFF; + rsp[rsp_length++] = tab_registers[i] >> 8; + rsp[rsp_length++] = tab_registers[i] & 0xFF; } } } From 786644a6250b2e008fac1373debb10274415b131 Mon Sep 17 00:00:00 2001 From: StalderT Date: Fri, 20 May 2016 11:33:47 +0200 Subject: [PATCH 32/41] Update configure.ac https://github.com/stephane/libmodbus/issues/248 --- configure.ac | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index e8df6a74b..425f105f3 100644 --- a/configure.ac +++ b/configure.ac @@ -57,6 +57,7 @@ AC_CANONICAL_HOST # OS check os_win32="false" +os_cygwin="false" os_qnx="false" case "${host_os}" in *mingw32*) @@ -64,6 +65,9 @@ case "${host_os}" in ;; *nto-qnx*) os_qnx="true" + ;; + *cygwin*) + os_cygwin="true" ;; esac AM_CONDITIONAL(OS_WIN32, test "$os_win32" = "true") @@ -120,11 +124,13 @@ AC_TYPE_UINT16_T AC_TYPE_UINT32_T AC_TYPE_UINT8_T -# Required for getaddrinfo (TCP PI - IPv6) -AC_CHECK_HEADERS([winsock2.h], HAVE_WINSOCK2_H=yes) -if test "x$HAVE_WINSOCK2_H" = "xyes"; then - LIBS="$LIBS -lws2_32" - AC_SUBST(LIBS) +if test "$os_cygwin" = "false"; then + # Required for getaddrinfo (TCP IP - IPv6) + AC_CHECK_HEADERS([winsock2.h], HAVE_WINSOCK2_H=yes) + if test "x$HAVE_WINSOCK2_H" = "xyes"; then + LIBS="$LIBS -lws2_32" + AC_SUBST(LIBS) + fi fi # Check for RS485 support (Linux kernel version 2.6.28+) From f280ed922ea8724e089110560cce7d98ba6dcb14 Mon Sep 17 00:00:00 2001 From: StalderT Date: Fri, 20 May 2016 11:38:30 +0200 Subject: [PATCH 33/41] Update modbus-data.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Making all in src CC modbus.lo CC modbus-data.lo modbus-data.c:51:5: attention : #warning "Fallback on C functions for bswap_16" [-Wcpp] # warning "Fallback on C functions for bswap_16" ^ modbus-data.c:52:24: erreur: redefinition of ‘bswap_16’ static inline uint16_t bswap_16(uint16_t x) --- src/modbus-data.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/modbus-data.c b/src/modbus-data.c index bbcbab870..f889e2d17 100644 --- a/src/modbus-data.c +++ b/src/modbus-data.c @@ -47,6 +47,7 @@ # define bswap_16 _byteswap_ushort #endif +#if !defined(__CYGWIN__) #if !defined(bswap_16) # warning "Fallback on C functions for bswap_16" static inline uint16_t bswap_16(uint16_t x) @@ -54,6 +55,7 @@ static inline uint16_t bswap_16(uint16_t x) return (x >> 8) | (x << 8); } #endif +#endif #if !defined(bswap_32) # warning "Fallback on C functions for bswap_32" From bd999ad859f4be63b740d41568183a8ba54750c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Fri, 20 May 2016 14:25:52 +0200 Subject: [PATCH 34/41] Minor coding conventions on defines --- src/modbus-data.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/modbus-data.c b/src/modbus-data.c index f889e2d17..dc7e5f37b 100644 --- a/src/modbus-data.c +++ b/src/modbus-data.c @@ -5,18 +5,20 @@ */ #include + #ifndef _MSC_VER -#include +# include #else -#include "stdint.h" +# include "stdint.h" #endif + #include #include #if defined(_WIN32) -# include +# include #else -# include +# include #endif #include @@ -28,10 +30,10 @@ #endif #if defined(__APPLE__) - #include - #define bswap_16 OSSwapInt16 - #define bswap_32 OSSwapInt32 - #define bswap_64 OSSwapInt64 +# include +# define bswap_16 OSSwapInt16 +# define bswap_32 OSSwapInt32 +# define bswap_64 OSSwapInt64 #endif #if defined(__GNUC__) @@ -42,14 +44,15 @@ # define bswap_32 __builtin_bswap32 # endif #endif + #if defined(_MSC_VER) && (_MSC_VER >= 1400) -# define bswap_32 _byteswap_ulong -# define bswap_16 _byteswap_ushort +# define bswap_32 _byteswap_ulong +# define bswap_16 _byteswap_ushort #endif #if !defined(__CYGWIN__) #if !defined(bswap_16) -# warning "Fallback on C functions for bswap_16" +# warning "Fallback on C functions for bswap_16" static inline uint16_t bswap_16(uint16_t x) { return (x >> 8) | (x << 8); @@ -58,7 +61,7 @@ static inline uint16_t bswap_16(uint16_t x) #endif #if !defined(bswap_32) -# warning "Fallback on C functions for bswap_32" +# warning "Fallback on C functions for bswap_32" static inline uint32_t bswap_32(uint32_t x) { return (bswap_16(x & 0xffff) << 16) | (bswap_16(x >> 16)); From 48174a8daf6d5ab5fdbc86660c78b29ab4d13592 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Fri, 20 May 2016 14:30:06 +0200 Subject: [PATCH 35/41] Improve ifdef around bswap_16 for __CYGWIN__ --- src/modbus-data.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/modbus-data.c b/src/modbus-data.c index dc7e5f37b..902b8c69c 100644 --- a/src/modbus-data.c +++ b/src/modbus-data.c @@ -50,15 +50,13 @@ # define bswap_16 _byteswap_ushort #endif -#if !defined(__CYGWIN__) -#if !defined(bswap_16) +#if !defined(__CYGWIN__) && !defined(bswap_16) # warning "Fallback on C functions for bswap_16" static inline uint16_t bswap_16(uint16_t x) { return (x >> 8) | (x << 8); } #endif -#endif #if !defined(bswap_32) # warning "Fallback on C functions for bswap_32" From 4d5dfb0d7a4a59641ed66ffea29af9942eee6776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Fri, 20 May 2016 14:51:02 +0200 Subject: [PATCH 36/41] C_PROG_RANLIB is rendered obsolete by LT_INIT --- configure.ac | 1 - 1 file changed, 1 deletion(-) diff --git a/configure.ac b/configure.ac index 425f105f3..648a5e8f6 100644 --- a/configure.ac +++ b/configure.ac @@ -114,7 +114,6 @@ AC_C_INLINE # libtool AC_PROG_CXX -AC_PROG_RANLIB # Various types AC_TYPE_INT64_T From 474217c8c2a214b1ea84f8e70e57d54be134dfbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Raimbault?= Date: Fri, 20 May 2016 15:26:50 +0200 Subject: [PATCH 37/41] Rewrite new unit test for invalid function code --- tests/unit-test-client.c | 75 ++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index 727cc192b..3e315f48e 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -13,6 +13,8 @@ #include "unit-test.h" +const int EXCEPTION_RC = 2; + enum { TCP, TCP_PI, @@ -457,7 +459,7 @@ int main(int argc, char *argv[]) uint8_t rsp[MODBUS_RTU_MAX_ADU_LENGTH]; /* No response in RTU mode */ - printf("1-A/4 No response from slave %d: ", INVALID_SERVER_ID); + printf("1-A/3 No response from slave %d: ", INVALID_SERVER_ID); ASSERT_TRUE(rc == -1 && errno == ETIMEDOUT, ""); /* The slave raises a timeout on a confirmation to ignore because if an @@ -472,7 +474,7 @@ int main(int argc, char *argv[]) modbus_send_raw_request(ctx, raw_rsp, RAW_RSP_LENGTH * sizeof(uint8_t)); rc = modbus_receive_confirmation(ctx, rsp); - printf("1-B/4 No response from slave %d on indication/confirmation messages: ", + printf("1-B/3 No response from slave %d on indication/confirmation messages: ", INVALID_SERVER_ID); ASSERT_TRUE(rc == -1 && errno == ETIMEDOUT, ""); @@ -480,12 +482,12 @@ int main(int argc, char *argv[]) modbus_send_raw_request(ctx, raw_invalid_req, RAW_REQ_LENGTH * sizeof(uint8_t)); rc = modbus_receive_confirmation(ctx, rsp); - printf("1-C/4 No response from slave %d with invalid request: ", + printf("1-C/3 No response from slave %d with invalid request: ", INVALID_SERVER_ID); ASSERT_TRUE(rc == -1 && errno == ETIMEDOUT, ""); } else { /* Response in TCP mode */ - printf("1/4 Response from slave %d: ", INVALID_SERVER_ID); + printf("1/3 Response from slave %d: ", INVALID_SERVER_ID); ASSERT_TRUE(rc == UT_REGISTERS_NB, ""); } @@ -494,25 +496,13 @@ int main(int argc, char *argv[]) rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS, UT_REGISTERS_NB, tab_rp_registers); - printf("2/4 No reply after a broadcast query: "); + printf("2/3 No reply after a broadcast query: "); ASSERT_TRUE(rc == -1 && errno == ETIMEDOUT, ""); /* Restore slave */ modbus_set_slave(ctx, use_backend == RTU ? SERVER_ID : MODBUS_TCP_SLAVE); - { - const int RAW_REQ_LENGTH = 6; - uint8_t raw_req[] = { use_backend == RTU ? SERVER_ID : MODBUS_TCP_SLAVE, 0x42, 0x00, 0x00, 0x00, 0x00 }; - uint8_t rsp[MODBUS_MAX_ADU_LENGTH]; - - rc = modbus_send_raw_request(ctx, raw_req, RAW_REQ_LENGTH * sizeof(uint8_t)); - ASSERT_TRUE(rc != -1, "Unable to send raw request with invalid function code"); - rc = modbus_receive_confirmation(ctx, rsp); - printf("3/4 Raise an exception on unknown function code: "); - ASSERT_TRUE(rc == -1, ""); - } - - printf("4/4 Response with an invalid TID or slave: "); + printf("3/3 Response with an invalid TID or slave: "); rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS_INVALID_TID_OR_SLAVE, 1, tab_rp_registers); ASSERT_TRUE(rc == -1, ""); @@ -700,9 +690,9 @@ int test_server(modbus_t *ctx, int use_backend) int i; /* Read requests */ const int READ_RAW_REQ_LEN = 6; + const int slave = (use_backend == RTU) ? SERVER_ID : MODBUS_TCP_SLAVE; uint8_t read_raw_req[] = { - /* slave */ - (use_backend == RTU) ? SERVER_ID : 0xFF, + slave, /* function, address, 5 values */ MODBUS_FC_READ_HOLDING_REGISTERS, UT_REGISTERS_ADDRESS >> 8, UT_REGISTERS_ADDRESS & 0xFF, @@ -711,8 +701,7 @@ int test_server(modbus_t *ctx, int use_backend) /* Write and read registers request */ const int RW_RAW_REQ_LEN = 13; uint8_t rw_raw_req[] = { - /* slave */ - (use_backend == RTU) ? SERVER_ID : 0xFF, + slave, /* function, addr to read, nb to read */ MODBUS_FC_WRITE_AND_READ_REGISTERS, /* Read */ @@ -729,8 +718,7 @@ int test_server(modbus_t *ctx, int use_backend) }; const int WRITE_RAW_REQ_LEN = 13; uint8_t write_raw_req[] = { - /* slave */ - (use_backend == RTU) ? SERVER_ID : 0xFF, + slave, /* function will be set in the loop */ MODBUS_FC_WRITE_MULTIPLE_REGISTERS, /* Address */ @@ -740,6 +728,12 @@ int test_server(modbus_t *ctx, int use_backend) /* Dummy data to write */ 0x02, 0x2B, 0x00, 0x01, 0x00, 0x64 }; + const int INVALID_FC = 0x42; + const int INVALID_FC_REQ_LEN = 6; + uint8_t invalid_fc_raw_req[] = { + slave, 0x42, 0x00, 0x00, 0x00, 0x00 + }; + int req_length; uint8_t rsp[MODBUS_TCP_MAX_ADU_LENGTH]; int tab_read_function[] = { @@ -767,6 +761,17 @@ int test_server(modbus_t *ctx, int use_backend) printf("\nTEST RAW REQUESTS:\n"); + uint32_t old_response_to_sec; + uint32_t old_response_to_usec; + + /* This requests can generate flushes server side so we need a higher + * response timeout than the server. The server uses the defined response + * timeout to sleep before flushing. + * The old timeouts are restored at the end. + */ + modbus_get_response_timeout(ctx, &old_response_to_sec, &old_response_to_usec); + modbus_set_response_timeout(ctx, 0, 600000); + req_length = modbus_send_raw_request(ctx, read_raw_req, READ_RAW_REQ_LEN); printf("* modbus_send_raw_request: "); ASSERT_TRUE(req_length == (backend_length + 5), "FAILED (%d)\n", req_length); @@ -810,8 +815,17 @@ int test_server(modbus_t *ctx, int use_backend) if (rc == -1) goto close; + /* Test invalid function code */ + modbus_send_raw_request(ctx, invalid_fc_raw_req, INVALID_FC_REQ_LEN * sizeof(uint8_t)); + rc = modbus_receive_confirmation(ctx, rsp); + printf("Return an exception on unknown function code: "); + ASSERT_TRUE(rc == (backend_length + EXCEPTION_RC) && + rsp[backend_offset] == (0x80 + INVALID_FC), "") + + modbus_set_response_timeout(ctx, old_response_to_sec, old_response_to_usec); return 0; close: + modbus_set_response_timeout(ctx, old_response_to_sec, old_response_to_usec); return -1; } @@ -821,19 +835,8 @@ int send_crafted_request(modbus_t *ctx, int function, uint16_t max_value, uint16_t bytes, int backend_length, int backend_offset) { - const int EXCEPTION_RC = 2; uint8_t rsp[MODBUS_TCP_MAX_ADU_LENGTH]; int j; - uint32_t old_response_to_sec; - uint32_t old_response_to_usec; - - /* This requests can generate flushes server side so we need a higher - * response timeout than the server. The server uses the defined response - * timeout to sleep before flushing. - * The old timeouts are restored at the end. - */ - modbus_get_response_timeout(ctx, &old_response_to_sec, &old_response_to_usec); - modbus_set_response_timeout(ctx, 0, 600000); for (j=0; j<2; j++) { int rc; @@ -869,9 +872,7 @@ int send_crafted_request(modbus_t *ctx, int function, rsp[backend_offset] == (0x80 + function) && rsp[backend_offset + 1] == MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, ""); } - modbus_set_response_timeout(ctx, old_response_to_sec, old_response_to_usec); return 0; close: - modbus_set_response_timeout(ctx, old_response_to_sec, old_response_to_usec); return -1; } From 2870019ecaf8a3a7c9712ca1c62aa199dad7d1e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frode=20Tenneb=C3=B8?= Date: Sat, 21 May 2016 09:56:29 +0200 Subject: [PATCH 38/41] Update according to API changes in response_exception() in commit 73a88a747c473a7427ef70f0351ff6f5370c3495. --- src/modbus.c | 119 +++++++++++++++++++-------------------------------- 1 file changed, 44 insertions(+), 75 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index 6fd9e31d0..b888bb633 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1034,12 +1034,9 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, switch (function) { case MODBUS_FC_READ_COILS: if (mb_callbacks->read_coils == NULL) { - if (ctx->debug) { - fprintf(stderr, "Callback not defined for _FC_READ_COILS.\n"); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, + FALSE, "Callback not defined for _FC_READ_COILS.\n"); } else { uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; @@ -1048,8 +1045,8 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( - ctx, &sft, - -rv, rsp); + ctx, &sft, -rv, rsp, + FALSE, ""); } else { rsp_length += rv; } @@ -1057,12 +1054,9 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, break; case MODBUS_FC_READ_DISCRETE_INPUTS: if (mb_callbacks->read_discrete_inputs == NULL) { - if (ctx->debug) { - fprintf(stderr, "Callback not defined for _FC_READ_DISCRETE_INPUTS.\n"); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, + FALSE, "Callback not defined for _FC_READ_DISCRETE_INPUTS.\n"); } else { uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; @@ -1070,8 +1064,8 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, rv = mb_callbacks->read_discrete_inputs(address, nb, &rsp[rsp_length], user_data); if (rv < 0) { rsp_length = response_exception( - ctx, &sft, - -rv, rsp); + ctx, &sft, -rv, rsp, + FALSE, ""); } else { rsp_length += rv; } @@ -1079,12 +1073,9 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, break; case MODBUS_FC_READ_HOLDING_REGISTERS: if (mb_callbacks->read_holding_registers == NULL) { - if (ctx->debug) { - fprintf(stderr, "Callback not defined for _FC_READ_HOLDING_REGISTERS.\n"); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, + FALSE, "Callback not defined for _FC_READ_HOLDING_REGISTERS.\n"); } else { uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; @@ -1093,8 +1084,8 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( - ctx, &sft, - -rv, rsp); + ctx, &sft, -rv, rsp, + FALSE, ""); } else { rsp_length += rv; } @@ -1102,12 +1093,9 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, break; case MODBUS_FC_READ_INPUT_REGISTERS: if (mb_callbacks->read_input_registers == NULL) { - if (ctx->debug) { - fprintf(stderr, "Callback not defined for _FC_READ_INPUT_REGISTERS.\n"); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, + FALSE, "Callback not defined for _FC_READ_INPUT_REGISTERS.\n"); } else { uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; @@ -1116,8 +1104,8 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( - ctx, &sft, - -rv, rsp); + ctx, &sft, -rv, rsp, + FALSE, ""); } else { rsp_length += rv; } @@ -1125,12 +1113,9 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, break; case MODBUS_FC_WRITE_SINGLE_COIL: if (mb_callbacks->write_single_coil == NULL) { - if (ctx->debug) { - fprintf(stderr, "Callback not defined for _FC_WRITE_SINGLE_COIL.\n"); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, + FALSE, "Callback not defined for _FC_WRITE_SINGLE_COIL.\n"); } else { uint16_t value = (req[offset + 3] << 8) + req[offset + 4]; @@ -1138,39 +1123,32 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, rv = mb_callbacks->write_single_coil(address, 1, &req[offset + 3], user_data); if (rv < 0) { rsp_length = response_exception( - ctx, &sft, - -rv, rsp); + ctx, &sft, -rv, rsp, + FALSE, ""); } else { memcpy(rsp, req, req_length); rsp_length = req_length; } } else { - if (ctx->debug) { - fprintf(stderr, - "Illegal data value %0X in _FC_WRITE_SINGLE_COIL request at address %0X.\n", - value, address); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, + FALSE, "Illegal data value %0X in _FC_WRITE_SINGLE_COIL request at address %0X.\n", + value, address); } } break; case MODBUS_FC_WRITE_SINGLE_REGISTER: if (mb_callbacks->write_single_register == NULL) { - if (ctx->debug) { - fprintf(stderr, "Callback not defined for _FC_WRITE_SINGLE_REGISTER.\n"); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, + FALSE, "Callback not defined for _FC_WRITE_SINGLE_REGISTER.\n"); } else { rv = mb_callbacks->write_single_register(address, 1, &req[offset + 3], user_data); if (rv < 0) { rsp_length = response_exception( - ctx, &sft, - -rv, rsp); + ctx, &sft, -rv, rsp, + FALSE, ""); } else { memcpy(rsp, req, req_length); rsp_length = req_length; @@ -1179,20 +1157,17 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, break; case MODBUS_FC_WRITE_MULTIPLE_COILS: if (mb_callbacks->write_multiple_coils == NULL) { - if (ctx->debug) { - fprintf(stderr, "Callback not defined for _FC_WRITE_MULTIPLE_COILS.\n"); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, + FALSE, "Callback not defined for _FC_WRITE_MULTIPLE_COILS.\n"); } else { uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; rv = mb_callbacks->write_multiple_coils(address, nb, &req[offset+6], user_data); if (rv < 0) { rsp_length = response_exception( - ctx, &sft, - -rv, rsp); + ctx, &sft, -rv, rsp, + FALSE, ""); } else { rsp_length = ctx->backend->build_response_basis(&sft, rsp); /* 4 to copy the bit address (2) and the quantity of bits */ @@ -1203,20 +1178,17 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, break; case MODBUS_FC_WRITE_MULTIPLE_REGISTERS: if (mb_callbacks->write_multiple_registers == NULL) { - if (ctx->debug) { - fprintf(stderr, "Callback not defined for _FC_WRITE_MULTIPLE_REGISTERS.\n"); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, + FALSE, "Callback not defined for _FC_WRITE_MULTIPLE_REGISTERS.\n"); } else { uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; rv = mb_callbacks->write_multiple_registers(address, nb, &req[offset+6], user_data); if (rv < 0) { rsp_length = response_exception( - ctx, &sft, - -rv, rsp); + ctx, &sft, -rv, rsp, + FALSE, ""); } else { rsp_length = ctx->backend->build_response_basis(&sft, rsp); /* 4 to copy the bit address (2) and the quantity of bits */ @@ -1245,12 +1217,9 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, case MODBUS_FC_WRITE_AND_READ_REGISTERS: if (mb_callbacks->read_holding_registers == NULL || mb_callbacks->write_multiple_registers == NULL) { - if (ctx->debug) { - fprintf(stderr, "Callbacks not defined for _FC_WRITE_AND_READ_REGISTERS.\n"); - } rsp_length = response_exception( - ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp); + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, + FALSE, "Callbacks not defined for _FC_WRITE_AND_READ_REGISTERS.\n"); } else { uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; uint16_t address_write = (req[offset + 5] << 8) + req[offset + 6]; @@ -1261,16 +1230,16 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, rv = mb_callbacks->write_multiple_registers(address_write, nb_write, &req[offset+10], user_data); if (rv < 0) { rsp_length = response_exception( - ctx, &sft, - -rv, rsp); + ctx, &sft, -rv, rsp, + FALSE, ""); } else { /* ...then read the response. */ rsp_length = ctx->backend->build_response_basis(&sft, rsp); rv = mb_callbacks->read_holding_registers(address, nb, &rsp[rsp_length], user_data); if (rv < 0) { rsp_length = response_exception( - ctx, &sft, - -rv, rsp); + ctx, &sft, -rv, rsp, + FALSE, ""); } else { rsp_length += rv; } @@ -1278,9 +1247,9 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, } break; default: - rsp_length = response_exception(ctx, &sft, - MODBUS_EXCEPTION_ILLEGAL_FUNCTION, - rsp); + rsp_length = response_exception( + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, + FALSE, ""); break; } From 94306ef63647d120de2831e63aeec1f74b9528a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frode=20Tenneb=C3=B8?= Date: Sat, 21 May 2016 10:06:05 +0200 Subject: [PATCH 39/41] Add debug message on unknown function code. --- src/modbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modbus.c b/src/modbus.c index b888bb633..baef6cc83 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1249,7 +1249,7 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, default: rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, - FALSE, ""); + FALSE, "Unknown Modbus function code: 0x%0X\n", function); break; } From ccfbe0c82166aa9f32885468b28a141350882f03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frode=20Tenneb=C3=B8?= Date: Sat, 21 May 2016 10:54:49 +0200 Subject: [PATCH 40/41] Add debug messages on illegal callback responses. --- src/modbus.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index baef6cc83..fb6838139 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1046,7 +1046,7 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, ""); + FALSE, "Illegal callback response (%d) in _FC_READ_COILS.\n", rv); } else { rsp_length += rv; } @@ -1065,7 +1065,7 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, ""); + FALSE, "Illegal callback response (%d) in _FC_READ_DISCRETE_INPUTS.\n", rv); } else { rsp_length += rv; } @@ -1085,7 +1085,7 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, ""); + FALSE, "Illegal callback response (%d) in _FC_READ_HOLDING_REGISTERS.\n", rv); } else { rsp_length += rv; } @@ -1105,7 +1105,7 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, ""); + FALSE, "Illegal callback response (%d) in _FC_READ_INPUT_REGISTERS.\n", rv); } else { rsp_length += rv; } @@ -1124,7 +1124,7 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, ""); + FALSE, "Illegal callback response (%d) in _FC_WRITE_SINGLE_COIL.\n", rv); } else { memcpy(rsp, req, req_length); rsp_length = req_length; @@ -1148,7 +1148,7 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, ""); + FALSE, "Illegal callback response (%d) in _FC_WRITE_SINGLE_REGISTER.\n", rv); } else { memcpy(rsp, req, req_length); rsp_length = req_length; @@ -1167,7 +1167,7 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, ""); + FALSE, "Illegal callback response (%d) in _FC_WRITE_MULTIPLE_COILS.\n", rv); } else { rsp_length = ctx->backend->build_response_basis(&sft, rsp); /* 4 to copy the bit address (2) and the quantity of bits */ @@ -1188,7 +1188,7 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, ""); + FALSE, "Illegal callback response (%d) in _FC_WRITE_MULTIPLE_REGISTERS.\n", rv); } else { rsp_length = ctx->backend->build_response_basis(&sft, rsp); /* 4 to copy the bit address (2) and the quantity of bits */ @@ -1231,7 +1231,7 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, ""); + FALSE, "Illegal callback response (%d) in write part of _FC_WRITE_AND_READ_REGISTERS.\n", rv); } else { /* ...then read the response. */ rsp_length = ctx->backend->build_response_basis(&sft, rsp); @@ -1239,7 +1239,7 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, ""); + FALSE, "Illegal callback response (%d) in read part of _FC_WRITE_AND_READ_REGISTERS.\n", rv); } else { rsp_length += rv; } From 493c4abc08c65b03985a4e50ec2ffa70efeee4c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frode=20Tenneb=C3=B8?= Date: Sun, 29 May 2016 15:33:48 +0200 Subject: [PATCH 41/41] Refactored code in modbus_reply_callback() to avoid duplication of code. --- doc/modbus_reply_callback.txt | 40 +++---- src/modbus.c | 213 +++++++++++++--------------------- src/modbus.h | 37 +++--- 3 files changed, 114 insertions(+), 176 deletions(-) diff --git a/doc/modbus_reply_callback.txt b/doc/modbus_reply_callback.txt index 547354302..0e17ea1a9 100644 --- a/doc/modbus_reply_callback.txt +++ b/doc/modbus_reply_callback.txt @@ -25,17 +25,17 @@ and sent by using the information of the modbus context _ctx_. For read functions, the callback will need to supply the response, returning the length of that response. For pure write functions (i.e. not -_FC_WRITE_AND_READ_REGISTERS), the response is automatically constructed. +MODBUS_FC_WRITE_AND_READ_REGISTERS), the response is automatically constructed. -The callbacks are supplied in the callback mapping _mb_callbacks_ according -to the type of the request. If a callback is not provided for a given -function, *modbus_reply_callback()* constructs the exception +The callbacks are supplied in an array of callbacks _mb_callbacks_ indexed by +the requested function. If a callback is not provided for a given function, +*modbus_reply_callback()* constructs the exception MODBUS_EXCEPTION_ILLEGAL_FUNCTION accordingly. It is the callback's responsibility to return the Modbus error code -*negated* (except for the case where the operation is _FC_WRITE_SINGLE_COIL -and the value is neither OFF nor ON) in case of errors. In this case an -exception response will be sent. +*negated* (except for the case where the operation is +MODBUS_FC_WRITE_SINGLE_COIL and the value is neither OFF nor ON) in case of +errors. In this case an exception response will be sent. Note: If the return value is => 0 it is assumed that the call was successful both for read and write callbacks. For read callbacks the value is also used @@ -44,22 +44,16 @@ as the response length supplied by the callback function. An optional parameter _user_data_ can be suppied which will be passed to the callback function, typically for context purpose. -The read and write callbacks have the following prototypes: +The callbacks have the following prototype: -int (*modbus_read_callback)(uint16_t addr, uint16_t nb, uint8_t *rsp, void *user_data); +int (*modbus_callback)(uint16_t addr, uint16_t nb, uint8_t *msg, void *user_data); -Each read callback function takes the address (_addr_), number of registers -(_nb_), address to response message (_rsp_)(, and pointer to optional user data -(_user_data_). The response is pre-filled with the MBAP header AND FCode -(function code), 8 bytes in total. The callback is responsible for filling -in the rest of the response PDU according to Modbus Application Protocol' +Each callback function takes the address (_addr_), number of registers +(_nb_), address to request/response message (_msg_), and pointer to optional +user data (_user_data_). The read response is pre-filled with the MBAP header +and FCode (function code), 8 bytes in total. The callback is responsible for +filling in the rest of the response PDU according to Modbus Application Protocol V1.1b3, returning the size of the PDU. - -int (*modbus_write_callback)(uint16_t addr, uint16_t nb, const uint8_t *req, void *user_data); - -Each write callback function takes the address (_addr_), number of registers -(_nb_), address to the request to be written (_req_), and pointer to optional -user data (_user_data_). This function is designed for Modbus server. @@ -82,11 +76,11 @@ EXAMPLE ------- [source,c] ------------------- -static struct modbus_callbacks_t callbacks; +static modbus_callbacks_t callbacks; int my_read_coil_implementation(uint16_t addr, uint16_t nb, uint8_t *rsp, void *user_data); ... -callbacks.read_coils = &my_read_coil_implementation; +callbacks[MODBUS_FC_READ_COILS] = &my_read_coil_implementation; ... modbus_reply_callback(context, &request, request_lenght, &callbacks, NULL); @@ -101,5 +95,5 @@ linkmb:libmodbus[7] AUTHORS ------- -The libmodbus documentation was written by Stéphane Raimbault +Theis libmodbus documentation was written by Frode Tennebø diff --git a/src/modbus.c b/src/modbus.c index fb6838139..f50419fba 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -41,6 +41,33 @@ typedef enum { _STEP_DATA } _step_t; +const char * const modbus_callback_names[] = + {NULL, + "READ_COILS", + "READ_DISCRETE_INPUTS", + "READ_HOLDING_REGISTERS", + "READ_INPUT_REGISTERS", + "WRITE_SINGLE_COIL", + "WRITE_SINGLE_REGISTER", + "READ_EXCEPTION_STATUS", + NULL, + NULL, + NULL, + NULL, + NULL, + NULL, + NULL, + "WRITE_MULTIPLE_COILS", + "WRITE_MULTIPLE_REGISTERS", + NULL, + NULL, + NULL, + NULL, + NULL, + "MASK_WRITE_REGISTER", + "WRITE_AND_READ_REGISTERS" + }; + const char *modbus_strerror(int errnum) { switch (errnum) { case EMBXILFUN: @@ -992,30 +1019,31 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, } /* This function is modelled on and takes the same parameters as modbus_reply() - above. Based on the request, modbus_reply_callback() will invoke either a - read or a write callback function. In addition, this function takes a - pointer to optional user data which is passed along to the callback function. + above. Based on the request, modbus_reply_callback() will invoke a + callback function. In addition, this function takes a pointer to optional + user data which is passed along to the callback function. For read functions, the callback will need to supply the response, returning the length of that response. For pure write (i.e. not - _FC_WRITE_AND_READ_REGISTERS) functions, the response is automatically + MODBUS_FC_WRITE_AND_READ_REGISTERS) functions, the response is automatically constructed. If a callback is not provided for a given function, modbus_reply_callback() constructs the exception MODBUS_EXCEPTION_ILLEGAL_FUNCTION accordingly. - Except for _FC_WRITE_SINGLE_COIL where (value != 0xFF00 || value != 0x0) - it is the callback's responsibility to return the MODBUS error code NEGATED! + Except for MODBUS_FC_WRITE_SINGLE_COIL where + (value != 0xFF00 || value != 0x0) it is the callback's responsibility to + return the MODBUS error code NEGATED! NB: If the return value is >= 0 it is assumed that the call was successful, and the value is used as the response length for read callbacks. Usage: - static struct modbus_callbacks_t callbacks; - callbacks.read_coils = &my_read_coil_implementation; + static modbus_callbacks_t callbacks; + callbacks[MODBUS_FC_READ_COILS] = &my_read_coil_implementation; modbus_reply_callback(context, &request, request_lenght, &callbacks, NULL); */ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, - int req_length, modbus_callbacks_t *mb_callbacks, + int req_length, const modbus_callbacks_t mb_callbacks, void *user_data) { int offset = ctx->backend->header_length; @@ -1033,98 +1061,44 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, switch (function) { case MODBUS_FC_READ_COILS: - if (mb_callbacks->read_coils == NULL) { - rsp_length = response_exception( - ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, - FALSE, "Callback not defined for _FC_READ_COILS.\n"); - } else { - uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; - - rsp_length = ctx->backend->build_response_basis(&sft, rsp); - rv = mb_callbacks->read_coils(address, nb, &rsp[rsp_length], user_data); - - if (rv < 0) { - rsp_length = response_exception( - ctx, &sft, -rv, rsp, - FALSE, "Illegal callback response (%d) in _FC_READ_COILS.\n", rv); - } else { - rsp_length += rv; - } - } - break; case MODBUS_FC_READ_DISCRETE_INPUTS: - if (mb_callbacks->read_discrete_inputs == NULL) { - rsp_length = response_exception( - ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, - FALSE, "Callback not defined for _FC_READ_DISCRETE_INPUTS.\n"); - } else { - uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; - - rsp_length = ctx->backend->build_response_basis(&sft, rsp); - rv = mb_callbacks->read_discrete_inputs(address, nb, &rsp[rsp_length], user_data); - if (rv < 0) { - rsp_length = response_exception( - ctx, &sft, -rv, rsp, - FALSE, "Illegal callback response (%d) in _FC_READ_DISCRETE_INPUTS.\n", rv); - } else { - rsp_length += rv; - } - } - break; case MODBUS_FC_READ_HOLDING_REGISTERS: - if (mb_callbacks->read_holding_registers == NULL) { + case MODBUS_FC_READ_INPUT_REGISTERS: { + if (mb_callbacks[function] == NULL) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, - FALSE, "Callback not defined for _FC_READ_HOLDING_REGISTERS.\n"); + FALSE, "Callback not defined for %s.\n", modbus_callback_names[function]); } else { uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; rsp_length = ctx->backend->build_response_basis(&sft, rsp); - rv = mb_callbacks->read_holding_registers(address, nb, &rsp[rsp_length], user_data); + rv = (mb_callbacks[function]) (address, nb, &rsp[rsp_length], user_data); if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, "Illegal callback response (%d) in _FC_READ_HOLDING_REGISTERS.\n", rv); + FALSE, "Illegal callback response (%d) in %s.\n", rv, modbus_callback_names[function]); } else { rsp_length += rv; } } break; - case MODBUS_FC_READ_INPUT_REGISTERS: - if (mb_callbacks->read_input_registers == NULL) { - rsp_length = response_exception( - ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, - FALSE, "Callback not defined for _FC_READ_INPUT_REGISTERS.\n"); - } else { - uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; - - rsp_length = ctx->backend->build_response_basis(&sft, rsp); - rv = mb_callbacks->read_input_registers(address, nb, &rsp[rsp_length], user_data); + } - if (rv < 0) { - rsp_length = response_exception( - ctx, &sft, -rv, rsp, - FALSE, "Illegal callback response (%d) in _FC_READ_INPUT_REGISTERS.\n", rv); - } else { - rsp_length += rv; - } - } - break; - case MODBUS_FC_WRITE_SINGLE_COIL: - if (mb_callbacks->write_single_coil == NULL) { + case MODBUS_FC_WRITE_SINGLE_COIL: { + if (mb_callbacks[function] == NULL) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, - FALSE, "Callback not defined for _FC_WRITE_SINGLE_COIL.\n"); + FALSE, "Callback not defined for %s.\n", modbus_callback_names[function]); } else { uint16_t value = (req[offset + 3] << 8) + req[offset + 4]; if (value == 0xFF00 || value == 0x0) { - rv = mb_callbacks->write_single_coil(address, 1, &req[offset + 3], user_data); + rv = (mb_callbacks[function]) (address, 1, (uint8_t *) &req[offset + 3], user_data); if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, "Illegal callback response (%d) in _FC_WRITE_SINGLE_COIL.\n", rv); + FALSE, "Illegal callback response (%d) in %s.\n", rv, modbus_callback_names[function]); } else { memcpy(rsp, req, req_length); rsp_length = req_length; @@ -1132,71 +1106,47 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, } else { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, - FALSE, "Illegal data value %0X in _FC_WRITE_SINGLE_COIL request at address %0X.\n", - value, address); + FALSE, "Illegal data value %0X in %s request at address %0X.\n", + value, modbus_callback_names[function], address); } } break; - case MODBUS_FC_WRITE_SINGLE_REGISTER: - if (mb_callbacks->write_single_register == NULL) { - rsp_length = response_exception( - ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, - FALSE, "Callback not defined for _FC_WRITE_SINGLE_REGISTER.\n"); - } else { - rv = mb_callbacks->write_single_register(address, 1, &req[offset + 3], user_data); + } - if (rv < 0) { - rsp_length = response_exception( - ctx, &sft, -rv, rsp, - FALSE, "Illegal callback response (%d) in _FC_WRITE_SINGLE_REGISTER.\n", rv); - } else { - memcpy(rsp, req, req_length); - rsp_length = req_length; - } - } - break; + case MODBUS_FC_WRITE_SINGLE_REGISTER: case MODBUS_FC_WRITE_MULTIPLE_COILS: - if (mb_callbacks->write_multiple_coils == NULL) { - rsp_length = response_exception( - ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, - FALSE, "Callback not defined for _FC_WRITE_MULTIPLE_COILS.\n"); - } else { - uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; - rv = mb_callbacks->write_multiple_coils(address, nb, &req[offset+6], user_data); + case MODBUS_FC_WRITE_MULTIPLE_REGISTERS: { + unsigned int is_single = (function == MODBUS_FC_WRITE_SINGLE_REGISTER); + uint16_t nb = is_single ? 1 : (req[offset + 3] << 8) + req[offset + 4]; + int skip = is_single ? 3 : 6; - if (rv < 0) { - rsp_length = response_exception( - ctx, &sft, -rv, rsp, - FALSE, "Illegal callback response (%d) in _FC_WRITE_MULTIPLE_COILS.\n", rv); - } else { - rsp_length = ctx->backend->build_response_basis(&sft, rsp); - /* 4 to copy the bit address (2) and the quantity of bits */ - memcpy(rsp + rsp_length, req + rsp_length, 4); - rsp_length += 4; - } - } - break; - case MODBUS_FC_WRITE_MULTIPLE_REGISTERS: - if (mb_callbacks->write_multiple_registers == NULL) { + if (mb_callbacks[function] == NULL) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, - FALSE, "Callback not defined for _FC_WRITE_MULTIPLE_REGISTERS.\n"); + FALSE, "Callback not defined for %s.\n", modbus_callback_names[function]); } else { - uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; - rv = mb_callbacks->write_multiple_registers(address, nb, &req[offset+6], user_data); + rv = (mb_callbacks[function]) (address, nb, (uint8_t *) &req[offset + skip], user_data); if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, "Illegal callback response (%d) in _FC_WRITE_MULTIPLE_REGISTERS.\n", rv); + FALSE, "Illegal callback response (%d) in %s.\n", rv, modbus_callback_names[function]); } else { - rsp_length = ctx->backend->build_response_basis(&sft, rsp); - /* 4 to copy the bit address (2) and the quantity of bits */ - memcpy(rsp + rsp_length, req + rsp_length, 4); - rsp_length += 4; + if (function == MODBUS_FC_WRITE_SINGLE_REGISTER) + { + memcpy(rsp, req, req_length); + rsp_length = req_length; + } else { + rsp_length = ctx->backend->build_response_basis(&sft, rsp); + /* 4 to copy the bit address (2) and the quantity of bits */ + memcpy(rsp + rsp_length, req + rsp_length, 4); + rsp_length += 4; + } } } break; + } + case MODBUS_FC_REPORT_SLAVE_ID: { int str_len; int byte_count_pos; @@ -1214,12 +1164,13 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, rsp[byte_count_pos] = rsp_length - byte_count_pos - 1; break; } - case MODBUS_FC_WRITE_AND_READ_REGISTERS: - if (mb_callbacks->read_holding_registers == NULL || - mb_callbacks->write_multiple_registers == NULL) { + + case MODBUS_FC_WRITE_AND_READ_REGISTERS: { + if (mb_callbacks[MODBUS_FC_READ_HOLDING_REGISTERS] == NULL || + mb_callbacks[MODBUS_FC_WRITE_MULTIPLE_REGISTERS] == NULL) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, - FALSE, "Callbacks not defined for _FC_WRITE_AND_READ_REGISTERS.\n"); + FALSE, "Callbacks not defined for %s.\n", modbus_callback_names[function]); } else { uint16_t nb = (req[offset + 3] << 8) + req[offset + 4]; uint16_t address_write = (req[offset + 5] << 8) + req[offset + 6]; @@ -1227,25 +1178,27 @@ int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, /* Write first... (10 is the offset to the start of the values to write) */ - rv = mb_callbacks->write_multiple_registers(address_write, nb_write, &req[offset+10], user_data); + rv = (mb_callbacks[MODBUS_FC_WRITE_MULTIPLE_REGISTERS]) (address_write, nb_write, (uint8_t *) &req[offset+10], user_data); if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, "Illegal callback response (%d) in write part of _FC_WRITE_AND_READ_REGISTERS.\n", rv); + FALSE, "Illegal callback response (%d) in write part of %s.\n", rv, modbus_callback_names[function]); } else { /* ...then read the response. */ rsp_length = ctx->backend->build_response_basis(&sft, rsp); - rv = mb_callbacks->read_holding_registers(address, nb, &rsp[rsp_length], user_data); + rv = (mb_callbacks[MODBUS_FC_READ_HOLDING_REGISTERS]) (address, nb, &rsp[rsp_length], user_data); if (rv < 0) { rsp_length = response_exception( ctx, &sft, -rv, rsp, - FALSE, "Illegal callback response (%d) in read part of _FC_WRITE_AND_READ_REGISTERS.\n", rv); + FALSE, "Illegal callback response (%d) in read part of %s.\n", rv, modbus_callback_names[function]); } else { rsp_length += rv; } } } break; + } + default: rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, diff --git a/src/modbus.h b/src/modbus.h index 886f37a50..02959934b 100644 --- a/src/modbus.h +++ b/src/modbus.h @@ -176,28 +176,19 @@ typedef enum MODBUS_ERROR_RECOVERY_PROTOCOL = (1<<2) } modbus_error_recovery_mode; -/* Each read callback function takes the address, number of registers, address - to response message and pointer to optional user data. The response is - pre-filled with the MBAP header AND FCode (function code), 8 bytes in total. - The callback is responsible for filling in the rest of the response PDU - according to Modbus Application Protocol V1.1b3, returning the size of the - PDU. */ -typedef int (*modbus_read_callback)(uint16_t addr, uint16_t nb, uint8_t *rsp, void *user_data); - -/* Each write callback function takes the address, number of registers, - address to the request to be written and pointer to optional user data. */ -typedef int (*modbus_write_callback)(uint16_t addr, uint16_t nb, const uint8_t *req, void *user_data); - -typedef struct { - modbus_read_callback read_coils; - modbus_read_callback read_discrete_inputs; - modbus_read_callback read_holding_registers; /* Also used for _FC_WRITE_AND_READ_REGISTERS */ - modbus_read_callback read_input_registers; - modbus_write_callback write_single_coil; - modbus_write_callback write_single_register; - modbus_write_callback write_multiple_coils; - modbus_write_callback write_multiple_registers; /* Also used for _FC_WRITE_AND_READ_REGISTERS */ -} modbus_callbacks_t; +/* Each callback function takes the address, number of registers, + address to the response (for read) or request (for write) message and + a pointer to optional user data. + For read callbacks, the response is pre-filled with the MBAP header + AND FCode (function code), 8 bytes in total. The callback is responsible + for filling in the rest of the response PDU according to + Modbus Application Protocol V1.1b3, returning the size of the PDU. */ +typedef int (*modbus_callback)(uint16_t addr, uint16_t nb, uint8_t *msg, void *user_data); + +/* NOTE: The entry for MODBUS_FC_WRITE_AND_READ_REGISTERS doesn't exist since + MODBUS_FC_READ_HOLDING_REGISTERS is used for read and + MODBUS_FC_WRITE_MULTIPLE_REGISTERS is used for write! */ +typedef modbus_callback modbus_callbacks_t[MODBUS_FC_WRITE_AND_READ_REGISTERS]; MODBUS_API int modbus_set_slave(modbus_t* ctx, int slave); MODBUS_API int modbus_set_error_recovery(modbus_t *ctx, modbus_error_recovery_mode error_recovery); @@ -258,7 +249,7 @@ MODBUS_API int modbus_reply_exception(modbus_t *ctx, const uint8_t *req, unsigned int exception_code); MODBUS_API int modbus_reply_callback(modbus_t *ctx, const uint8_t *req, - int req_length, modbus_callbacks_t *mb_callbacks, + int req_length, const modbus_callbacks_t mb_callbacks, void *user_data); /**