ResourceItem cannot hold UINT64 values

[ebaauw]

Does the issue really belong here?

• I definitively want to report a bug within deCONZ or its REST-API

Is there already an existing issue for this?

• I have searched the existing issues and there is none for the bug at hand

Describe the bug

I see negative values reported for extaddress, for instance:

{
  "capabilities": {
    "alerts": [
      "none",
      "select",
      "lselect",
      "blink",
      "breathe",
      "okay",
      "channelchange",
      "finish",
      "stop"
    ],
    "otau": {
      "file_version": 16777220,
      "image_type": 16649,
      "manufacturer_code": 4476
    },
    "sleeper": false
  },
  "config": {
    "bri": {
      "execute_if_off": true,
      "on_level": "previous",
      "onoff_transitiontime": 4,
      "startup": "previous"
    },
    "groups": [
      "0",
      "322",
      "2",
      "320"
    ],
    "on": {
      "startup": false
    }
  },
  "ddf_hash": null,
  "ddf_policy": "latest_prefer_stable",
  "etag": "5e019f63bbe36f26a81e367495497495",
  "extaddress": -8639569845441401000,
  "hascolor": false,
  "lastannounced": null,
  "lastseen": "2024-12-02T19:29Z",
  "manufacturername": "IKEA of Sweden",
  "modelid": "TRADFRI Driver 10W",
  "name": "Bedroom Cupboard",
  "nwkaddress": 21832,
  "productid": "ICPSHC2410EUIL2",
  "state": {
    "alert": "none",
    "bri": 254,
    "on": false,
    "reachable": true
  },
  "swversion": "1.0.4",
  "type": "Dimmable light",
  "uniqueid": "88:1a:14:ff:fe:49:f0:b7-01"
}

Of course, the Mac Address is a 64-bit unsigned integer. RAttrExtAddress is defined as such alright:

deconz-rest-plugin/resource.cpp

Line 389 in 8ae69a9

rItemDescriptors.emplace_back(ResourceItemDescriptor(DataTypeUInt64, QVariant::Double, RAttrExtAddress));

However, internally ResourceItem stores numeric values as qint64 and ResourceItem::toNumber() also returns a qint64.

Steps to reproduce the behavior

Need PR #7979 to show extaddress in the API, then request a lights or sensors resource for a device with a MAC address of 80:00:00:00:00:00:00 or greater.

Expected behavior

extaddress should be shown as unsigned number.

Screenshots

No response

Environment

• Host system: all
• Running method: all
• Firmware version: all
• deCONZ version: 2.29.1 with PR Refactor lightToMap() and sensorToMap() #7979.
• Device: all
• Do you use an USB extension cable: n/a
• Is there any other USB or serial devices connected to the host system? If so: Which?

deCONZ Logs

No response

Additional context

No response

[ebaauw]

I'm not sure what would be the best approach to address this. We could probably simply cast the value to quint64 in the API in rest_lights.cpp and rest_sensors.cpp and leave the internal representation unchanged. It would mean an additional exception in the item handling, though. Or we could make RAttrExtAddress non-public, as it's already available through uniqueid.

Changing the internal representation in ResourceItem seems more work and more risky. I'm not sure if that's worth the effort. There are a few of other resource items with DataTypeUInt64, but I think we could get away with these:

item	issue	public	remarks
RAttrExtAddress	yes	yes	Is an issue - see above.
RStateConsumption RStateConsumption_2	no	yes	Will be changed to DataTypeReal for DDFs by PR #7979. The legacy code still uses DataTypeUInt64, but I doubt we'll see consumption values this big in practice. Anyway, can be solved by creating DDFs.
RStateGPDLastPair	?	no	Used by C++ code for ZGP devices. Seems to be a timestamp - might be an issue when elapsed time increases beyond 2^63.
RStateProduction	no	yes	Is the opposite of RStateConsumption and could (should) be changed to DatatTypeReal as well. Not used in legacy code.
RCapColorEffects	no	yes	Used by Hue lights to indicate which effects the light supports. This is actually a bitmap and bit 63 doesn't seem to be used.

[ebaauw]

The change of items to DataTypeReal breaks the evaluation of rules conditions, at least for lt and gt. Event defines m_num and m_numPrev as int, where ResourceItem defines these as qint64. Not sure if this is causing the issue, or if its simply incorrect conversion from real to int.

[manup]

From my perspective Rules can be refined to properly compare values, currently it's a bit naive with things like:

        if (c->op() == RuleCondition::OpEqual)
        {
            if (c->numericValue() != item->toNumber())
            {
                return false;
            }

            if (item == eItem && e.num() == e.numPrevious())
            {
                return false; // item was not changed
            }
        }

Here c->numericValue() is a 32-bit signed int, as well as e.num().
So this already falls short even for all signed 64-bit numbers in ResourceItem which are outside of the signed 32-bit range.

I guess most of the time so far we get away with that since likely not many rules need the larger numbers. This can and should be fixed anyway. I have an idea how to make this with little effort, also accounting for safe signed vs unsigned 64-bit comparisons.

---

The elephant in the room is the Javascript and JSON representation. Here we can only represent 52-bit values as numeric.

While the following 64-bit number is perfectly valid JSON according to the spec, most parsers will read the value as 64-bit double and we're back to 52-bit precision.

{
  "n": 18446744073709551615
}

Quick test in NodeJS.

> JSON.parse("{\"n\": 18446744073709551615}")

{ large_number: 18446744073709552000 }

Python doesn't has this problem since it has proper large number support by default:

import json
json.loads('{"n": 18446744073709551615}')
{'n': 18446744073709551615}

---

Therefore we can only safely represent these as string in JSON

{
  "n": "18446744073709551615"
}

And if needed a Javascript application can calculate with them as BigInt("18446744073709551615").

The Javascript engine in deCONZ doesn't support BigInt yet but that can be added if needed.

---

But back to RAttrExtAddress the purpose for this item is to have the actual numeric value used by various parts in the DDF/Device machinery. It's not meant to be exposed publicly to JSON.

However an actual safe string representation already exists for every Device top level resource as RAttrUniqueId, for example on a sensor or light, the following could be used:

lightNode->parentResource()->item(RAttrUniqueId)->toString()  // e.g. "00:0b:57:ff:fe:26:56:80"

However personally I don't see any value in exposing this since we already have this with the sub-resource uniqueid, if anybody needs the MAC address it can already be derived from it.

[ebaauw]

But back to RAttrExtAddress the purpose for this item is to have the actual numeric value used by various parts in the DDF/Device machinery. It's not meant to be exposed publicly to JSON.

Happy to make RAttrExtAddress non-public so the API won't show it. I'll double-check whether RAttrUniqueId is available on the Device, but the API won't show it for /lights nor /sensors endpoints, since it's overwritten by the ReosurceItem on the (child) Resource.

I guess most of the time so far we get away with that since likely not many rules need the larger numbers. This can and should be fixed anyway. I have an idea how to make this with little effort, also accounting for safe signed vs unsigned 64-bit comparisons.

Yes, as mentioned above, there are only a few u64 ResourceItem types, and, I think, only one floating point (RAttrMeasuredValue). I tried to change some to floating point, but then discovered that the rule conditions break. Please make sure the change also addresses floating point values.