search

jisotalo / ads-client

Unofficial Node.js ADS library for connecting to Beckhoff TwinCAT automation systems using ADS protocol.
https://jisotalo.fi/ads-client/
MIT License
80 stars 17 forks source link

_parseDataType: Incorrect parsing of flags #109

Closed mycroes closed 1 year ago

mycroes commented 1 year ago

In the following excerpt from _parseDataType:

  //20..23 Offset
  dataType.offset = data.readUInt32LE(pos)
  pos += 4

  //24..27 ADS data type (AdsDataType)
  dataType.adsDataType = data.readUInt32LE(pos)
  dataType.adsDataTypeStr = ADS.ADS_DATA_TYPES.toString(dataType.adsDataType)
  pos += 4

  //27..30 Flags (AdsDataTypeFlags)
  dataType.flags = data.readUInt16LE(pos)
  dataType.flagsStr = ADS.ADS_DATA_TYPE_FLAGS.toStringArray(dataType.flags)
  pos += 4

  //31..32 Name length
  dataType.nameLength = data.readUInt16LE(pos)
  pos += 2

  //33..34 Type length
  dataType.typeLength = data.readUInt16LE(pos)
  pos += 2

The offsets in the comments are off by one starting at Flags (should have been 28..31) and flags is parsed using readUInt16LE, while ADS_DATA_TYPE_FLAGS actually contains values > 16 bits. pos is incremented by 4, so it's mostly comments that are messed up.

Noticed this while expanding the AdsClient library, so big thank you for the detailed implementation.

jisotalo commented 1 year ago

Hi @mycroes!

Thank you for reporting this, it indeed seems to be a bug! Also by looking at ADS_DATA_TYPE_FLAGS we can see that these values go over 16 bits.

I will merge your pull request, thanks.

Your library looks nice, keep up the good work!

mycroes commented 1 year ago

You're welcome! It's actually not my library, as in, I'm not the original author. However, I've recently acquired ownership of the GitHub repository and am in the process of expanding and refactoring the library to make it well-suited and performant for professional use.

I fixed at least one issue by using your code as reference and I'm making use of a lot of the other code to implement (or extend) things like retrieving symbols and datatypes. I still have to tackle updates to symbols though, but I guess your library provides enough information to handle that as well. So again, a big thanks for the detailed implementation, I'll put it to good use!

jisotalo commented 1 year ago

Sounds good! Let me know if you have any issues by email, for example. It's always interesting to help.