grodansparadis/node-red-contrib-socketcan

socketcan-in node not working properly if msg.payload.data is a Buffer

nonujigu opened this issue · 5 comments

The node "socketcan-in" works as expected if the object "msg.payload.data" is an array that contains integer numbers from 0 to 255:

msg.payload = {
    canfd   :   false,
    ext     :   false,
    rtr     :   false,
    canid   :   123,
    dlc     :   5,
    data    :   [1, 2, 3, 4, 5]
}

In this case the output CAN frame on the CAN bus is: can0 07B [5] 01 02 03 04 05
By the way, this also works if there is no msg.payload.dlc property.

Howerver, if msg.payload.data is not an array but a buffer, then the output CAN frame on the CAN bus contains no data:

msg.payload = {
    canfd   :   false,
    ext     :   false,
    rtr     :   false,
    canid   :   123,
    dlc     :   5,
    data    :   Buffer.from([1, 2, 3, 4, 5])
}

In this case the output CAN frame on the CAN bus is: can0 07B [0]
Also, the node throws an error with this message: "Error: CAN data has unknown format."
By the way, this error message is not thrown if there is no msg.payload.dlc property.

The problem can be retraced with this flow:

[
    {
        "id": "4e8751acb96b9497",
        "type": "socketcan-in",
        "z": "920d756.26f6f08",
        "name": "socketcan-in",
        "config": "6b55e2935811f28a",
        "x": 970,
        "y": 500,
        "wires": []
    },
    {
        "id": "6b664ae32ef5ea1a",
        "type": "function",
        "z": "920d756.26f6f08",
        "name": "array + dlc",
        "func": "msg.payload = {\n    canfd   :   false,\n    ext     :   false,\n    rtr     :   false,\n    canid   :   123,\n    dlc     :   5,\n    data    :   [1, 2, 3, 4, 5]\n};\nreturn msg;",
        "outputs": 1,
        "noerr": 0,
        "initialize": "",
        "finalize": "",
        "libs": [],
        "x": 710,
        "y": 500,
        "wires": [
            [
                "4e8751acb96b9497"
            ]
        ]
    },
    {
        "id": "a0c8b25771b9e576",
        "type": "inject",
        "z": "920d756.26f6f08",
        "name": "",
        "props": [
            {
                "p": "payload"
            },
            {
                "p": "topic",
                "vt": "str"
            }
        ],
        "repeat": "",
        "crontab": "",
        "once": false,
        "onceDelay": 0.1,
        "topic": "",
        "payload": "",
        "payloadType": "date",
        "x": 460,
        "y": 500,
        "wires": [
            [
                "6b664ae32ef5ea1a"
            ]
        ]
    },
    {
        "id": "63263a229e896eaa",
        "type": "function",
        "z": "920d756.26f6f08",
        "name": "buffer + dlc",
        "func": "msg.payload = {\n    canfd   :   false,\n    ext     :   false,\n    rtr     :   false,\n    canid   :   123,\n    dlc     :   5,\n    data    :   Buffer.from([1, 2, 3, 4, 5])\n};\nreturn msg;",
        "outputs": 1,
        "noerr": 0,
        "initialize": "",
        "finalize": "",
        "libs": [],
        "x": 710,
        "y": 620,
        "wires": [
            [
                "4e8751acb96b9497"
            ]
        ]
    },
    {
        "id": "6fc974ca539bb005",
        "type": "inject",
        "z": "920d756.26f6f08",
        "name": "",
        "props": [
            {
                "p": "payload"
            },
            {
                "p": "topic",
                "vt": "str"
            }
        ],
        "repeat": "",
        "crontab": "",
        "once": false,
        "onceDelay": 0.1,
        "topic": "",
        "payload": "",
        "payloadType": "date",
        "x": 460,
        "y": 620,
        "wires": [
            [
                "63263a229e896eaa"
            ]
        ]
    },
    {
        "id": "7d0d92a0f73dd337",
        "type": "function",
        "z": "920d756.26f6f08",
        "name": "array - dlc",
        "func": "msg.payload = {\n    canfd   :   false,\n    ext     :   false,\n    rtr     :   false,\n    canid   :   123,\n    data    :   [1, 2, 3, 4, 5]\n};\nreturn msg;",
        "outputs": 1,
        "noerr": 0,
        "initialize": "",
        "finalize": "",
        "libs": [],
        "x": 710,
        "y": 560,
        "wires": [
            [
                "4e8751acb96b9497"
            ]
        ]
    },
    {
        "id": "78e9580b29b46eb8",
        "type": "inject",
        "z": "920d756.26f6f08",
        "name": "",
        "props": [
            {
                "p": "payload"
            },
            {
                "p": "topic",
                "vt": "str"
            }
        ],
        "repeat": "",
        "crontab": "",
        "once": false,
        "onceDelay": 0.1,
        "topic": "",
        "payload": "",
        "payloadType": "date",
        "x": 460,
        "y": 560,
        "wires": [
            [
                "7d0d92a0f73dd337"
            ]
        ]
    },
    {
        "id": "e11fee2808e2c75d",
        "type": "function",
        "z": "920d756.26f6f08",
        "name": "buffer - dlc",
        "func": "msg.payload = {\n    canfd   :   false,\n    ext     :   false,\n    rtr     :   false,\n    canid   :   123,\n    data    :   Buffer.from([1, 2, 3, 4, 5])\n};\nreturn msg;",
        "outputs": 1,
        "noerr": 0,
        "initialize": "",
        "finalize": "",
        "libs": [],
        "x": 710,
        "y": 680,
        "wires": [
            [
                "4e8751acb96b9497"
            ]
        ]
    },
    {
        "id": "aec26853affbc6ed",
        "type": "inject",
        "z": "920d756.26f6f08",
        "name": "",
        "props": [
            {
                "p": "payload"
            },
            {
                "p": "topic",
                "vt": "str"
            }
        ],
        "repeat": "",
        "crontab": "",
        "once": false,
        "onceDelay": 0.1,
        "topic": "",
        "payload": "",
        "payloadType": "date",
        "x": 460,
        "y": 680,
        "wires": [
            [
                "e11fee2808e2c75d"
            ]
        ]
    },
    {
        "id": "6b55e2935811f28a",
        "type": "socketcan-config",
        "interface": "can0"
    }
]

Thanks for reporting this. I am currently away on vacation but will take a look at this next week.

I found a solution inside the send.js script that works for me.

The original code in line 134 only checks for JavaScript Arrays:
if ( Array.isArray(msg.payload.data) ) {

If msg.payload.data is a Buffer, then the result of Array.isArray(msg.payload.data) is false.

I changed the code in line 134 to:
if ( Array.isArray(msg.payload.data) || Buffer.isBuffer(msg.payload.data) ) {

With this modification, for me everything works as expected if msg.payload.data is a Buffer, whether the msg.payload.dlc property is present or not.

I tested this with Node-RED v3.0.1 and both Node.js v16.16.0 and Node.js v18.6.0.

Hi and sorry for late response here. I have updated the code as of your work and released version 1.2.9. Thanks a lot for this. Probably the best bug report I have ever seen and with a fix to. Nice to come back from vacation to that. Thanks again!

Hi and sorry for late response here. I have updated the code as of your work and released version 1.2.9. Thanks a lot for this. Probably the best bug report I have ever seen and with a fix to. Nice to come back from vacation to that. Thanks again!

Thank you for the kind words.
For the socketcan-out node, it would be nice to also have the option to get msg.payload.data as a buffer instead of an array. The advantage of the buffer type is that you can easily decode the raw data of the can frame with different read methods from Buffer.js. Another possibility would be to utilize node-red-contrib-buffer-parser to decode the raw data without any function nodes.

To make this work for me, I edited the line 97 in receive.js:

original line 97 in receive.js: msg.payload.data = Array.prototype.slice.call(frame.data, 0);

edited line 97 in receive.js: msg.payload.data = frame.data;.

However, I think it would make sense to make this configurable in the socketcan-config node.

I will put this in a feature request if you don't mind.

Good idea. Do that. I will add that the next time I enter this project again. Thanks again for your help.