Feature/add micro800 to plc init

[drbitboy]

Short description of change

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] I have read the docs/CONTRIBUTING.md document.
• [x] My code follows the code style of this project.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [x] I have read tests/README.md.
• [x] I have added tests to cover my changes.
• [?] All new and existing tests passed.

What is the change?

1) Add ,Micro800=False keyword to pylogix.PLC class, so Micro800 users can create the class within the single instantiation call and not require a subsequent comm,Micro800=True statement.

1.1) I used the **kwargs approach because it makes it easier down the road to do the same thing for other attributes of class pylogix.PLC, but I am not commited to it.

2) Add a technique to skip selected tests (maybe this exists in unittest already?). 2.1) Update tests/README.md 2.2) Add sample tests/plcConfig.py.template

What does it fix/add?

See above.

Test Configuration

No change, but the new code is tested.

• PLC Model
• PLC Firmware
• pylogix version
• python version
• OS type and version

[drbitboy]

Sorry I was not done, I had some more changes to make.

Fernando, I am willing to switch from ,**kwargs to ,Micro800=False if you prefer.

[dmroeder]

I also would prefer Micro800=False.

[TheFern2]

I think pylogix test files should remain the same unless you're adding a new test. It doesn't take long to run all the tests anyways and you never know when one change breaks something else. So is better to run the entire suite of tests.

I do like the addition of the template for the plcConfig.py, but without all the extra boolean flags.

[drbitboy]

The boolean flags are inactive by default, but they make it possible to run the tests without getting a lot of errors. For example, I do not have any Controllogix or Compactlogix, so I cannot run those tests. But I do have a Micro820, so I could develop tests for that.

[drbitboy]

I changed it to ,Micro800=False, and the new tests pass. Also, do you know why I am getting these warning or error when I run the tests?

/home/dad/dev/plcs/pylogix/pylogix/eip.py:915: ResourceWarning: unclosed <socket.socket fd=3, family=AddressFamily.AF_INET, type=SocketKind.SOCK_DGRAM, proto=0, laddr=('127.0.1.1', 51569)>
  s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
ResourceWarning: Enable tracemalloc to get the object allocation traceback

[TheFern2]

I do remember them seeing them with tox, I think is safe to ignore them.

A few suggestions to keep it clean:

• Delete all the extra testing variables on the template, we can just use Micro800 as a control flag.
• Try/Except isn't needed specially if you have pass, a simple if statement should suffice and use Micro800 on the if statement.

[TheFern2]

Also to conform with documentation if you want to go that extra mile, you could create another folder for micro800_setup tags, save the test program, and update tests/readme with pertinent instructions if different from clx so future users can run tests easily for micro800.

Are tag import/export similar to clx plc's? If not we might need those instructions too.

[drbitboy]

With the method I am using for disabling select tests, it has to be try/except; an [if] statement will throw an exception if the [plcConfig.skip_test_whatever] variable is not present. I could use hasattr(plcConfig,'skip_test_whatever') instead. e.g.

if hasattr(plcConfig,'skip_test_basic') and plcConfig.skip_test_basic: return

However, unittest has a built-in to handle skipping tests, see here, so we should probably use that instead. What do you think about using environment variables i.e.

@unittest.skipIf('SKIP_TEST_BASIC' in os.environ,'Skipped test_basic')

?

[TheFern2]

With the method I am using for disabling select tests, it has to be try/except; an [if] statement will throw an exception if the [plcConfig.skip_test_whatever] variable is not present. I could use hasattr(plcConfig,'skip_test_whatever') instead. e.g.

if hasattr(plcConfig,'skip_test_basic') and plcConfig.skip_test_basic: return

However, unittest has a built-in to handle skipping tests, see here, so we should probably use that instead. What do you think about using environment variables i.e.

@unittest.skipIf('SKIP_TEST_BASIC' in os.environ,'Skipped test_basic')

?

That's the main reason why I was suggesting to use isMicro800 boolean flag with an if statement plcConfig.py should remain unchanged, that variable should always be there in the config. However I think maybe a cleaner option is to leave PylogixTests.py as original before this PR and make change below:

if __name__ == "__main__":
    if not isMicro800:
         unittest.main()

Then sorta copy the same testing structure from that file into a new one Micro800Tests.py delete all uncessary tests that do not pertain to micro800. I think this might be a clear way of separation and clear unittests for both plc families.

if __name__ == "__main__":
    if isMicro800:
         unittest.main()

I am not close to my setup, but in theory when running tox that should take care of it, no?

[TheFern2]

I also like the @unittest.skipIf('SKIP_TEST_BASIC' in os.environ,'Skipped test_basic') if we want to really stay with one tests file, but I would say to use isMicro800 boolean flag for skipping unittests since that's the main factor, we should only skip tests that micro800 can't perform. What are your thoughts on these two options, two tests files vs one?

[drbitboy]

Definitely we should go with unittest-builtin @unittests.skip...(...) (and remove my silly idea with the plcConfig.skiptest... attributes ;-))

For tests of a physical Micro800: @unittest.skipIf(('SKIP_TEST_BASIC' in os.environ,'Skipped test_basic') or (not plcConfig.isMicro800))

For tests of Logix PLC: @unittest.skipIf(('SKIP_TEST_BASIC' in os.environ,'Skipped test_basic') or (plcConfig.isMicro800))

[TheFern2]

Definitely we should go with unittest-builtin @unittests.skip...(...) (and remove my silly idea with the plcConfig.skiptest... attributes ;-))

For tests of a physical Micro800: @unittest.skipIf(('SKIP_TEST_BASIC' in os.environ,'Skipped test_basic') or (not plcConfig.isMicro800))

For tests of Logix PLC: @unittest.skipIf(('SKIP_TEST_BASIC' in os.environ,'Skipped test_basic') or (plcConfig.isMicro800))

Yeah I believe that is the better option.

[drbitboy]

Do the decorators work in Python 2?

[drbitboy]

Done.

[drbitboy]

Done. Also corrected some typos.

[TheFern2]

Can you also add micro8xx testing program so future users can quickly test? Or are you able to import csv files from clx setup? We probably need a new folder called micro8xx_setup.

[drbitboy]

I don't know if Micro8xx/CCW has import capability, but I will look into it.

Ooh, I just groked what you meant by CSV; I'll check that too! Not today though.

[TheFern2]

Yeah on the initial clx setup, I chose to provide all the tags needed for testing, and instructions on how to import them. The reason behind it, is that in rslogix5000 if you create a project in one version, it might not be easy to open on another version.

Now I don't know about micro8xx/ccw if all version are backwards compatible we can provide the program, but if not it might be easier to provide just the tags if ccw can import/export.

[drbitboy]

If nothing else I could make a YouTube video of how to create the tags, perhaps with a supporting CSV, and that might be enough.

[dmroeder]

I ran this PR and unittests on my setup, looks good. Well at least on clx plc. @dmroeder do you want to run the micro800 with tox?

I'll work on a CCW program with the same tags when I get some time.