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
77 stars 18 forks source link

(v2) TypeError in `onAdsCommandReceived` #144

Closed crishoj closed 2 weeks ago

crishoj commented 2 weeks ago

Observed after reconnecting to running PLC with V2:

🛜 [plc SAFETY] Initiating PLC connection to AMSNetId 10.3.200.1.1.1 IP 10.3.200.2 port 48898 ...                                                 10:47:25.608
1417 |                 for (let stamp of packet.ads.payload.stamps) {
1418 |                     //Stamp has n samples inside
1419 |                     for (let sample of stamp.samples) {
1420 |                         //Try to find the subscription
1421 |                         const key = ADS.amsAddressToString(packet.ams.sourceAmsAddress);
1422 |                         const subscription = this.activeSubscriptions[key][sample.notificationHandle];
                                                                             ^
TypeError: undefined is not an object (evaluating 'this.activeSubscriptions[key][sample.notificationHandle]')
      at onAdsCommandReceived (.../node_modules/ads-client/dist/ads-client.js:1422:71)
      at onAmsTcpPacketReceived (.../node_modules/ads-client/dist/ads-client.js:1348:26)
      at parseAmsTcpPacket (.../node_modules/ads-client/dist/ads-client.js:1104:14)
crishoj commented 2 weeks ago

I should note:

I struggle to see how this.activeSubscriptions could ever be undefined, and assume the index access Bun complains about is this.activeSubscriptions[key][sample.notificationHandle].

jisotalo commented 2 weeks ago

There is no null-check at all which is my fault, need to fix that.

I think this issue is possible when you first subscribe to a variable and then the connection closes without unsubscribing (so the PLC keeps sending notifications).

When connecting again, the client receives notification with unknown notification handle. And probably unknown key too, if that happens just after connecting.

By the way, I still have a one breaking change to commit. I'm changing subscribeSymbol() to subscribeValue() to keep the naming consistent. Just a heads up!

jisotalo commented 2 weeks ago

Hopefully now fixed in beta.2!

crishoj commented 2 weeks ago

I think this issue is possible when you first subscribe to a variable and then the connection closes without unsubscribing (so the PLC keeps sending notifications).

Makes sense. A situation with stale subscription handles could arise if e.g. the application crashes during development, before unsubscribe() was called.

Hopefully now fixed in beta.2!

Thanks!

jisotalo commented 2 weeks ago

Closing this for now. Let me know if there are still any issues :)