daxbot / node-canopen Goto Github PK

Fields "inhibitTime", "eventTime" and "syncStart" are not mandatory in RPDO.
Example solution (src/protocol/pdo.js):

@@ -232,17 +232,17 @@ class Pdo {
         /* sub-index 3:
          *   bit 0..15      Inhibit time.
          */
-        const inhibitTime = entry[3].value;
+        const inhibitTime = (index >= 0x1800)?entry[3].value:0;
 
         /* sub-index 5:
          *   bit 0..15      Event timer value.
          */
-        const eventTime = entry[5].value;
+        const eventTime = (index >= 0x1800)?entry[5].value:0;
 
         /* sub-index 6:
          *   bit 0..7       SYNC start value.
          */
-        const syncStart = entry[6].value;
+        const syncStart = (index >= 0x1800)?entry[6].value:0;
 
         let pdo = {
             cobId:          cobId,

This is in reference to #36

The current implementation of the SDO block transfer may run into issues when running multiple transfers. This needs to be tested and fixed if needed.

Could you export DataObject
I found that this class has not been exported, can you export this class in this module?

TIME write function (time.js:128) uses conversion typeToRaw (eds.js:242) which operate on epoch timestamp.
For CANopen time producer offset should be 441763200 seconds.
CAN-CiA.org
Wikipedia

I am trying to read a Can Bus USB device that support CanOpen on a windows machine but its failing with

npm ERR! code EBADPLATFORM
npm ERR! notsup Unsupported platform for [email protected]: wanted {"os":"linux","arch":"any"} (current: {"os":"win32","arch":"x64"})
npm ERR! notsup Valid OS: linux
npm ERR! notsup Valid Arch: any
npm ERR! notsup Actual OS: win32
npm ERR! notsup Actual Arch: x64

Whenever a client writes SDO data to a server, I would like to act on this new data. I cannot find any provisions in the code to do this. I have implemented a callback in the SDO server myself, but I was wondering if there is a supported way to do this, without continiously checking the object dictionary for changes?

Thanks in advance!

First bit (s) is not set in 4 byte expedited download and therefore a client replies with an error. |0x02 should be |0x03 in init_download function?

Noticed this this morning, if an SDO download times out (With err: SDO protocol timed out), it seems subsequent requests to the same index+subindex fail immediately with a timeout as well.

Looking at the CAN dump, it does indeed send the expected CAN message on the write starting, but it does not send the SDO abort message once the timeout is reached, and the subsequent times it does not send anything at all.

In our tests we're currently generating an EDS at runtime based on input data, and then verifying our SDO operations work against a mock server based on the EDS generated at runtime. In Device.js, it would be nice if we could just pass the already loaded string or Buffer to this here instead of having to save it to a file.

As well, it would be nice to be able to dynamically add entries to the Device at runtime, as some cases we do not use an EDS. Whilst we can just do this by manually loading it via device.dataObjects, it'd be nice if it was an official API in case the Device.js object changes later.

Example solution (src/protocol/pdo.js):

@@ -178,7 +178,10 @@ class Pdo {
 
             for(const dataObject of pdo.dataObjects) {
                 const index = dataObject.index;
-                const entry = this.device.dataObjects[index];
+                let entry = this.device.dataObjects[index];
+                if(entry.subNumber > 0) {
+                    entry = entry[dataObject.subIndex];
+                }
                 const size = entry.raw.length;

Based on the code in Pdo._parsePDO() it looks the intention was to keep track of lastValue per dataObject in pdo.dataObjects, but lines 153-156 store lastValue in the Pdo itself:

            if(pdo.lastValue != newValue) {
                pdo.lastValue = newValue;
                dataUpdated = true;
            }

I think these lines should be replaced by

            if(dataObject.lastValue != newValue) {
                dataObject.lastValue = newValue;
                dataUpdated = true;
            }

I don't use Pdo._onMessage() because we don't have Rx PDO's in our application, but it looks like that function has the same problem.

When the listener function on pdo.js:104-106 gets executed, it has the "this" scope of the dataObject it was assigned to on line 113: entry.on('update', listener);.
The problem is that you want to call Pdo.write(), so the "this" should be the Pdo and not the dataObject.
Solved by replacling pdo.js:104-106 by an arrow function: const listener = () => this.write(cobId);

I haven't tested the other cases in Pdo.start(), but the other listener functions problably need similar fixes.

When a client tries to initiate an upload or download of an entry that does not exist, device.receive() throws a TypeError.

Example stack trace for an upload:

TypeError: Cannot read property 'send' of undefined
    at Transfer.send (/appfs/apps/nodejs-server/node_modules/canopen/src/protocol/sdo.js:221:21)
    at Transfer.abort (/appfs/apps/nodejs-server/node_modules/canopen/src/protocol/sdo.js:260:14)
    at Sdo._serverUploadInitiate (/appfs/apps/nodejs-server/node_modules/canopen/src/protocol/sdo.js:796:27)
    at Sdo._onMessage (/appfs/apps/nodejs-server/node_modules/canopen/src/protocol/sdo.js:987:26)
    at emitOne (events.js:121:20)
    at Device.emit (events.js:211:7)
    at Device.receive (/appfs/apps/nodejs-server/node_modules/canopen/src/device.js:104:18)

It looks like this.device of the Transfer objects in the Sdo's this.clients are not initialized anywhere and that's why the send() function throws.
I'm only using the library to implement an SDO server, but it looks like the SDO client implementation does not have this problem: this.device of the Transfer objects in the Sdo's this.transfers is initialized correctly, it seems.

Hey,
I looked through code and saw that the documentation is already really good. But it could be easier to work with if it had Typescript support. Did you ever think about doing something like this?
For me it is a lot easier to work with, because my IDE tells me what functions are there and the documentation of the code. This could make it easier especially for people who are new to lower the bar for them.
I started using your codebase and was working with the documentation. This is also valid and should be sustained, but TS is easier to work with then JS, because with the compile errors you avoid doing a lot of mistakes.

It would be very handy to include the original abort code in the SDO rejection error passed to the Promise reject. We have a few custom codes in our device that require it's own handling that aren't included in the list here.

I think their may be a bug in the latest changes for the EDS getters where the data is defined as boolean, as the parseInt function is returning NaN instead of an integer value and hence the "this.device.eds.lssSupported" in the LSS.init function is always false. May be the same issue in other locations.

In pdo.js:101-102, I think you the entry in the Eds should be used instead of the dataObject stored in the Pdo:

                        let entry = this.device.eds.getEntry(dataObject.index);
                        if(entry.subNumber > 0)
                            entry = entry[dataObject.subIndex];

                        this.events.push([entry, 'update', listener]);
                        entry.on('update', listener);

It confused me at first, but pdo.dataObjects does not contain objects of class DataObject. That's why dataObject.on() does not exist.

I think entry = entry[pdo.subIndex]; in pdo.js:138 should be entry = entry[pdo.dataObjects[i].subIndex]; instead.

I recently upgrade an old project from canopen 2.8.2 to 6.0.0 and found all SDO uploads failing with timeout.
The reason is that the cobIdTx and cobIdRx are in version 6.0.0 no longer automatically enriched with the server node id.
CAN traffix while SDO timeout in canopen 6.0.0:

(2024-03-31 21:41:42.263225) can0 600 [8] 40 E6 55 02 00 00 00 00
(2024-03-31 21:41:42.294210) can0 600 [8] 80 E6 55 02 00 00 04 05

CAN traffic without SDO timeout in canopen 2.8.2:

(2024-03-31 21:47:04.374661) can0 620 [8] 40 E6 55 02 00 00 00 00
(2024-03-31 21:47:04.375400) can0 5A0 [8] 41 E6 55 02 2A 00 00 00
...

possible workaround in 6.0.0 is to call
device.eds.addSdoClientParameter(0x20, 0x600|0x20 ,0x580|0x20);
to manually set correct effective cobId(Tx/Rx), instead of
device.eds.addSdoClientParameter(0x20, 0x600 ,0x580);

if looks odd to me that you are checking like this if serverId already has been applied to cobId(Tx/Rx):

            if ((cobIdRx & 0xF) == 0x0)
                cobIdRx |= deviceId;

I assume this more should be

            if ((cobIdRx & 0x7F) == 0x0)
                cobIdRx |= deviceId;

as node ids are valid from 1 to 127 and not just from 1 to 15.
Please review if this is a bug or a desired behaviour, but if it is desired you at least need to update the SDO examples to reflect the changed call of addSdoClientParameter.

We're currently sending NMT commands to specific nodes directly via node-can (So not using the lib, but on the same canopen interface this lib uses), and we're seeing the following exception happen if we send an NMT reset request to a non-zero node ID:

canopen/src/Network.js:88
                this.devices[target].state = state;
                                           ^

TypeError: Cannot set property 'state' of undefined
    at Network._onMessage (canopen/src/Network.js:88:44)

Messages we send are echoed back to us, so are picked up by _onMessage in Network.js. The code there then assumes we have a device registered with the node ID we received. Since we don't, it then throws the type error above.

This also means we can be crashed by receiving any NMT command with a non-zero ID.

When the COB_ID for PDO is different from the default value, the node address should not be added.

Partial solution (src/protocol/pdo.js):

@@ -221,7 +221,7 @@ class Pdo {
             throw TypeError("CAN extended frames are not supported.")
 
         cobId &= 0x7FF;
-        if((cobId & 0xF) == 0x0)
+        if((cobId & 0x7F) == 0x0)
             cobId |= this.device.id;

I currently have a 10 second timeout set for SDO transfers, and I noticed that on segmented transfers the timeout is applied for the entire operation, and not per segment like I expected. Sometimes I need to send a ~100KB or so file to a device on my network, which takes 30-40 seconds or so. I would expect the 10 second timeout to count for each segment, not the entire transfer.

Currently the timeout will trigger, despite the SDO transfer still working correctly. Ideally I'd only want the timeout to trigger when the device stops responding to an expected segment.