Skip to content
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

feature/enip2 #2

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open

feature/enip2 #2

wants to merge 35 commits into from

Conversation

aporan
Copy link
Collaborator

@aporan aporan commented Apr 7, 2017

Notes:

  1. server still doesn't support multikey yet read/write yet, so it will respond with a proper error message.
  2. server still needs to take in log files.
  3. Pipe out messages from single_read and single_write script to the api.

Copy link
Collaborator

@francozappa francozappa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

francozappa
francozappa previously approved these changes Apr 27, 2017
@francozappa
Copy link
Collaborator

Make sure same API without datatype.

@francozappa
Copy link
Collaborator

@remmihsorp

Make sure that it can be installed adding it to the requirements.txt and setup.py.

Is it really necessary to depend upon a fork of pycomm?

@aporan
Copy link
Collaborator Author

aporan commented May 2, 2017

  1. Noted.
  2. Unfortunately, yes it does.

@francozappa
Copy link
Collaborator

Make sure that we add eq_ tests
Make sure that we return only the (string) value from _receive
Use our modbus implementation as a reference

@aporan
Copy link
Collaborator Author

aporan commented May 5, 2017

I have also updated my enipserver, you'd need to pull the latest changes.

@francozappa
Copy link
Collaborator

francozappa commented May 5, 2017

Thanks @remmihsorp .

Can you please remove the newline from the return value, eg:

plc1.receive(SENSOR2, PLC2_IP)

should return '1' and not '1\n'

p.s. Travis is failing because we don't have packaged enipserver and the pycomm fork

@francozappa
Copy link
Collaborator

Hi @remmihsorp,

What is the status of the package?

@aporan
Copy link
Collaborator Author

aporan commented Jun 22, 2017

Hi @francozappa, i have paused the development of the branch for a while as i have extensively changed (changing) the enipserver api. Hopefully, I aim to finish a complete refactoring of by next week, and then should be able proceed working on the feature/enip2 agian promptly.

@aporan
Copy link
Collaborator Author

aporan commented Jul 25, 2017

Hi @francozappa, I have tested the new additions inside a vm. The tests passes on my side. Provided that enipserver is not available as a package, you'd need to manually clone remmihsorp/pycomm and scy-phy/enipserver, add both pycomm and enipserver to PYTHONPATH to tests the changes on your side.

Thanks!

Also, enipserver now has udp messaging. Perhaps, we can think of test scenario to incorporate that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants