thingsboard / thingsboard-gateway

Open-source IoT Gateway - integrates devices connected to legacy and third-party systems with ThingsBoard IoT Platform using Modbus, CAN bus, BACnet, BLE, OPC-UA, MQTT, ODBC and REST protocols
https://thingsboard.io/docs/iot-gateway/what-is-iot-gateway/
Apache License 2.0
1.72k stars 829 forks source link

[BUG] - Registers Are Written in Reverse Order When Processing Attribute Updates for Modbus Connector #1135

Open helsmore opened 1 year ago

helsmore commented 1 year ago

Describe the bug In my case the Modbus connector is configured to (amongst others) allow a set of several Modbus registers that contain a single 8-character string to be read and written.

The data is read from the Modbus RTU device successfully but when a shared attribute is updated on the TB platform to trigger an attribute update the Modbus connector writes the registers in the reverse order, e.g. if the user enters 'ABCD0123' as the shared attribute value then '2301CDAB' is what will actually be written to the device's registers. The bytes are in the correct order in each register but the register payloads themselves are written in reverse order.

I think this bug is caused due to this line: https://github.com/thingsboard/thingsboard-gateway/blob/a2da5b836bddaa15a9e8e5db8ee959bddf9796a0/thingsboard_gateway/connectors/modbus/modbus_connector.py#L613

I cannot see any reason to reverse the list of register payloads here, the Modbus connector downlink converter already takes care of byte order and word order when building the payloads, thanks to the PyModbus 'BinaryPayloadBuilder' functions, so the unconditional reversal should not be required.

Excerpts from my Modbus connector configuration are as follows:

...
"type": "serial",
"method": "rtu",
...
"byteOrder": "BIG",
"wordOrder": "BIG",
...
"attributes": [
{
  "tag": "name",
  "type": "string",
  "functionCode": 3,
  "objectsCount": 4,
  "address": 10
},
...
],
"attributeUpdates": [
{
  "tag": "name",
  "type": "string",
  "functionCode": 16,
  "objectsCount": 4,
  "address": 10
},
...
]
...

Note that when reproducing this bug it is necessary to set the shared attribute value as an exactly 4- or 8-character string, this is due to an unrelated issue that will be reportedly separately.

Connector name (If bug in the some connector): Modbus Connector (tested in serial RTU mode)

Error traceback (If available): Not applicable

Versions (please complete the following information):

helsmore commented 1 year ago

Note, I've tested a version of the connector locally that removes the line with converted_data.reverse() and it seems to function correctly.

samson0v commented 11 months ago

Hi @helsmore, thanks for your interest in ThingsBoard IoT Gateway! This issue was fixed. Try the version from the master branch and let us know about the result.

helsmore commented 11 months ago

Hi @samson0v, I've inspected the diff between the 3.3 release (where I last confirmed the issue) and the master branch. I can see no code changes that would seem to affect the issue, and indeed the problematic line is still present in the latest version of the code: https://github.com/thingsboard/thingsboard-gateway/blob/1f2b4d662f6613eb519a66aa290f28eed0ec4bc5/thingsboard_gateway/connectors/modbus/modbus_connector.py#L642 Which commit is it that you think fixed the issue?