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

Fix method parsing inconsistency #127

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

Conversation

icon-bobk
Copy link

This patch addresses two issues with parsing RPC Methods.

The first issue is an incorrect size of the method.flag during the read. In the master branch it is stated as a Uint16, however it is an Uint32. This is already corrected in the V2 branch. Comments were adjusted to represent the correct memory alignment (also corrected in V2).

The second patch uses the RPC method length as the ending position before continuing parsing the method array. If for some reason the data structure adds any additional information beyond the method.parameters value, it would offset the reads creating invalid parsing of the structure. (This was found while connecting to a Beckhoff 4026.8 host)

@jisotalo
Copy link
Owner

Thank you very much @icon-bobk!

I found that issue with flag size during v2 development, but probably forgot that it should be fixed in v1 too...

The second one: The fix makes sense. I'm going to develop the RPC to v2 soon, so I will check it out and then merge this pull request too. So will be backported to v1 also.

By checking TwinCAT.Ads.dll from nuget, we can see that RPC methods seem to have possibility for attributes after parameters (this is new I think). So you are absolutely right.

/// <summary>Parameters Collection</summary>
internal AdsMethodParaEntry[] parameters = Array.Empty<AdsMethodParaEntry>();
internal AdsAttributeEntry[]? attributes;
int start18 = start17 + SubStructureReader<AdsMethodParaEntry>.Unmarshal((uint) this.parameterCount, marshaler, span.Slice(start17, num2 - start17), out this.parameters);
if (this.flags.HasFlag((Enum) AdsMethodFlags.Attributes))
{
  ushort elementCount = BinaryPrimitives.ReadUInt16LittleEndian(span.Slice(start18));
  int start19 = start18 + 2;
  start18 = start19 + SubStructureReader<AdsAttributeEntry>.Unmarshal((uint) elementCount, marshaler, span.Slice(start19, num2 - start19), out this.attributes);
}

@jisotalo jisotalo added this to the Version 2.0 milestone Jul 11, 2024
jisotalo added a commit that referenced this pull request Jul 18, 2024
- Added invokeRpcMethod() + tests
- Added ADS_RCP_METHOD_FLAGS
- Updated ADS_RCP_METHOD_PARAM_FLAGS
- Added parsing of RPC method and RPC method parameter attributes (see #127)
@jisotalo jisotalo mentioned this pull request Jul 18, 2024
13 tasks
@jisotalo
Copy link
Owner

Parsing attributes should now work in v2, however I don't have TC 4026 yet installed for testing.

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.

None yet

2 participants