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

AdsNotificationHeader has nTimeStamp & nNotification backwards #12

Closed
tdjordan opened this issue Oct 13, 2015 · 5 comments
Closed

AdsNotificationHeader has nTimeStamp & nNotification backwards #12

tdjordan opened this issue Oct 13, 2015 · 5 comments

Comments

@tdjordan
Copy link

In AdsDef.h at line 287,
nTimeStamp and hNotification are in the wrong order.

typedef struct {
    uint64_t nTimeStamp;
    uint32_t hNotification;
    uint32_t cbSampleSize;
#ifndef __cplusplus
    uint8_t data[];
#endif
} AdsNotificationHeader, * PAdsNotificationHeader;

My reference for this is from these two places.

TwinCat2 docs

typedef struct {
  ULONG           hNotification;
  __int64         nTimeStamp;
  ULONG           cbSampleSize;
  UCHAR           data[ANYSIZE_ARRAY];
} AdsNotificationHeader, *PAdsNotificationHeader;

TwinCat3 docs

      typedef struct {

       ULONG
      hNotification;
      __int64
      nTimeStamp;
      ULONG
      cbSampleSize;
      UCHAR
      data[ANYSIZE_ARRAY];
      } AdsNotificationHeader, *PAdsNotificationHeader;
@soberschmidt
Copy link
Contributor

Our TcAdsDll.dll for Windows OSs differs to our OpenSource AdsLib to provide a wider range of architecture support. Please use the structure information of our AdsLib.

@tdjordan
Copy link
Author

OK.

But how does that change the structure order of the binary data coming over the wire?

In my experiment, talking to a CX9020, accessing the nTimeStamp using your unmodified structure ordering, shows it never changing.

If I flip it around, it actually changed and is decoded as an actual timestamp.

Therefore, the order of the two fields in this specific structure is wrong and needs to be flipped around.

Or are you saying, that it doesn't matter?

How does producing a library that can be used on Linux as well as Windows magically change the binary protocol?

@soberschmidt
Copy link
Contributor

Check if you use the AdsLib with the related header and NOT the TcAdsDll.

@tdjordan
Copy link
Author

I am using the related header file from this project on a Linux machine accessing a CX9020.

The CX9020 was programmed using the stock TwinCAT 3 development environment.

So I am assuming that the stock TwinCAT 3 is using the TcAdsDll on the PLC itself while I am using this library on the Linux side.

Thus the difference in the binary protocol structure mismatch on the wire.

@pbruenn
Copy link
Member

pbruenn commented Oct 15, 2015

I think we have a misunderstanding here. The AdsNotificationHeader is never send on the wire. It is only used to share data between your application and the AdsLib.
The corresponding data structures on the wire would be: ADS Device Notification
If you run a short git grep AdsNotificationHeader and look into AdsNotification.h you can see that it is not a 1:1 copy of the data from wire.
We kept the API of AdsLib close to TcAdsDll to support applications which are capable to link against both libraries (one at a time). However if you decide to link against TcAdsDll you have to build your program with the TcAdsDll headers. And if you switch to link against AdsLib, you have to rebuild your application to use the AdsLib headers.

Please check that you rebuild your application and the AdsLib with the same version of the header and if the issue still persists, give us more details on your specific setup.
As a side note: the timestamp can be a very large number and the last digits might not change. So on the first look it looks like it's not changing, just on the second look you will spot the difference in the middle of the number

@pbruenn pbruenn closed this as completed Dec 1, 2015
pbruenn added a commit that referenced this issue Jul 12, 2016
This might be the phenomenon reported by tdjordan with issue #12
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

No branches or pull requests

3 participants