Salus PS600 pipe temperature sensor: Model Id is reported as hexadecimal string representation

opechunka commented 6 months ago

Does the issue really belong here?

[X] I definitively want to report a bug within deCONZ or its REST-API

Is there already an existing issue for this?

[X] I have searched the existing issues and there is none for the bug at hand

Describe the bug

Model Id attribute is reported as hexadecimal representation of the string: 0x5053363030000020f60120b199000090000000a7750000b7c500009000000090

Actually the first part "0x505336303000" is null a terminated representation of "PS600". A worse thing is that the part after the "0x505336303000" is not stable, some of the numbers randomly change. So I cannot just replace the model id in DDF file ss909zb_temp_sensor.json I have 4 of those sensors. The behavior are the same for all of them. Of cause the second part differs for every of 4 devices.

Steps to reproduce the behavior

Try to add Salus PS600 as a new device.
A new node will appear in Deconz GUI. But it's not reported as added in Phoscon App.
Check the Cluster info for Basic endpoint. Check "Model Identifier" attribute. It's shown as

0x5053363030000020f60120b199000090000000a7750000b7c500009000000090

Expected behavior

Model Id is reported as a string (it's null terminated in the buffer).

Screenshots

Environment

Host system: Raspberry Pi
Running method: Raspbian
Firmware version: (26720700)
deCONZ version (not Home assistant Addon version!): (2.26.3)
Device: ConBee II
Do you use an USB extension cable: (yes) -- only relevant for ConBee I/II/III
Is there any other USB or serial devices connected to the host system? No

deCONZ Logs

No response

Additional context

Smanar commented 6 months ago

A worse thing is that the part after the "0x505336303000" is not stable, some of the numbers randomly change. So I cannot just replace the model id in DDF file ss909zb_temp_sensor.json

Seriously ? you have the model id changing ?

"PS600" in hexa is just 0x5053363030, so 0x0x505336303000 is enought for the model id, the number after are probably other thing. But why deconz don't "cut" the string ?

opechunka commented 6 months ago

A worse thing is that the part after the "0x505336303000" is not stable, some of the numbers randomly change. So I cannot just replace the model id in DDF file ss909zb_temp_sensor.json

Seriously ? you have the model id changing ?

"5053363030" in hexa is just 0x5053363030, so 0x0x505336303000 is enought for the model id, the number after are probably other thing. But why deconz don't "cut" the string ?

Here is with the same device. The id has been changed. Probaly after reseting the device and adding again.

Now it's

0x5053363030000020f60120b199000090000000a7750000112000009000000090

opechunka commented 6 months ago

The part of Id, which actually is just some memory block, contains node NWK address. When NWK changes, it's reflected in the memory block.

0x5053363030000020f60120b199000090000000a775000035c400009000000090

NWK address now is 0xC435, and we can see it in the Id memory as 35c4

Smanar commented 6 months ago

Have made a typo ^^, I want to say

"PS600" in hexa is just 0x5053363030, so 0x0x505336303000 is enought for the model id, the number after are probably other thing.

But the problem is not from deconz, as the issue is too on Z2M https://github.com/dresden-elektronik/deconz-rest-plugin/issues/6665#issuecomment-1478757377

For me it's the device that send a bad data. And the string is converted in hexa on deconz because of the 0x00 value, deconz converted the string in hexa to don't loose data.

Perhaps the API code cut the model ID itsel, else I think there is nothing to do.

There is no better firmware for this device ?

opechunka commented 6 months ago

There is no firmware update. At least I couldn't find. If it's a problem to fix it in general. Can you please point me in the deconz source code where the 0x00 is detected and the decision is made to convert content to hexa? I've spent a whole day to find the right place in the code, but unfortunately couldn't. I could track up to DBG_Printf(DBG_INFO, "[4.1] Get model ID\n"); in de_web_plugin.cpp.

And no further. I saw then some "ZCL got data for node" message in log, and then this hex sting. That's it. So where is the place in the code that makes a conversion? I'll be able then just use my local build. In other case my 4 pieces of PS600 device are not usable at all. Thanks in advance.

Smanar commented 6 months ago

All is fine in the deconz code for me, I don't see a problem on his working mode. And from manup some device need the string was no "shortened" https://github.com/dresden-elektronik/deconz-rest-plugin/issues/6665#issuecomment-1382965339

It's possible make a "hack" in the code according to manufacture number, but not sure others will be happy with that.

For sensor the model id is set in de_web_plugin.cpp with the fonction void Sensor::setModelId(const QString &mid)

                            if (ia->id() == 0x0005) // Model identifier
                            {
                                if (i->mustRead(READ_MODEL_ID))
                                {
                                    i->clearRead(READ_MODEL_ID);
                                }

                                QString str = ia->toString().simplified();

......................

                                if (!str.isEmpty())
                                {
                                    if (i->modelId() != str)
                                    {
                                        auto *item = i->item(RAttrModelId);
                                        i->setModelId(str);
                                        i->setNeedSaveDatabase(true);
                                        checkInstaModelId(&*i);
                                        updateSensorEtag(&*i);
                                        pushSensorInfoToCore(&*i);
                                        queSaveDb(DB_SENSORS, DB_LONG_SAVE_DELAY);
                                        enqueueEvent({i->prefix(), item->descriptor().suffix, i->id(), item, i->address().ext()});
                                    }

I think you can do something here, there is already some hacks in this part. The part where deconz convert to hexa string is probably in deconz code, this part is open too now, but if I m not sure the hack will be accepted in the API code, I m almost sure it will be not on deconz core itself ^^.

opechunka commented 6 months ago

Tried, it doesn't reach this part of code in DeRestPluginPrivate::updateSensorNode()

opechunka commented 6 months ago

I think it makes sense to check DeRestPluginPrivate::addSensorNode

Smanar commented 6 months ago

Tried, it doesn't reach this part of code in DeRestPluginPrivate::updateSensorNode()

You have tried to put debug line in c++ code in the de_web_plugin.cpp ? DBG_Printf(DBG_INFO_L2, "Just to test");

And enable info_l2 to see if this code part is used ?

It's possible the DDF core can bypass this part (and yes it's Legacy code), but as it need the model id and the manufacture name to know wich one DDF to use, for me this part is still used.

opechunka commented 6 months ago

Adding this code to addSensorNode and UpdateSensorNode did the trick. All of my PS600 sensors work perfectly. Also I created a copy of the DDF file devices\computime-salus\ss909zb_temp_sensor.json to ps600_temp_sensor.json and chaged to modelid in it:

"modelid": "SS909ZB", to "modelid": "PS600",

Thank you for the cooperation!

addSensorNode

`

           case BASIC_CLUSTER_ID:

            {
                if (modelId.isEmpty() || manufacturer.isEmpty())
                {
                    std::vector<deCONZ::ZclAttribute>::const_iterator j = ci->attributes().begin();
                    std::vector<deCONZ::ZclAttribute>::const_iterator jend = ci->attributes().end();

                    for (; j != jend; ++j)
                    {
                        if (manufacturer.isEmpty() && j->id() == 0x0004) // manufacturer id
                        {
                            manufacturer = j->toString().trimmed();
                        }
                        else if (modelId.isEmpty() && j->id() == 0x0005) // model id
                        {

          ........................................................................................................................................................................................................

          /*---------------------------------------------  My changes ---------------------------------------------*/
          else if (node->nodeDescriptor().manufacturerCode() == VENDOR_COMPUTIME && 
                                     modelId.startsWith(QLatin1String("0x505336303000")))
                            {
                                auto& attribute = const_cast<deCONZ::ZclAttribute&>(*j);
                                attribute.setFormatHint(deCONZ::ZclAttribute::FormatHint::DefaultFormat);
                                modelId = j->toString().trimmed();
                            }

`

UpdateSensorNode

`

                      if (ia->id() == 0x0005) // Model identifier
                        {
                            if (i->mustRead(READ_MODEL_ID))
                            {
                                i->clearRead(READ_MODEL_ID);
                            }
                            QString str = ia->toString().simplified();
                            if (isLidlDevice(str, i->manufacturer()))
                            {
                                // Ignore non-unique ModelIdentifier; modelid set from unqiue ManufacturerName.
                                continue;
                            }
                            if (str == QLatin1String("895a2d80097f4ae2b2d40500d5e03dcc"))
                            {

                            ..............................................................................................................................
                           /*---------------------------------------------  My changes ---------------------------------------------*/

                           else if (str.startsWith(QLatin1String("0x505336303000")))
                            {
                                str = QLatin1String("PS600");
                            }

`

dresden-elektronik / deconz-rest-plugin