3.0.0 #658
3.0.0 #658
Conversation
Update from ubuntu xenial(16.04) to bionic(18.04) Mentioned issue in #544
Try to make python 3.7 standard on osx
Drop python 2 from travis.sh Try fixing osx to get python installed...
…nted python3 while installing package dependencies
Drop python2 support in travis
Use async/await keywords in test_server_asyncio.py
Set "strict" to False since it mailfunctions
Update sync.py
This patch adds server requiring client's certificate feature which is mentioned in the 6th step CertificateRequest to 9th step VerifyClientCertSig in Table 5 TLS Full Handshake Protocol of MODBUS/TCP Security Protocol Specification [1]. This patch implements the feature within both sync and async_io version. * Server side: Add an optional argument "reqclicert" of StartTlsServer(). So, users can force server require client's certificate for TLS full handshake, or according to the SSL Context's original behavior [2]. * Client side: Add optional arguments "certfile" and "keyfile" for replying, if the server requires client's certificate for TLS full handshake. Besides, also add an optional argument "password" on both server and client side for decrypting the private keyfile. This fixes part of #606 [1]: http://modbus.org/docs/MB-TCP-Security-v21_2018-07-24.pdf [2]: https://docs.python.org/3/library/ssl.html#ssl.SSLContext.verify_mode
|
If you think some of the changes from the PR's are not merged correctly or have been overidden by subsequent merges, please raise a new PR against |
|
@starnight please take a look in to this PR |
|
Have a PR #661 to fix the conflicts. @dhoomakethu please take a look. |
Developers add/fix features at the same time, then produce the conflicts in pymodbus' TLS module. This patch tries to fix the conflicts.
|
SonarCloud Quality Gate failed.
|
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
| @@ -103,11 +108,12 @@ def async_io_factory(port=None, framer=None, **kwargs): | |||
|
|
|||
| client = AsyncioModbusSerialClient(port, proto_cls, framer, loop, **kwargs) | |||
| coro = client.connect() | |||
There was a problem hiding this comment.
This coroutine is never awaited if the following is true: loop.is_running() and loop is asyncio.get_event_loop()
There was a problem hiding this comment.
Proposed fixes if you create from within another coroutine,- option 3 seems the cleanest!
option 1
inside the factory, create the task and return
coro = client.connect()
if loop.is_running() and loop is asyncio.get_event_loop():
asyncio.create_task(coro)
return loop, client
the the caller should just pause its execution to allow the connect schedule on the event loop to complete
async def main():
_loop, client = AsyncSerialClient(...)
asyncio.sleep(0) # this allows this excution to wait
option 2
from the factory, return the coro and client (instead of the loop - here the signature is different)
coro = client.connect()
if loop.is_running() and loop is asyncio.get_event_loop():
return coro, client
the the caller you can then await the coro
async def main():
coro, client = AsyncSerialClient(...)
await coro
option 3
inside the factory, dont create the coro
# coro = client.connect()
if loop.is_running() and loop is asyncio.get_event_loop():
return coro, client
coro = client.connect()
the the caller you can then await client.connect()
async def main():
loop, client = AsyncSerialClient(...)
await client.connect()
There was a problem hiding this comment.
The problem with asyncio code is not most of the users will be knowing how to use it and end up creating more tickets. I will see what could be done. Thanks for the suggestion.
There was a problem hiding this comment.
Indeed, that makes option 1&2 above rather cryptic.
My suggestion would be NOT to connect during the creation, but always leave it to the user to call client.connect()
Then they can do either one of the following, depending on where they start it from
loop, client = AsyncSerialClient(...)
# Option a - Loop has yet to start
asyncio.run_until_complete(client.connect())
# Option b - if the loop is already running and they are inside a coroutine/async def function
await client.connect()
# Option c - from python 3.7 replaces the ensure_future code currently there..
# although this works from a running coroutine, you might have to await it, so awaiting the client.connect (option a) is better)
asyncio.create_task(client.connect())
There was a problem hiding this comment.
I think #558 was an attempt to fix this for TCP. In that case they also left client.connect() to the caller, but this was not clearly mentioned in the PR
Also, you really want to same interface, irrespective of where you call things from. So believe splitting out the connect is the way to go. Let me know if you want me to do a PR in the 3.0.0 branch (its not clear when/if you will merge this PR?)
There was a problem hiding this comment.
That's a good question, it was supposed to be merged and available a long time ago but due to time constraints and the pandemic this is getting nowhere. I have no idea of the timeline but I am hopeful to get a 3.x release by the end of year.
* Replace distutils.version.Looseversion with pcg_resources.parse_version * Replace distutils.core.Command with setuptools.Command
|
SonarCloud Quality Gate failed.
|
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
This PR was closed because it has been stalled for 10 days with no activity. |
This PR contains all the existing PR's intended for 3.x branch.
This PR will be held open against the other PR's
Closes