New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Socket not closing on upgrade from v2.4.0 #39
Comments
I don't see an issue here. The socket is not automatically closed in version 2.4.0 either. The data from other clients was buffered until some of the read/write methods were called (see #19). The debug output shows that unwanted by Client_2 data is discarded i.e. Client_2 will receive exactly the data which was requested by it. The connection will be closed if the client is terminated even if the disconnect method is not called. |
Hi @connesha, Thanks for the bug report. It does appear based on the DEBUG logs provided that the v2.5.0 client is correctly discarding the spurious responses which were intended for the v2.4.0 polling client. I would agree that this appears be a regression between v2.4.0 and v2.5.0 because of the fact that you had to amend your script. The issue is of course the fact that the data loggers appear to send a reply for any client to all open connections. I hadn't quite considered your use case when doing my own pre-release testing. I'm not sure it warrants a version bump to v3.0.0, but I have added the |
Hi @connesha, I am summarising the details of our email discussion here for posterity;
Thanks for this test case. I was able to recreate your results. Prior to #33, the socket was implicitly closed when the pysolarmanv5 object was dereferenced. This meant that one could just re-instantiate another instance and it worked. Following #33, the socket is still referenced in the worker thread, with the result that when re-instantiated, the old object (and socket) remain. It would appear that the Solarman data loggers permit six(?) simultaneous connections, so the above example failed consistently for me on the same iteration. Depending on the behavior of the calling application, this is a breaking pysolarmanv5 change for those upgrading from v2.4.0 to v2.5.0. I am currently of the view to withdraw v2.5.0 and re-release as v3.0.0, highlighting the new |
OK I'm wrong in my previous comment. The socket is automagically closed when the object is dereferenced before #33 and the same behavior hardly can be achieved with the background thread. |
I've just released v3.0.0. Thanks both for your help! |
v2.4.0 did not have a disconnect method (it was added in v2.5.0)
Consider 2 separate clients are using pysolarmanv5 to access the same datastick.
Client_1 runs in a loop, polling a few registers every few seconds.
Client_2 does some infrequent reads/writes.
Both clients were previously on v2.4.0 and working well.
On upgrading Client_2 to v2.5.0, it can be seen that the socket is not getting closed, as the debug logs show the following output continuously until Client_2 is terminated:
DEBUG:pysolarmanv5.pysolarmanv5:[POLL-DISCARDED] RECD: a5 1f 00 10 15 00 2e 8f 0a ...
DEBUG:pysolarmanv5.pysolarmanv5:[POLL-DISCARDED] RECD: a5 17 00 10 15 00 2f 8f 0a ...
Simplified Client_2:
modbus = PySolarmanV5( "192.168.1.24", 123456789, port=8899, mb_slave_id=1, verbose=True )
print(modbus.read_input_registers(register_addr=33022, quantity=1))
sleep(100)
Calling disconnect() in Client_2 appears to resolve the issue, however this is a compatibility issue as this method did not exist in previous versions and now appears to be required.
I haven't looked to see if its the frequent traffic from Client_1 that is keeping the Client_2 socket alive.
Also consider, disconnect() is not described in the docs or called in the sample
thank you
The text was updated successfully, but these errors were encountered: