Cloud-Automation / node-modbus

Modbus TCP Client/Server implementation for Node.JS
456 stars 169 forks source link

ModbusRTUClientRequestHandler "close" event #283

Closed kollerDominik closed 1 month ago

kollerDominik commented 3 years ago

Hi, the ModbusTCPClientReqeuestHandler listens for the "close" event of the socket in the constructor and binds the _onClose method: this._socket.on('close', this._onClose.bind(this)) Is there a special reason why this is not the case in the ModbusRTUClientRequestHandler? Because what I'm currently experiencing is that I just get a request timeout error if the serial connection is closed and if the serial connection is reconnected I sometimes still receive request timeout errors before the requests are finished successfully again. If the same line as for the ModbusTCPClientRequestHandler is added, errors like "OutOfSync" or "Offline" are shown (which should be fine) and the requests are successful nearly immediately after the serial connection is restablished. But maybe I'm overlooking something important, so therefore I would ask for some clarification. Thanks.

stefanpoeter commented 3 years ago

Actually I am not sure why there is no link to the close event on the serialport module. I remember that there was no close event when I implemented that but I am not sure. Just checked the serialport API and there is actually a close event. Does anyone know if the open/close state on an RTU connection is more that an internal state on the serialport module? @alexbuczynsky maybe?

kollerDominik commented 3 years ago

Hi, is there something new on that issue @stefanpoeter @alexbuczynsky?

alexbuczynsky commented 3 years ago

Based on the API docs here: https://serialport.io/docs/7.x.x/api-stream#close

The close event is emitted when the port is closed. In the case of a disconnect, it will be called with a Disconnect Error object (err.disconnected == true). In the event of an error while closing (unlikely), an error event is triggered.

kollerDominik commented 3 years ago

@stefanpoeter @alexbuczynsky so there should be a link to the close event?

stefanpoeter commented 2 years ago

@kollerDominik sorry for the late reply. Do you care about handling this in a pull request?

kollerDominik commented 2 years ago

Yes we could handle this in a pull request