Commit 1941fd4aa96007b69b4c119bd39491d39ef2373e

Authored by Stéphane Raimbault
1 parent 6bad3494

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
@@ -313,6 +313,24 @@ ssize_t _modbus_rtu_recv(modbus_t *ctx, uint8_t *rsp, int rsp_length) @@ -313,6 +313,24 @@ ssize_t _modbus_rtu_recv(modbus_t *ctx, uint8_t *rsp, int rsp_length)
313 313
314 int _modbus_rtu_flush(modbus_t *); 314 int _modbus_rtu_flush(modbus_t *);
315 315
  316 +int _modbus_rtu_pre_check_confirmation(modbus_t *ctx, const uint8_t *req,
  317 + const uint8_t *rsp, int rsp_length)
  318 +{
  319 + /* Check responding slave is the slave we requested (except for broacast
  320 + * request) */
  321 + if (req[0] != 0 && req[0] != rsp[0]) {
  322 + if (ctx->debug) {
  323 + fprintf(stderr,
  324 + "The responding slave %d it not the requested slave %d",
  325 + rsp[0], req[0]);
  326 + }
  327 + errno = EMBBADSLAVE;
  328 + return -1;
  329 + } else {
  330 + return 0;
  331 + }
  332 +}
  333 +
316 /* The check_crc16 function shall return the message length if the CRC is 334 /* The check_crc16 function shall return the message length if the CRC is
317 valid. Otherwise it shall return -1 and set errno to EMBADCRC. */ 335 valid. Otherwise it shall return -1 and set errno to EMBADCRC. */
318 int _modbus_rtu_check_integrity(modbus_t *ctx, uint8_t *msg, 336 int _modbus_rtu_check_integrity(modbus_t *ctx, uint8_t *msg,
@@ -940,7 +958,7 @@ const modbus_backend_t _modbus_rtu_backend = { @@ -940,7 +958,7 @@ const modbus_backend_t _modbus_rtu_backend = {
940 _modbus_rtu_send, 958 _modbus_rtu_send,
941 _modbus_rtu_recv, 959 _modbus_rtu_recv,
942 _modbus_rtu_check_integrity, 960 _modbus_rtu_check_integrity,
943 - NULL, 961 + _modbus_rtu_pre_check_confirmation,
944 _modbus_rtu_connect, 962 _modbus_rtu_connect,
945 _modbus_rtu_close, 963 _modbus_rtu_close,
946 _modbus_rtu_flush, 964 _modbus_rtu_flush,
src/modbus.c
@@ -494,12 +494,6 @@ static int check_confirmation(modbus_t *ctx, uint8_t *req, @@ -494,12 +494,6 @@ static int check_confirmation(modbus_t *ctx, uint8_t *req,
494 } 494 }
495 } 495 }
496 496
497 - /* Check responding slave is the slave we requested */  
498 - if (req[0] != 0 && rsp[0] != req[0]) {  
499 - errno = EMBBADSLAVE;  
500 - return -1;  
501 - }  
502 -  
503 rsp_length_computed = compute_response_length_from_request(ctx, req); 497 rsp_length_computed = compute_response_length_from_request(ctx, req);
504 498
505 /* Check length */ 499 /* Check length */
tests/unit-test-client.c
@@ -495,7 +495,7 @@ int main(int argc, char *argv[]) @@ -495,7 +495,7 @@ int main(int argc, char *argv[])
495 uint8_t rsp[MODBUS_TCP_MAX_ADU_LENGTH]; 495 uint8_t rsp[MODBUS_TCP_MAX_ADU_LENGTH];
496 496
497 /* No response in RTU mode */ 497 /* No response in RTU mode */
498 - printf("1/4-A No response from slave %d: ", INVALID_SERVER_ID); 498 + printf("1/5-A No response from slave %d: ", INVALID_SERVER_ID);
499 499
500 if (rc == -1 && errno == ETIMEDOUT) { 500 if (rc == -1 && errno == ETIMEDOUT) {
501 printf("OK\n"); 501 printf("OK\n");
@@ -509,7 +509,7 @@ int main(int argc, char *argv[]) @@ -509,7 +509,7 @@ int main(int argc, char *argv[])
509 RAW_REQ_LENGTH * sizeof(uint8_t)); 509 RAW_REQ_LENGTH * sizeof(uint8_t));
510 rc = modbus_receive_confirmation(ctx, rsp); 510 rc = modbus_receive_confirmation(ctx, rsp);
511 511
512 - printf("1/4-B No response from slave %d with invalid request: ", 512 + printf("1/5-B No response from slave %d with invalid request: ",
513 INVALID_SERVER_ID); 513 INVALID_SERVER_ID);
514 514
515 if (rc == -1 && errno == ETIMEDOUT) { 515 if (rc == -1 && errno == ETIMEDOUT) {
@@ -539,7 +539,7 @@ int main(int argc, char *argv[]) @@ -539,7 +539,7 @@ int main(int argc, char *argv[])
539 539
540 rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS, 540 rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS,
541 UT_REGISTERS_NB, tab_rp_registers); 541 UT_REGISTERS_NB, tab_rp_registers);
542 - printf("2/4 Reply after a broadcast query: "); 542 + printf("2/5 Reply after a broadcast query: ");
543 if (rc == UT_REGISTERS_NB) { 543 if (rc == UT_REGISTERS_NB) {
544 printf("OK\n"); 544 printf("OK\n");
545 } else { 545 } else {
@@ -554,7 +554,7 @@ int main(int argc, char *argv[]) @@ -554,7 +554,7 @@ int main(int argc, char *argv[])
554 modbus_set_slave(ctx, MODBUS_TCP_SLAVE); 554 modbus_set_slave(ctx, MODBUS_TCP_SLAVE);
555 } 555 }
556 556
557 - printf("3/4 Report slave ID: \n"); 557 + printf("3/5 Report slave ID: \n");
558 /* tab_rp_bits is used to store bytes */ 558 /* tab_rp_bits is used to store bytes */
559 rc = modbus_report_slave_id(ctx, tab_rp_bits); 559 rc = modbus_report_slave_id(ctx, tab_rp_bits);
560 if (rc == -1) { 560 if (rc == -1) {
@@ -587,6 +587,16 @@ int main(int argc, char *argv[]) @@ -587,6 +587,16 @@ int main(int argc, char *argv[])
587 printf("\n"); 587 printf("\n");
588 } 588 }
589 589
  590 + printf("5/5 Response with an invalid TID or slave: ");
  591 + rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS_INVALID_TID_OR_SLAVE,
  592 + 1, tab_rp_registers);
  593 + if (rc == -1) {
  594 + printf("OK\n");
  595 + } else {
  596 + printf("FAILED\n");
  597 + goto close;
  598 + }
  599 +
590 /* Save original timeout */ 600 /* Save original timeout */
591 modbus_get_response_timeout(ctx, &old_response_timeout); 601 modbus_get_response_timeout(ctx, &old_response_timeout);
592 602
tests/unit-test-server.c
@@ -128,11 +128,23 @@ int main(int argc, char*argv[]) @@ -128,11 +128,23 @@ int main(int argc, char*argv[])
128 MODBUS_SET_INT16_TO_INT8(query, header_length + 3, 128 MODBUS_SET_INT16_TO_INT8(query, header_length + 3,
129 UT_REGISTERS_NB_SPECIAL - 1); 129 UT_REGISTERS_NB_SPECIAL - 1);
130 } else if (MODBUS_GET_INT16_FROM_INT8(query, header_length + 1) 130 } else if (MODBUS_GET_INT16_FROM_INT8(query, header_length + 1)
131 - == UT_REGISTERS_ADDRESS_SPECIAL) { 131 + == UT_REGISTERS_ADDRESS_SPECIAL) {
132 printf("Reply to this special register address by an exception\n"); 132 printf("Reply to this special register address by an exception\n");
133 modbus_reply_exception(ctx, query, 133 modbus_reply_exception(ctx, query,
134 MODBUS_EXCEPTION_SLAVE_OR_SERVER_BUSY); 134 MODBUS_EXCEPTION_SLAVE_OR_SERVER_BUSY);
135 continue; 135 continue;
  136 + } else if (MODBUS_GET_INT16_FROM_INT8(query, header_length + 1)
  137 + == UT_REGISTERS_ADDRESS_INVALID_TID_OR_SLAVE) {
  138 + const int RAW_REQ_LENGTH = 5;
  139 + uint8_t raw_req[] = {
  140 + (use_backend == RTU) ? INVALID_SERVER_ID : 0xFF,
  141 + 0x03,
  142 + 0x02, 0x00, 0x00
  143 + };
  144 +
  145 + printf("Reply with an invalid TID or slave\n");
  146 + modbus_send_raw_request(ctx, raw_req, RAW_REQ_LENGTH * sizeof(uint8_t));
  147 + continue;
136 } 148 }
137 } 149 }
138 150
tests/unit-test.h.in
@@ -45,8 +45,11 @@ const uint16_t UT_INPUT_BITS_NB = 0x16; @@ -45,8 +45,11 @@ const uint16_t UT_INPUT_BITS_NB = 0x16;
45 const uint8_t UT_INPUT_BITS_TAB[] = { 0xAC, 0xDB, 0x35 }; 45 const uint8_t UT_INPUT_BITS_TAB[] = { 0xAC, 0xDB, 0x35 };
46 46
47 const uint16_t UT_REGISTERS_ADDRESS = 0x6B; 47 const uint16_t UT_REGISTERS_ADDRESS = 0x6B;
48 -/* Raise a manual exception when this adress is used for the first byte */ 48 +/* Raise a manual exception when this address is used for the first byte */
49 const uint16_t UT_REGISTERS_ADDRESS_SPECIAL = 0x6C; 49 const uint16_t UT_REGISTERS_ADDRESS_SPECIAL = 0x6C;
  50 +/* The response of the server will contains an invalid TID or slave */
  51 +const uint16_t UT_REGISTERS_ADDRESS_INVALID_TID_OR_SLAVE = 0x6D;
  52 +
50 const uint16_t UT_REGISTERS_NB = 0x3; 53 const uint16_t UT_REGISTERS_NB = 0x3;
51 const uint16_t UT_REGISTERS_TAB[] = { 0x022B, 0x0001, 0x0064 }; 54 const uint16_t UT_REGISTERS_TAB[] = { 0x022B, 0x0001, 0x0064 };
52 /* If the following value is used, a bad response is sent. 55 /* If the following value is used, a bad response is sent.