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

Running 1.12.0 in an electron environment #80

Closed enenkel closed 2 years ago

enenkel commented 2 years ago

Hey there,

we just tried to include this in an electron environment but it seems one of your dependencies has an issue at the referenced version, see here: https://github.com/ashtuchkin/iconv-lite/issues/204

Could you please check, if you can update this package to a version >= 0.6.0?

Thanks a lot! Best regards, Andy

jisotalo commented 2 years ago

Hi Andy!

There seems no reason why not to update it. However, just wondering, shouldn't the "iconv-lite": "^0.5.2" in package.json mean that it should install the most recent 0.x.x version always? So 0.6.2 would be installed? Or am I understanding something wrong?

Ps. Been wanting to try Electron and this library too, but haven't got time yet. Let me know how it goes.

enenkel commented 2 years ago

Hey, thanks for the quick reply.

That holds true for all Versions greater than 1.0.0. However, npm treats the 0.x.y version as pre-release and does not apply the same logic (as it is expected to have breaking changes between 0.5.y and 0.6.y for example).

jisotalo commented 2 years ago

Oh I see. Thanks for the info!

I'll update the package.json in few days.

enenkel commented 2 years ago

Thanks alot!

I also had issues with following statements (ads-client-ads.js) return ((ret = Object.keys(this).find(key => this[key] == "asd")) != null ? ret : 'UNKNOWN') due to the fact that ret is not declared as let/const/var. While this works in plain JS, in my environment it did not. I replaced it with following code to get it running: return (Object.keys(this).find(key => this[key] == value)) || 'UNKNOWN';

Should I open another issue for this?

Best regards Andy

jisotalo commented 2 years ago

No need to, I'll check them out too! Thanks for reporting, The original code makes no sense :D

jisotalo commented 2 years ago

Sorry for delay, been a little busy. I'll do this today or tomorrow!

enenkel commented 2 years ago

No worries, just text when you're done and I'll verify the new version. Thanks for your effort!

jisotalo commented 2 years ago

Just released new version, let me know how it goes!

jisotalo commented 2 years ago

Hey @enenkel! Any news? :)

enenkel commented 2 years ago

Hey, sry been quite busy with some other stuff. I hope to give you feedback within this week. However, commit looks good 👍

jisotalo commented 2 years ago

I'm closing this now. If there are problems you can create a new issue.