Commit 20281893a781f25af0092c41a9b09553b360f1aa
1 parent
e0ea0303
Avoid to call check_crc16 in a TCP communication.
The test is now outside of check_crc16 and directly included in receive_msg().
Showing
1 changed file
with
30 additions
and
37 deletions
modbus/modbus.c
| @@ -330,33 +330,27 @@ static uint16_t crc16(uint8_t *buffer, uint16_t buffer_length) | @@ -330,33 +330,27 @@ static uint16_t crc16(uint8_t *buffer, uint16_t buffer_length) | ||
| 330 | } | 330 | } |
| 331 | 331 | ||
| 332 | /* If CRC is correct returns 0 else returns INVALID_CRC */ | 332 | /* If CRC is correct returns 0 else returns INVALID_CRC */ |
| 333 | -int check_crc16(modbus_param_t *mb_param, | ||
| 334 | - uint8_t *msg, | ||
| 335 | - const int msg_length) | 333 | +static int check_crc16(modbus_param_t *mb_param, |
| 334 | + uint8_t *msg, | ||
| 335 | + const int msg_length) | ||
| 336 | { | 336 | { |
| 337 | int ret; | 337 | int ret; |
| 338 | - | ||
| 339 | - if (mb_param->type_com == RTU) { | ||
| 340 | - uint16_t crc_calc; | ||
| 341 | - uint16_t crc_received; | 338 | + uint16_t crc_calc; |
| 339 | + uint16_t crc_received; | ||
| 342 | 340 | ||
| 343 | - crc_calc = crc16(msg, msg_length - 2); | ||
| 344 | - crc_received = (msg[msg_length - 2] << 8) | msg[msg_length - 1]; | ||
| 345 | - | ||
| 346 | - /* Check CRC of msg */ | ||
| 347 | - if (crc_calc == crc_received) { | ||
| 348 | - ret = 0; | ||
| 349 | - } else { | ||
| 350 | - char s_error[64]; | ||
| 351 | - sprintf(s_error, | ||
| 352 | - "invalid crc received %0X - crc_calc %0X", | ||
| 353 | - crc_received, crc_calc); | ||
| 354 | - ret = INVALID_CRC; | ||
| 355 | - error_treat(mb_param, ret, s_error); | ||
| 356 | - } | ||
| 357 | - } else { | ||
| 358 | - /* In TCP, the modbus CRC is not present (see HDLC level) */ | 341 | + crc_calc = crc16(msg, msg_length - 2); |
| 342 | + crc_received = (msg[msg_length - 2] << 8) | msg[msg_length - 1]; | ||
| 343 | + | ||
| 344 | + /* Check CRC of msg */ | ||
| 345 | + if (crc_calc == crc_received) { | ||
| 359 | ret = 0; | 346 | ret = 0; |
| 347 | + } else { | ||
| 348 | + char s_error[64]; | ||
| 349 | + sprintf(s_error, | ||
| 350 | + "invalid crc received %0X - crc_calc %0X", | ||
| 351 | + crc_received, crc_calc); | ||
| 352 | + ret = INVALID_CRC; | ||
| 353 | + error_treat(mb_param, ret, s_error); | ||
| 360 | } | 354 | } |
| 361 | 355 | ||
| 362 | return ret; | 356 | return ret; |
| @@ -585,6 +579,10 @@ int receive_msg(modbus_param_t *mb_param, | @@ -585,6 +579,10 @@ int receive_msg(modbus_param_t *mb_param, | ||
| 585 | if (mb_param->debug) | 579 | if (mb_param->debug) |
| 586 | printf("\n"); | 580 | printf("\n"); |
| 587 | 581 | ||
| 582 | + if (mb_param->type_com == RTU) { | ||
| 583 | + check_crc16(mb_param, msg, *msg_length); | ||
| 584 | + } | ||
| 585 | + | ||
| 588 | /* OK */ | 586 | /* OK */ |
| 589 | return 0; | 587 | return 0; |
| 590 | } | 588 | } |
| @@ -611,11 +609,6 @@ static int modbus_check_response(modbus_param_t *mb_param, | @@ -611,11 +609,6 @@ static int modbus_check_response(modbus_param_t *mb_param, | ||
| 611 | ret = receive_msg(mb_param, response_length_computed, | 609 | ret = receive_msg(mb_param, response_length_computed, |
| 612 | response, &response_length); | 610 | response, &response_length); |
| 613 | if (ret == 0) { | 611 | if (ret == 0) { |
| 614 | - /* Check message */ | ||
| 615 | - ret = check_crc16(mb_param, response, response_length); | ||
| 616 | - if (ret != 0) | ||
| 617 | - return ret; | ||
| 618 | - | ||
| 619 | /* Good response */ | 612 | /* Good response */ |
| 620 | switch (response[offset + 1]) { | 613 | switch (response[offset + 1]) { |
| 621 | case FC_READ_COIL_STATUS: | 614 | case FC_READ_COIL_STATUS: |
| @@ -644,13 +637,16 @@ static int modbus_check_response(modbus_param_t *mb_param, | @@ -644,13 +637,16 @@ static int modbus_check_response(modbus_param_t *mb_param, | ||
| 644 | 637 | ||
| 645 | } else if (ret == COMM_TIME_OUT && | 638 | } else if (ret == COMM_TIME_OUT && |
| 646 | response_length == offset + 3 + mb_param->checksum_length) { | 639 | response_length == offset + 3 + mb_param->checksum_length) { |
| 647 | - /* Optimisation allowed because exception response is | 640 | + /* Optimization allowed because exception response is |
| 648 | the smallest trame in modbus protocol (3) so always | 641 | the smallest trame in modbus protocol (3) so always |
| 649 | - raise an timeout error */ | ||
| 650 | - /* CRC */ | ||
| 651 | - ret = check_crc16(mb_param, response, response_length); | ||
| 652 | - if (ret != 0) | ||
| 653 | - return ret; | 642 | + raise a timeout error */ |
| 643 | + | ||
| 644 | + /* CRC must be checked here (not done in receive_msg) */ | ||
| 645 | + if (mb_param->type_com == RTU) { | ||
| 646 | + ret = check_crc16(mb_param, response, response_length); | ||
| 647 | + if (ret != 0) | ||
| 648 | + return ret; | ||
| 649 | + } | ||
| 654 | 650 | ||
| 655 | /* Check for exception response. | 651 | /* Check for exception response. |
| 656 | 0x80 + function is stored in the exception | 652 | 0x80 + function is stored in the exception |
| @@ -925,9 +921,6 @@ int modbus_listen(modbus_param_t *mb_param, uint8_t *query, int *query_length) | @@ -925,9 +921,6 @@ int modbus_listen(modbus_param_t *mb_param, uint8_t *query, int *query_length) | ||
| 925 | int ret; | 921 | int ret; |
| 926 | 922 | ||
| 927 | ret = receive_msg(mb_param, MSG_LENGTH_UNDEFINED, query, query_length); | 923 | ret = receive_msg(mb_param, MSG_LENGTH_UNDEFINED, query, query_length); |
| 928 | - if (ret == 0) { | ||
| 929 | - ret = check_crc16(mb_param, query, *query_length); | ||
| 930 | - } | ||
| 931 | 924 | ||
| 932 | return ret; | 925 | return ret; |
| 933 | } | 926 | } |