-
Notifications
You must be signed in to change notification settings - Fork 4
Fix #2 server not starting on python 3.8 #5
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
Conversation
Hmm, it's still not working for me after
Edit: well, it works with |
@alexrudd2 do not use |
I did some more testing with both The namespace problem is still there. However, imports in the other files are pulling in the "old" REPL code from the |
Can you try a fresh virtual env and run just |
|
|
@alexrudd2 can you give this branch a try |
Yes, I did. the
|
@alexrudd2 I see what you are pointing at, it works fine for client but for server its picking from the |
From your asciicinema sharing, I believe you are still importing code from repl/server/cli.py
async def run_repl(server):
"""Run repl server."""
raise NotImplementedError # <-- ADD THIS
await main(server) If the code is importing from |
OK, I'm glad. It's difficult to explain :)
It is not about client versus server. it works fine only for |
@alexrudd2 I see your point, the simplest fix would be to go with your PR for this. |
Certain changes from this PR will be cherry picked once #4 is merged. |
pymodbus_repl/client/main.py
Outdated
|____| / ____\____|__ /\____/\____ | /\ |____|_ /\___ > __/|____/ | ||
\/ \/ \/ \/ \/ \/|__| | ||
v1.3.1 - {pymodbus_version} | ||
v0.1.1 - {pymodbus_version} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should go "backwards" with the version number here. Instead, probably better to change the whole package to 1.4.0
? or perhaps 2.0.0
to indicate a major break?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not understand why from __future__ import annotations
does not work as it should. However, your fix works for me.
Closes #2
from __future__ import annotations
.Works also on the other versions of python