From 1941fd4aa96007b69b4c119bd39491d39ef2373e Mon Sep 17 00:00:00 2001 From: =?utf8?q?St=C3=A9phane=20Raimbault?= Date: Tue, 25 Oct 2011 09:38:58 +0200 Subject: [PATCH] Check slave only in RTU backend and add unit tests - new function _modbus_rtu_pre_check_confirmation - new special test value to trigger an invalid response - add unit test to cover invalid TID too --- src/modbus-rtu.c | 20 +++++++++++++++++++- src/modbus.c | 6 ------ tests/unit-test-client.c | 18 ++++++++++++++---- tests/unit-test-server.c | 14 +++++++++++++- tests/unit-test.h.in | 5 ++++- 5 files changed, 50 insertions(+), 13 deletions(-) diff --git a/src/modbus-rtu.c b/src/modbus-rtu.c index 371ed14..ea192d1 100644 --- a/src/modbus-rtu.c +++ b/src/modbus-rtu.c @@ -313,6 +313,24 @@ ssize_t _modbus_rtu_recv(modbus_t *ctx, uint8_t *rsp, int rsp_length) int _modbus_rtu_flush(modbus_t *); +int _modbus_rtu_pre_check_confirmation(modbus_t *ctx, const uint8_t *req, + const uint8_t *rsp, int rsp_length) +{ + /* Check responding slave is the slave we requested (except for broacast + * request) */ + if (req[0] != 0 && req[0] != rsp[0]) { + if (ctx->debug) { + fprintf(stderr, + "The responding slave %d it not the requested slave %d", + rsp[0], req[0]); + } + errno = EMBBADSLAVE; + return -1; + } else { + return 0; + } +} + /* The check_crc16 function shall return the message length if the CRC is valid. Otherwise it shall return -1 and set errno to EMBADCRC. */ int _modbus_rtu_check_integrity(modbus_t *ctx, uint8_t *msg, @@ -940,7 +958,7 @@ const modbus_backend_t _modbus_rtu_backend = { _modbus_rtu_send, _modbus_rtu_recv, _modbus_rtu_check_integrity, - NULL, + _modbus_rtu_pre_check_confirmation, _modbus_rtu_connect, _modbus_rtu_close, _modbus_rtu_flush, diff --git a/src/modbus.c b/src/modbus.c index c68ddd0..0695363 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -494,12 +494,6 @@ static int check_confirmation(modbus_t *ctx, uint8_t *req, } } - /* Check responding slave is the slave we requested */ - if (req[0] != 0 && rsp[0] != req[0]) { - errno = EMBBADSLAVE; - return -1; - } - rsp_length_computed = compute_response_length_from_request(ctx, req); /* Check length */ diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index 533e6a7..3a16419 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -495,7 +495,7 @@ int main(int argc, char *argv[]) uint8_t rsp[MODBUS_TCP_MAX_ADU_LENGTH]; /* No response in RTU mode */ - printf("1/4-A No response from slave %d: ", INVALID_SERVER_ID); + printf("1/5-A No response from slave %d: ", INVALID_SERVER_ID); if (rc == -1 && errno == ETIMEDOUT) { printf("OK\n"); @@ -509,7 +509,7 @@ int main(int argc, char *argv[]) RAW_REQ_LENGTH * sizeof(uint8_t)); rc = modbus_receive_confirmation(ctx, rsp); - printf("1/4-B No response from slave %d with invalid request: ", + printf("1/5-B No response from slave %d with invalid request: ", INVALID_SERVER_ID); if (rc == -1 && errno == ETIMEDOUT) { @@ -539,7 +539,7 @@ int main(int argc, char *argv[]) rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS, UT_REGISTERS_NB, tab_rp_registers); - printf("2/4 Reply after a broadcast query: "); + printf("2/5 Reply after a broadcast query: "); if (rc == UT_REGISTERS_NB) { printf("OK\n"); } else { @@ -554,7 +554,7 @@ int main(int argc, char *argv[]) modbus_set_slave(ctx, MODBUS_TCP_SLAVE); } - printf("3/4 Report slave ID: \n"); + printf("3/5 Report slave ID: \n"); /* tab_rp_bits is used to store bytes */ rc = modbus_report_slave_id(ctx, tab_rp_bits); if (rc == -1) { @@ -587,6 +587,16 @@ int main(int argc, char *argv[]) printf("\n"); } + printf("5/5 Response with an invalid TID or slave: "); + rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS_INVALID_TID_OR_SLAVE, + 1, tab_rp_registers); + if (rc == -1) { + printf("OK\n"); + } else { + printf("FAILED\n"); + goto close; + } + /* Save original timeout */ modbus_get_response_timeout(ctx, &old_response_timeout); diff --git a/tests/unit-test-server.c b/tests/unit-test-server.c index 9a986c5..8143a66 100644 --- a/tests/unit-test-server.c +++ b/tests/unit-test-server.c @@ -128,11 +128,23 @@ int main(int argc, char*argv[]) MODBUS_SET_INT16_TO_INT8(query, header_length + 3, UT_REGISTERS_NB_SPECIAL - 1); } else if (MODBUS_GET_INT16_FROM_INT8(query, header_length + 1) - == UT_REGISTERS_ADDRESS_SPECIAL) { + == UT_REGISTERS_ADDRESS_SPECIAL) { printf("Reply to this special register address by an exception\n"); modbus_reply_exception(ctx, query, MODBUS_EXCEPTION_SLAVE_OR_SERVER_BUSY); continue; + } else if (MODBUS_GET_INT16_FROM_INT8(query, header_length + 1) + == UT_REGISTERS_ADDRESS_INVALID_TID_OR_SLAVE) { + const int RAW_REQ_LENGTH = 5; + uint8_t raw_req[] = { + (use_backend == RTU) ? INVALID_SERVER_ID : 0xFF, + 0x03, + 0x02, 0x00, 0x00 + }; + + printf("Reply with an invalid TID or slave\n"); + modbus_send_raw_request(ctx, raw_req, RAW_REQ_LENGTH * sizeof(uint8_t)); + continue; } } diff --git a/tests/unit-test.h.in b/tests/unit-test.h.in index d2aa14c..85fe877 100644 --- a/tests/unit-test.h.in +++ b/tests/unit-test.h.in @@ -45,8 +45,11 @@ const uint16_t UT_INPUT_BITS_NB = 0x16; const uint8_t UT_INPUT_BITS_TAB[] = { 0xAC, 0xDB, 0x35 }; const uint16_t UT_REGISTERS_ADDRESS = 0x6B; -/* Raise a manual exception when this adress is used for the first byte */ +/* Raise a manual exception when this address is used for the first byte */ const uint16_t UT_REGISTERS_ADDRESS_SPECIAL = 0x6C; +/* 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_NB = 0x3; const uint16_t UT_REGISTERS_TAB[] = { 0x022B, 0x0001, 0x0064 }; /* If the following value is used, a bad response is sent. -- 2.11.4.GIT