Modbus

Fix bug: RTU layer must distinguish between response and request frames

2022-10-16, Mario de Sousa
a2bb438a215b
Parents 74e9ff734cea
Children a0f1c6c9de70
Fix bug: RTU layer must distinguish between response and request frames
7 files changed, 80 insertions(+), 9 deletions(-)
  • +1 -0
    mb_ascii.c
  • +6 -0
    mb_layer1.h
  • +16 -2
    mb_layer1_prototypes.h
  • +4 -4
    mb_master.c
  • +51 -3
    mb_rtu.c
  • +1 -0
    mb_slave.c
  • +1 -0
    mb_tcp.c
    • --- a/mb_ascii.c Sun Oct 16 07:10:20 2022 +0100
    +++ b/mb_ascii.c Sun Oct 16 14:25:06 2022 +0100
    @@ -1452,6 +1452,7 @@
    int modbus_ascii_read(int *nd,
    u8 **recv_data_ptr,
    u16 *transaction_id,
    + mb_frame_type_t requested_frame_type, /* ignored by this layer ! */
    const u8 *send_data,
    int send_length,
    const struct timespec *recv_timeout) {
    --- a/mb_layer1.h Sun Oct 16 07:10:20 2022 +0100
    +++ b/mb_layer1.h Sun Oct 16 14:25:06 2022 +0100
    @@ -38,6 +38,12 @@
    #define MAX_WRITE_REGS 123 /* Function 0x10 */
    +/* Data type required for modbus_read().
    + * Has to be declared here as mb_layer1_prototypes.h is included several times!
    + */
    +typedef enum {MB_req_frame, MB_resp_frame, MB_any_frame} mb_frame_type_t;
    +
    +
    /* Declare TCP layer1 functions */
    #define modbus_write modbus_tcp_write
    #define modbus_read modbus_tcp_read
    --- a/mb_layer1_prototypes.h Sun Oct 16 07:10:20 2022 +0100
    +++ b/mb_layer1_prototypes.h Sun Oct 16 14:25:06 2022 +0100
    @@ -21,8 +21,6 @@
    */
    -
    -
    /* write a modbus frame */
    /* WARNING: when calling this function, the *frame_data buffer
    * must be allocated with an extra *extra_bytes
    @@ -73,6 +71,21 @@
    * converters, this functionality is essential as not all these converters
    * are capable of not echoing back the sent data.
    * These parameters are ignored when using TCP!
    + *
    + * requested_frame_type: one of -> MB_req_frame, MB_resp_frame, MB_any_frame
    + * the type of frame we should search for (request, response, or any)
    + * NOTE: only used by the RTu layer. Other layers simply ignore this parameter.
    + * NOTE: whatever the type of frame, searching for error frames is
    + * always enabled (i.e. this function may always return that it found
    + * an error frame, whatever frame type it was told to look for)
    + * NOTE:
    + * This is needed because the RTU protocol may confuse some valid
    + * response frames with valid query frames (e.g. the response to
    + * read registers may contain data with values that just so happens
    + * to match the correct CRC of the read registers query packet, making
    + * it impossible to distinguish the beginning of a read registers
    + * response to a read registers query). We only give this priority
    + * if it is possible to have this con
    */
    /* RETURNS: number of bytes read
    @@ -82,6 +95,7 @@
    int modbus_read(int *nd, /* node descriptor */
    u8 **recv_data_ptr,
    u16 *transaction_id,
    + mb_frame_type_t requested_frame_type,
    const u8 *send_data,
    int send_length,
    const struct timespec *recv_timeout);
    --- a/mb_master.c Sun Oct 16 07:10:20 2022 +0100
    +++ b/mb_master.c Sun Oct 16 14:25:06 2022 +0100
    @@ -221,7 +221,7 @@
    * response we are waiting for could be coming 'any minute now'.
    */
    do {
    - response_length = modbus_read(&ttyfd, response_packet, &recv_transaction_id,
    + response_length = modbus_read(&ttyfd, response_packet, &recv_transaction_id, MB_resp_frame,
    query_packet, query_length, response_timeout);
    /* TIMEOUT condition */
    @@ -360,9 +360,9 @@
    response_packet += 3;
    /* handle groups of 4 bytes... */
    for(i = 0, dest_pos = 0; i + 3 < byte_count; i += 4, dest_pos++)
    - dest[dest_pos] = response_packet[i]
    - + response_packet[i+1]*0x100
    - + response_packet[i+2]*0x10000
    + dest[dest_pos] = response_packet[i]
    + + response_packet[i+1]*0x100
    + + response_packet[i+2]*0x10000
    + response_packet[i+3]*0x1000000;
    /* handle any remaining bytes... begining with the last! */
    if (i < byte_count) dest[dest_pos] = 0;
    --- a/mb_rtu.c Sun Oct 16 07:10:20 2022 +0100
    +++ b/mb_rtu.c Sun Oct 16 14:25:06 2022 +0100
    @@ -1287,6 +1287,12 @@
    /* Search for a valid frame in the current data.
    * If no valid frame is found, then we return -1.
    + *
    + * requested_frame_type: one of -> MB_req_frame, MB_resp_frame, MB_any_frame (default)
    + * the type of frame we should search for (request, response, or any)
    + * NOTE: whatever the type of frame, searching for error frames is always enabled
    + * (i.e. this function may always return that it found an error frame,
    + * whatever frame type it was told to look for)
    *
    * NOTE: Since frame verification is done by calculating the CRC, which is rather
    * CPU intensive, and this function may be called several times with the same,
    @@ -1295,6 +1301,7 @@
    */
    static int search_for_frame(u8 *frame_data,
    int frame_data_length,
    + mb_frame_type_t requested_frame_type,
    int *search_history) {
    int query_length, resp_length;
    u8 function_code;
    @@ -1303,6 +1310,17 @@
    #define SFF_HIST_NO_RESPONSE_FRAME 0x02
    #define SFF_HIST_NO_FRAME (SFF_HIST_NO_RESPONSE_FRAME + SFF_HIST_NO_QUERY_FRAME)
    + if (requested_frame_type == MB_req_frame)
    + /* only search for request frames => don't search for response frames */
    + /* we simply mark the flag indicating not to search for response frames */
    + *search_history |= SFF_HIST_NO_RESPONSE_FRAME;
    +
    + if (requested_frame_type == MB_resp_frame)
    + /* only search for response frames => don't search for request frames */
    + /* we simply mark the flag indicating not to search for request frames */
    + *search_history |= SFF_HIST_NO_QUERY_FRAME;
    +
    +
    if ((*search_history == SFF_HIST_NO_FRAME) ||
    (frame_data_length < MIN_FRAME_LENGTH) ||
    (frame_data_length > MAX_RTU_FRAME_LENGTH))
    @@ -1438,6 +1456,12 @@
    /* A function to read a valid frame off the rtu bus.
    *
    + * requested_frame_type: one of -> MB_req_frame, MB_resp_frame, MB_any_frame (default)
    + * the type of frame we should search for (request, response, or any)
    + * NOTE: whatever the type of frame, searching for error frames is always enabled
    + * (i.e. this function may always return that it found an error frame,
    + * whatever frame type it was told to look for)
    + *
    * NOTES:
    * - The returned frame is guaranteed to be a valid frame.
    * - The returned length does *not* include the CRC.
    @@ -1516,7 +1540,9 @@
    static inline int read_frame(nd_entry_t *nd_entry,
    u8 **recv_data_ptr,
    struct timespec *end_time,
    - u8 *slave_id)
    + u8 *slave_id,
    + mb_frame_type_t requested_frame_type
    + )
    {
    /* temporary variables... */
    fd_set rfds;
    @@ -1553,6 +1579,7 @@
    */
    frame_length = search_for_frame(lb_data(&recv_buf->data_buf),
    lb_data_count(&recv_buf->data_buf),
    + requested_frame_type,
    &recv_buf->frame_search_history);
    if (frame_length > 0)
    /* We found a valid frame! */
    @@ -1625,6 +1652,7 @@
    *-----------------------*/
    frame_length = search_for_frame(lb_data(&recv_buf->data_buf),
    lb_data_count(&recv_buf->data_buf),
    + requested_frame_type,
    &recv_buf->frame_search_history);
    if (frame_length > 0)
    /* We found a valid frame! */
    @@ -1765,6 +1793,20 @@
    * ignore_echo == 0, then the first valid frame read off
    * the bus is returned.
    *
    + * requested_frame_type: one of -> MB_req_frame, MB_resp_frame, MB_any_frame (default)
    + * the type of frame we should search for (request, response, or any)
    + * NOTE: whatever the type of frame, searching for error frames is
    + * always enabled (i.e. this function may always return that it found
    + * an error frame, whatever frame type it was told to look for)
    + * NOTE:
    + * This is needed because the RTU protocol may confuse some valid
    + * response frames with valid query frames (e.g. the response to
    + * read registers may contain data with values that just so happens
    + * to match the correct CRC of the read registers query packet, making
    + * it impossible to distinguish the beginning of a read registers
    + * response to a read registers query). We only give this priority
    + * if it is possible to have this con
    + *
    * return value: The length (in bytes) of the valid frame,
    * -1 on error
    * -2 on timeout
    @@ -1773,6 +1815,7 @@
    int modbus_rtu_read(int *nd,
    u8 **recv_data_ptr,
    u16 *transaction_id,
    + mb_frame_type_t requested_frame_type,
    const u8 *send_data,
    int send_length,
    const struct timespec *recv_timeout) {
    @@ -1786,6 +1829,10 @@
    if (nd == NULL)
    return -1;
    +#ifdef DEBUG
    + fprintf(stderr, "modbus_rtu_read(fd=%d) called...\n", *nd);
    +#endif
    +
    if (recv_data_ptr == NULL)
    recv_data_ptr = &local_recv_data_ptr;
    @@ -1840,7 +1887,7 @@
    */
    iter = 0;
    - while ((res = recv_length = read_frame(nd_entry, recv_data_ptr, ts_ptr, slave_id)) >= 0) {
    + while ((res = recv_length = read_frame(nd_entry, recv_data_ptr, ts_ptr, slave_id, requested_frame_type)) >= 0) {
    if (iter < INT_MAX) iter++;
    if ((send_length <= 0) || (nd_entry->ignore_echo == 0))
    @@ -1853,7 +1900,8 @@
    *
    * We must only do this for the first frame we read. Subsequent
    * frames are guaranteed not to be the previously sent frame
    - * since the modbus_rtu_write() resets the recv buffer.
    + * since the modbus_rtu_write() resets the recv buffer
    + * (_before_ sending out the message so as to avoid race conditions).
    * Remember too that valid modbus responses may be exactly the same
    * as the request frame!!
    */
    --- a/mb_slave.c Sun Oct 16 07:10:20 2022 +0100
    +++ b/mb_slave.c Sun Oct 16 14:25:06 2022 +0100
    @@ -784,6 +784,7 @@
    byte_count = modbus_read(&nd, /* node descriptor */
    &query_packet, /* u8 **recv_data_ptr, */
    &transaction_id, /* u16 *transaction_id, */
    + MB_req_frame, /* mb_frame_type_t */
    NULL, /* const u8 *send_data, */
    0, /* int send_length, */
    NULL /* wait indefenitely */ /* const struct timespec *recv_timeout); */
    --- a/mb_tcp.c Sun Oct 16 07:10:20 2022 +0100
    +++ b/mb_tcp.c Sun Oct 16 14:25:06 2022 +0100
    @@ -1249,6 +1249,7 @@
    int modbus_tcp_read(int *nd, /* node descriptor */
    u8 **recv_data_ptr,
    u16 *transaction_id,
    + mb_frame_type_t requested_frame_type, /* ignored by this layer ! */
    const u8 *send_data, /* ignored ! */
    int send_length, /* ignored ! */
    const struct timespec *recv_timeout) {