search

gijzelaerr / python-snap7

A Python wrapper for the snap7 PLC communication library
http://python-snap7.readthedocs.org/
MIT License
643 stars 246 forks source link

implement `__main__.py` #407

Closed Cube707 closed 1 year ago

Cube707 commented 1 year ago

It would be helpful and more pythonic to implement a __main__.py. This would allow to start the server like in many other packages using:

python -m snap7

even better, by implementing server and client as proper sub-packages of snap7 it would allow for the even more verbose syntax: (however, this might break the current api and might requirte a major version change)

python -m snap7.server
python -m snap7.client

This would also allow for entrypoints, enableing pip to create binarys on install and making the command avalable as snap7-server or similar.

Please assigne me this issue if it is of interest to you and I should implement it

gijzelaerr commented 1 year ago

i'm open to this proposal, but I prefer not to introduce API changes. how would you supply the arguments to starting the client that way?

Cube707 commented 1 year ago

I have tested this a little and it seems to work without requrirering API changes (will investigate further)

Suppling arguments is very easy, just slap them on the end. For example, just moving bin/snap7-server.py -> server2/__main__.py is enough to use this: (the 2 is just to avoid naming collisions)

python -m snap7.server2 path/to/snap7.dll

see the python docs on how the default http.server can be used. This is what I am talking about

If you are interesed I can open a PR for further discussion.

gijzelaerr commented 1 year ago

Sure, a PR is always welcome. I would not make the path to the DLL a function argument though since it is already configurable by setting the PATH environment variable and in general not required to be supplied.

Cube707 commented 1 year ago

I would not make the path to the DLL a function argument

Than I missunderstood what the current implementation is doing with its first argument. I agree that this should definitely not be a required argument (maybe an optional though?). It was just an example...