Bug fix in the GetSZL function #143
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
December 5, 2019 13:23
…ly an Uint16 and not an Int16. The int16 definition lead to the following problem: Some SZL's have their most significant bit set to TRUE. this happens in particular for SZL 0x0D91 (Uploading Module Information of an Profinet/DP device), when the station is on an Profinet System. In that case the INDEX parameter is something like "0x8006" or similar. The important thing is that the bit 15 is true (it indicates that we area dealing with an Profinet and not Profibus). However the "readSZL" in the IDaveConnection interface defines an integer (32/64 bits). Unfortunately when you call into IDaveConnection.ReadSZL, the INDEX gets sign-expanded from an int16 to an int32/int64. But that is not correct. The Index parameter must not be expanded. In the end this results that LibNoDave sends Index 0x8106 instead 0x8006 on the wire. How exactly this transformation happens, i also don't know exactly, I think it is an artifact of some ByteSwappint. I confirmed this with Wireshark, and the change from int16 to Uint16 fixes this issue. This was tested on an VIPA 017 SLIO CPU (basically it is an S7-317 PN/DP in a smaller form factor), with an real Profinet Configuration.
|
I just realized, the change i made fixes the bug, but it might break existing code.. |
|
maybe create a overload? |
|
true, did not think about that. But anyway we should remove the int16 eventually. for now i will make an overload |
|
OK. made an overload for compatibility. |
|
thanks again. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a Bug fix in the GetSZL function. The INDEX data-type is really an Uint16 and not an Int16.
The int16 definition lead to the following problem:
Some SZL's have their most significant bit set to TRUE. this happens in particular for SZL 0x0D91 (Uploading Module Information of an Profinet/DP device), when the station is on an Profinet System. In that case the INDEX parameter is something like "0x8006" or similar. The important thing is that the bit 15 is true (it indicates that we area dealing with an Profinet and not Profibus).
However the "readSZL" in the IDaveConnection interface defines an integer (32/64 bits). Unfortunately when you call into IDaveConnection.ReadSZL, the INDEX gets sign-expanded from an int16 to an int32/int64. But that is not correct. The Index parameter must not be expanded.
In the end this results that LibNoDave sends Index 0x8106 instead 0x8006 on the wire. I confirmed this with Wireshark, and the change from int16 to Uint16 fixes this issue.
This was tested on an VIPA 017 SLIO CPU (basically it is an S7-317 PN/DP in a smaller form factor), with an real Profinet Configuration.