-
Notifications
You must be signed in to change notification settings - Fork 184
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
Increase Discovery Cohesion #199
Conversation
…moving the network-level business logic to the Connection class, where it usually takes place.
…class to the Connection class. Decided to avoid the unnecessary import of Device by passing in procedural parameter to the interface.
* increase_cohesion_discovery: Add the discovery network-level business logic previously in the PLC class to the Connection class. Decided to avoid the unnecessary import of Device by passing in procedural parameter to the interface. Remove the socket module coupling and increase PLC class cohesion by moving the network-level business logic to the Connection class, where it usually takes place.
Thanks for the PR, Is this really a breaking change? Are any tests failing with tox? |
Not really breaking, but checked that box due to functionality (routing) change. Took me a bit to get internet/system setup with everything needed to run all Python versions. Python versions: Result: I needed to add the tags: Nemesis array, and Str1-50, to the program. But the video and readme were very descriptive in getting a test environment setup. |
Nice work. I'll update the setup files |
…eded by the unit tests.
* update_clx_controller_tags: I updated the clx control tag file to include nemesis and str tags needed by the unit tests.
I went ahead and updated the controller scope tags for you guys. |
Good work! When I run tox, I usually just run latest major versions on 2 and 3. That way you don't have to install every single python version. |
Short description of change
Software architecture change to reduce unnecessary coupling and increase class cohesion.
lgx_comm.py
Add the discovery network-level business logic previously in the PLC class to the Connection class.
Decided to avoid the unnecessary import of Device by passing in procedural parameter to the interface.
eip.py
Remove the socket module coupling and increase PLC class cohesion by moving the network-level business
logic to the Connection class, where it usually takes place.
Types of changes
What is the change?
lgx_comm.py
Add the discovery network-level business logic previously in the PLC class to the Connection class.
Decided to avoid the unnecessary import of Device by passing in procedural parameter to the interface.
eip.py
Remove the socket module coupling and increase PLC class cohesion by moving the network-level business
logic to the Connection class, where it usually takes place.
What does it fix/add?
Software architecture change to reduce unnecessary coupling and increase class cohesion.
Test Configuration
I ran example 20_discover_devices without issue.
1756-L84E/B
32.014
0.8.6
3.7.7
Winblows Server 2016 Version 10.0.14393 Build 14393