Proxy - modbus_proxy function

[jaimealsilva]

Hello there.

Maybe you won't remember by now but about 4 months ago you rejected a pull request of mine because of formatting issues and also because all the changes were made on master instead of branches (I was a new git user). There were two main issues addressed on those changes:

1. Defective hardware that has echo enabled on the RS485 port.
2. Implementing a function to send requests from one context to another (a proxy).

From those features you seemed interested on the proxy one so here I'm sending you another pull request about that one (you can also grab the echo hardware stuff from my echohw branch, if interested).

I finally found the time this weekend to work on this project again and as you suggested I deleted the old fork an moved the changes to a new branch.

This function was developed to solve the following problem: I have a Single Board Computer (SBC) that acts as the master of a MODBUS/RS485 bus and also has an Ethernet port and I wanted to use it as a bridge between Ethernet and RS485 so that is possible to send MODBUS requests from Ethernet to the devices on the RS485 bus.

The following is the program I used to test the function, this one runs on the SBC to connect the Ethernet and RS485 contexts:

/*
 * File:   main.cpp
 */

#include <cstdlib>
#include <iostream>
#include <modbus/modbus.h>

using namespace std;

#ifndef MODBUS_PORT
#define MODBUS_PORT 1502
#endif

inline void debug_msg(string msg) {
#ifdef DEBUG
    cerr << msg << endl;
#endif
}

int main(int argc, char** argv) {
    modbus_t *tcp_ctx, *rtu_ctx;
    string ser_dev;

    if (argc > 1)
        ser_dev = argv[1];
    else
        ser_dev = "/dev/ttts2";

    /* TCP Context -> Server. IP string doesn't matter since 
       is changed to INADDR_ANY by modbus_tcp_listen */
    tcp_ctx = modbus_new_tcp("0.0.0.0", MODBUS_PORT);
    if (!tcp_ctx) {
        debug_msg("Can't create TCP context");
        exit(1);
    }
    /* RS485 Context */
    rtu_ctx = modbus_new_rtu(ser_dev.c_str(), 38400, 'E', 8, 1);
#ifdef DEBUG
    modbus_set_debug(tcp_ctx, 1);
    modbus_set_debug(rtu_ctx, 1);
#endif

    int socket = modbus_tcp_listen(tcp_ctx, 1);
    if (socket < 0) {
        debug_msg("Couldn't listen");
        exit(1);
    }
    for (;;) {

        /* Connect to slave side */
        if (modbus_connect(rtu_ctx) < 0) {
            debug_msg("Can't connect to RTU side");
            exit (1);
        }

        debug_msg("Waiting connection ...");
        int in_socket = modbus_tcp_accept(tcp_ctx, &socket);
        if (in_socket < 0 ) {
            debug_msg("Connect failed");
            exit(1);
        }

        for (;;) {
            debug_msg("Reading request from TCP/IP ...");
            uint8_t tcp_buf[MODBUS_TCP_MAX_ADU_LENGTH];
            int tcp_bs = modbus_receive(tcp_ctx, tcp_buf);
            if (tcp_bs < 0)
                break;

            // Send request to RTU slave
            tcp_bs = modbus_proxy(tcp_ctx, rtu_ctx, tcp_buf, tcp_bs);
        }

        debug_msg("Closing connections");
        modbus_close(tcp_ctx);
        modbus_close(rtu_ctx);
    }

    return 0;
}

Now from any other device connected to the Ethernet network I can read the devices on the RS485 by setting the slave address on the TCP context.

[stephane]

Does it still issues on this proxy code?

[sebastian-foss]

Proxy code is working well for all standard modbus queries. The problem was with non standard modbus frames (unknown to modbus_receive in libmodbus) with function codes 0x64 and 0x67.

In my opinion, to be 100% safe and eliminate potential SIGSEGV for unknown frames, it will be nice to add some check for l<0 in this calculation before memcpy:

int l = master_req_length - offset - master_ctx->backend->checksum_length;
//check for l<0 and if negative, return error/return from function
memcpy(buf + buf_length, master_req + offset, l);

But again, for standard modbus frames with known function codes works very well.

[jaimealsilva]

modbus_proxy, as any other function on the library, assumes it is receiving valid pointers and a valid request length. Adding the check proposed by sebastiangajda may cause the function to fail silently and then it will be harder to debug errors as the one he had in which non standard frames caused modbus_receive to fail.

I think is better to check, in the program that uses the library, if the frame received by modbus_receive is valid before passing it to modbus_proxy and, if it is not valid, then report that situation in the program's most convenient way.

[hanusek]

Hi, I have the same problem with the function - int tcp_bs = modbus_receive(tcp_ctx, tcp_buf);
I am writing code in C + +. I create an object, destroy object and create a new object and then the program is crashing. I noticed that the problem appears when this function.
Please help me.

void ModbusSlave::run()
{

    QMutex mutex;
    int i=0;

        ctx = modbus_new_tcp("0.0.0.0", 1506);

        modbus_set_debug(ctx, TRUE);
        mb_mapping = modbus_mapping_new(0, 0, 50, 0);
        if (mb_mapping == NULL) {
            fprintf(stderr, "Failed to allocate the mapping: %s\n",
                    modbus_strerror(errno));
            modbus_free(ctx);
            //return -1;
        }

        socket = modbus_tcp_listen(ctx, 1);
        if (socket < 0) {
            qDebug("Couldn't listen");
            exit(1);
        }


   for (;;) {



//       qDebug("Waiting connection ...");
//       // qDebug() <<"ModbusThread EHdevice ID:"<< this->getThreadID()<<"- Waiting connection ...";
       in_socket = modbus_tcp_accept(ctx, &socket);
       if (in_socket < 0 ) {
           qDebug("Connect failed");
           exit(1);
       }


       for (;;) {
          qDebug("Reading request from TCP/IP ...");


        uint8_t tcp_buf[MODBUS_TCP_MAX_ADU_LENGTH]; //OK
        tcp_bs = modbus_receive(ctx, query);  // <----- CRASH
            qDebug()<<"tcp_bs:"<<tcp_bs;


 //         if (tcp_bs < 0)
//          {
//               break;
//          }

//           mutex.lock();
//           mb_mapping->tab_registers[5]=i;
//           mutex.unlock();
//           i++;
//           modbus_reply(ctx, tcp_buf, tcp_bs, mb_mapping);

//           mutex.lock();
//           if (this->Stop==true) break;
//           mutex.unlock();



//           mutex.lock();
//            if (this->Stop==true) {
//                        modbus_mapping_free(mb_mapping);
//                        modbus_close(ctx);
//                        modbus_free(ctx);
//                        qDebug("ModbusSlave stop1");

//                        break;
//            }
//           mutex.unlock();
      }

       qDebug("Closing connections");
      // qDebug() <<"Thread ID:"<< this->getThreadID()<<"-Closing connections.";

//       mutex.lock();
//        if (this->Stop==true) {
//                    modbus_mapping_free(mb_mapping);
//                    modbus_close(ctx);
//                    modbus_free(ctx);
//                    qDebug("ModbusSlave stop2");

//                    break;
//        }
//       mutex.unlock();


   }


//  modbus_mapping_free(mb_mapping);
//  modbus_close(ctx);
//  modbus_free(ctx);
//  qDebug("ModbusSlave stop3");

}

[hanusek]

I solved this problem.
I've added the code :

#include <sys/types.h>
#include <sys/socket.h>

shutdown(socket,2);

;)

[desowin]

I tried this with latest libmodbus on Windows and it is working fine. The conflict is trivial to resolve (EXPORT was renamed to MODBUS_API).

Is there still any reason why it cannot be merged?

[stephane]

The PR hasn't be updated to handle non standard modbus frames and I can't read the code sample from @hanusek (#include lines are empty).
Unit tests and documentation are missing and I'm running out of time to do that these days.

[jasonfsmitty]

FYI, I'm using this patch for a project and was getting a segfault when testing with unit-test-server and unit-test-client. Seemed to be caused by modbus_report_slave_id() sending 4 bytes less than what modbus_proxy() expected. We fixed it by modifying modbus_proxy() to only call memcpy() if l > 0.

[stephane]

Anyone interested to update this PR?