Trio support #583
Trio support #583
Conversation
|
|
though these may not be super applicable even
(sure, pieces may come back later such as when supporting RTU)
|
Mostly just for linkage, I have a quick SunSpec layer running on top of this PR over at altendky/ssst#19. That code will likely move out to it's own project once this all settles down a bit. Anyways, this functions in a basic scenario. Maybe I'll take another pass at this in the next couple days. |
|
SonarCloud Quality Gate failed.
|
test/test_server_trio.py
Outdated
|
|
||
|
|
||
| @pytest.mark.trio | ||
| async def test_read_holding_registers(trio_tcp_client, trio_tcp_server): |
There was a problem hiding this comment.
These tests looks more like functional tests than unit tests. I can move this to functional tests later to #2 when functional tests actually are in place. Also the test seems to be failing on mac osx with trio.TrioInternalError: internal error in Trio - please file a bug!.
There was a problem hiding this comment.
I added a bunch more unit-y tests as well.
|
@altendky sorry for picking on this late, can you also add an example for trio client? |
| proto_cls = kwargs.get("proto_cls", None) | ||
| client = init_tcp_client(proto_cls, host, port) | ||
|
|
||
| return client |
There was a problem hiding this comment.
This is inconsistent with other factories which return two values.
There was a problem hiding this comment.
They do return two values, but I don't see the consistency in the meaning of those two values. Can you help me understand what sort of thing the first value should be and what the second should be? Then I can try to map that into the Trio stuff.
Twisted: protocol / deferred
Tornado: protocol / future
asyncio: event loop / client
Trio: ? / ?
I can see the similarity between Twisted and Tornado. I don't see how asyncio fits. Apparently when I coded this a couple months ago I didn't see what other useful thing I could return in the Trio case. We'll see if I can find something now.
There was a problem hiding this comment.
Relatedly, the following docstring seems to only get asyncio right.
|
I was going to come back to this after the CI was redone in #592 so I could have functional CI while working on this. Adding examples is already in the list in the first post explaining why this is a draft PR. I'll address the other concerns when working on this again. Thanks for the feedback. |
|
(hopefully I didn't already comment on this and forget...) @dhoomakethu, any interest in enabling read the docs builds for each PR? If you go to https://readthedocs.org/dashboard/pymodbus/advanced/ there should be a checkbox labeled |
|
Welp, @dhoomakethu I think this is ready for at least a second pass review. I understand that I may not be testing sensibly or need a pile more docs or... So, don't feel obliged to dig into the details if something big is out of line. Anyways, let me know what you think. Side note, I took this on so I could develop a Trio library for communication with SunSpec devices. That ended up being sundog. |
|
@altendky it will be sometime before I go through this. Apologies in advance and thanks for the efforts. |
|
I understand. Took me awhile to get back to it myself and I've got a few other projects I'm playing catch up on. No worries. |
|
SonarCloud Quality Gate failed.
|
|
The PyPy issues should get fixed by #637. |
|
SonarCloud Quality Gate failed.
|
|
As discussed on the issue, I close this for now. |
#581
Draft for: