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

adsGetNetIdForPLC: Solve too small buffer size for build 4026+ #437

Merged
merged 2 commits into from
Dec 3, 2024

Conversation

maxkofler
Copy link
Contributor

Some PLCs that run TwinCAT >4026 send more bytes than expected when requesting the AMS NetID. This patch allocates more buffer size to the response to solve an error where the function would error out for no appearant reason.

Some PLCs that run TwinCAT >4026 send more bytes than expected when requesting the AMS NetID. This patch allocates more buffer size to the response to solve an error where the function would error out for no appearant reason.
@chrisbeardy
Copy link
Collaborator

Thanks for this, will look into merging shortly, do you think this will fix #432 ?

@chrisbeardy
Copy link
Collaborator

Does this also work with no consequences for 4024?

@maxkofler
Copy link
Contributor Author

Thankyou for your fast response. I do not think that this would solve the other issue. This error originated in that the PLC sent more data than expected...

Regarding the tests, I can do some tomorrow. I will add another comment to let you know about that.

@maxkofler
Copy link
Contributor Author

@chrisbeardy I confirm that this change can be used with 4024. In fact, the buffer size can be way bigger even, so that's something we can think about doing to prevent such errors

@coveralls
Copy link

Pull Request Test Coverage Report for Build 12134055749

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 94.951%

Totals Coverage Status
Change from base Build 11345095278: 0.0%
Covered Lines: 1730
Relevant Lines: 1822

💛 - Coveralls

@chrisbeardy chrisbeardy merged commit 87c0681 into stlehmann:master Dec 3, 2024
6 checks passed
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.

3 participants