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

fixed static port 801 #3

Merged
merged 2 commits into from
Nov 10, 2021
Merged

fixed static port 801 #3

merged 2 commits into from
Nov 10, 2021

Conversation

HWilke
Copy link
Contributor

@HWilke HWilke commented Nov 9, 2021

Currently the port 801 is fixed and cannot be changed.
This is problematic especially because TwinCAT3 uses port 851 per default for PLC.
Beside that I also used uint16 for port variable because this is sufficient.

Copy link
Owner

@stamp stamp left a comment

Choose a reason for hiding this comment

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

Hey :) Thank you for the PR!

It was a long time since I used this code so I'm mostly guessing here. I did find a small detail that could still stand in the way of connecting to a TC3. Have you managed to connect to TC3 with only these changes?

main.go Outdated
@@ -80,7 +76,7 @@ func (conn *Connection) Connect() {
logger.Trace("Connected")
conn.shutdown = make(chan bool)
conn.shutdownFinal = make(chan bool)

conn.target = stringToNetId(conn.netid)
conn.target.port = 801
Copy link
Owner

@stamp stamp Nov 10, 2021

Choose a reason for hiding this comment

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

Could it be that we need to update 801 to port as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. I replaced that as well with a new commit.
I tested it on a separate system and forgot to take both changes into the repo on my development machine.

@stamp stamp merged commit f0e7b10 into stamp:master Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants