node-ads icon indicating copy to clipboard operation
node-ads copied to clipboard

getSymbols bug fix is not available on npm

Open MarkusHardardt opened this issue 7 years ago • 8 comments

Hello Mister Musolino When I do an update with npm I do receive version 1.4.3 of node-ads but unfortunately the getSymbols bugfix you did at 25 Sep 2018 is not contained. Could you please have a look at this. Thanks for your help and another thanks for your library. It looks much better to me than the TcAdsWebService stuff from Beckhoff. Good job! Best regards Markus Hardardt

MarkusHardardt avatar Mar 08 '19 15:03 MarkusHardardt

It wasn't released on npm, once https://github.com/roccomuso/node-ads/pull/27 is merged we'll publish a new patch release.

roccomuso avatar Mar 10 '19 13:03 roccomuso

Hello Mister Musolino

 

Thanks for your information!

 

There is another thing you might be interessted in (when doing fixes anyway):

 

The following regex 

var re = /ARRAY[\s]+[([-\d]+)..([-\d]+)][\s]+of[\s]+(.*)/i

located inside the function:

var getSymbols = function (cb, raw) { ... }

at line 604 in the latest github version of ads.js does not match for raw == false in case of multidimensional arrays (like ARRAY [0..X,0..Y, ...] OF Z).

 

I saw this accidentally when I did a textual diff of dumps calling this function with true or false (just to find out what is the difference). But in my case I'm gonna use the getSymbols function with raw == true, so this is not a problem for me.

 

Greetings

Markus Hardardt  

 

Gesendet: Sonntag, 10. März 2019 um 14:14 Uhr Von: "Rocco Musolino" [email protected] An: roccomuso/node-ads [email protected] Cc: "Markus Hardardt" [email protected], Author [email protected] Betreff: Re: [roccomuso/node-ads] getSymbols bug fix is not available on npm (#26)

It wasn't released on npm, once #27 is merged we'll publish a new patch release.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

MarkusHardardt avatar Mar 11 '19 09:03 MarkusHardardt

You are absolutely right. Since it never worked, I introduced the RAW feature. Sorry, it was a quick hack. You can correct it if you want.

PLCHome avatar Mar 11 '19 21:03 PLCHome

@MarkusHardardt thanks! @PLCHome added you on npm as mantainer for node-ads :+1: feel free to release new version

roccomuso avatar Mar 12 '19 12:03 roccomuso

Published...

PLCHome avatar Mar 12 '19 20:03 PLCHome

var getSymbols = function (cb, raw) { var ads = this var cmdLength = { indexGroup: ADSIGRP.SYM_UPLOADINFO2, indexOffset: 0x00000000, bytelength: 0x30 }

var cmdSymbols = { indexGroup: ADSIGRP.SYM_UPLOAD, indexOffset: 0x00000000 }

readCommand.call(ads, cmdLength, function (err, result) { if (!err) { var data = result.readInt32LE(4) cmdSymbols.bytelength = data

  readCommand.call(ads, cmdSymbols, function (err, result) {
    var symbols = []
    var initialPos = 0
    if (!err) {
      while (initialPos < result.length) {
        var symbol = {}
        var pos = initialPos
        var readLength = result.readUInt32LE(pos)
        initialPos = initialPos + readLength
        symbol.indexGroup = result.readUInt32LE(pos + 4)
        symbol.indexOffset = result.readUInt32LE(pos + 8)
        symbol.size = result.readUInt32LE(pos + 12)
        // symbol.type = result.readUInt32LE(pos + 16) //ADST_ ...
        // symbol.something = result.readUInt32LE(pos + 20)
        var nameLength = result.readUInt16LE(pos + 24) + 1
        var typeLength = result.readUInt16LE(pos + 26) + 1
        var commentLength = result.readUInt16LE(pos + 28) + 1

        pos = pos + 30

        var nameBuf = Buffer.alloc(nameLength)
        result.copy(nameBuf, 0, pos, pos + nameLength)
        symbol.name = nameBuf.toString('binary', 0, findStringEnd(nameBuf, 0))
        pos = pos + nameLength

        var typeBuf = Buffer.alloc(typeLength)
        result.copy(typeBuf, 0, pos, pos + typeLength)
        symbol.type = typeBuf.toString('binary', 0, findStringEnd(typeBuf, 0))
        pos = pos + typeLength

        var commentBuf = Buffer.alloc(commentLength)
        result.copy(commentBuf, 0, pos, pos + commentLength)
        symbol.comment = commentBuf.toString('binary', 0, findStringEnd(commentBuf, 0))
        pos = pos + commentLength

        symbols.push(symbol)
        // we additionally add symbols for each array element when not in raw-mode
        var arraytype = !raw ? parseArrayType(symbol.type) : false
        if (arraytype) {
          forEachArrayIndex(arraytype.range, function (indices) {
            var newSymbol = JSON.parse(JSON.stringify(symbol))
            newSymbol.name += indices
            newSymbol.type = arraytype.type
            symbols.push(newSymbol)
          })
        }
      }
    }

    cb.call(ads.adsClient, err, symbols)
  })
} else {
  cb.call(ads.adsClient, err)
}

}) }

var parseArrayType = function (type) { var m = /^\sARRAY\s[\s*(-?\d+\s*..\s*-?\d+(?:\s*,\s*-?\d+\s*..\s*-?\d+))\s]\sOF\s+(.+)\s$/i.exec(type) if (m) { var range = [], r, rx = /\s*,?\s*(-?\d+)\s*..\s*(-?\d+)\s*/g rx.lastIndex = 0 while ((r = rx.exec(m[1])) !== null) { range.push([parseInt(r[1]), parseInt(r[2])]) } return {type : m[2], range : range} } else { return false } }

var forEachArrayIndex = function (range, cb) { var offset = arguments.length === 4 ? arguments[2] : 0 var indices = arguments.length === 4 ? arguments[3] : [] var start = range[offset][0], end = range[offset][1] for (var i = start; i <= end; i++) { indices.push(i) if (offset < range.length - 1) { forEachArrayIndex(range, cb, offset + 1, indices) } else { cb(JSON.stringify(indices)) } indices.pop() } }

MarkusHardardt avatar Mar 17 '19 07:03 MarkusHardardt

Thanx.

I currently have less time. However, at first glance, it did not seem to me that the same result would be returned to one-dimensional arrays as earlier. This is important so that working code is not denied at one go

PLCHome avatar Mar 17 '19 12:03 PLCHome

Hi

The difference is probably because of the missing "arrayid". I found no other reference to "arrayid" in ads.js and - on the other hand - had no idea how to build an integer based id for multidimensional array elements, so I just left it away.

But no problem for me anyway, because for me "raw" mode is pretty good. So I can life with the unchanged version.

 

 

Gesendet: Sonntag, 17. März 2019 um 13:49 Uhr Von: "PLCHome" [email protected] An: roccomuso/node-ads [email protected] Cc: "Markus Hardardt" [email protected], Mention [email protected] Betreff: Re: [roccomuso/node-ads] getSymbols bug fix is not available on npm (#26)

Thanx.

I currently have less time. However, at first glance, it did not seem to me that the same result would be returned to one-dimensional arrays as earlier. This is important so that working code is not denied at one go

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

MarkusHardardt avatar Mar 17 '19 15:03 MarkusHardardt