search

jacobschaer / python-doipclient

Pure Python ISO 13400 Client
MIT License
151 stars 50 forks source link

The A_PROCESSING_TIME in constants.py is not effective when using a TCP connection？ #45

Open mixc0lumn opened 9 months ago

mixc0lumn commented 9 months ago

In _connect function of client.py, self._tcp_sock.settimeout(A_PROCESSING_TIME)is set after self._tcp_sock.connect((self._ecu_ip_address, self._tcp_port)), which make A_PROCESSING_TIME is not effective. Below is the fix suggestion, where the position of these two lines change: 20231123-100546

jacobschaer commented 9 months ago

The order was intentional - I can't think of a good reason for a user to want to deal with timeouts on first connect, and I would imagine that many ECU's have trouble with the initial connection as there's more packets back/forth and setup. The OS will still have a timeout in the overall TCP layer as well as any timeouts with IP resolution, etc. I would accept a patch to allow the client constructor to accept an optional connect_timeout if you have a use case for it though.

mixc0lumn commented 9 months ago

Thanks, the patch is useful for us if allowed. We need to deal with so many ECUs with uncertain state (connected or not). The only possible way is to send a doip package to the specific logical address and observe the response. The default timeout will be too long to accept if the timeouts on first connect is not optional in doipclient : )