Expose NWK address as hex value through REST API

[SwoopX]

Currently, the NWK address is exposed as decimal value. Exposing it as hex value may aid in expediting debugging, as some log line only provide the NWK address.

[ebaauw]

This will work, but would it be cleaner to implement this logic only once, in ResourceItem::toVariant()? Ideally, I’d create a subclass for RAttrNwkAddress, but that seems too big a change.

[SwoopX]

This will work, but would it be cleaner to implement this logic only once, in ResourceItem::toVariant()? Ideally, I’d create a subclass for RAttrNwkAddress, but that seems too big a change.

@ebaauw Thanks Erik for the great suggestion. I always tend to overlook those functions, dunno why :) If @manup agrees, I'm more than happy to move the code into the toVariant() function.

[manup]

Hmm no not really. I'm against bringing item specifics into toVariant() or even ResourceItem this method should know nothing about specific items. The moment we start this it would grow to another monster function over time. toVariant() is called in myny context which aren't export to REST-API. Note the RAttrNwkAddress item was never meant to be exposed to the API, it's main purpose is for internal addressing in the Device code.

Lets take a step back to see what this is about: exposing the item to the REST-API and here the external presentation is different from the internal one.

I'd propose for now we add a free standing helper function in resource.h/cpp.

QVariant R_ItemToRestApiVariant(const ResourceItem *item)
{
    if (item)
    {
        const ResourceItemDescriptor *rid = item->descriptor();
        if (rid.suffix == RStateUnicorn)
        {
            // do special stuff
            // return special stuff
        }
        return item->toVariant();
    }
    return {};
}

In future this can be specified genericly in the item JSON descriptions like "this item should be exposed as hex string in the REST API". I have some plans here but this will take a while.

[ebaauw]

I'm against bringing item specifics into toVariant() or even ResourceItem this method should know nothing about specific items.

We currently have quite some resource item descriptor specific logic in ResourceItem::toString(), which is called by ResourceItem::toVariant(). To me this is cleaner (less dirty) than having a helper function outside the ResourceItem class.

In future this can be specified genericly in the item JSON descriptions

I like that, but that's a significant refactor, probably beyond the scope of this PR.

[SwoopX]

Let's start small and then let it grow as necessary 😉

Made the suggested changes hoping they're ok.

[manup]

Nice almost there, I have two small change requests.

[manup]

I suggest a small change to save two QString constructions :)

return QString("0x%1").arg(item->toNumber(), 4, 16, QLatin1Char('0')).toUpper();

[SwoopX]

I'm afraid, I did that on purpose, as the suggested amendment does not keep the small letter X. See the output of

deconz-rest-plugin/de_web_plugin.cpp

Lines 4382 to 4383 in 04bd307

	dbgCluster = QString("0x%1").arg(ind.clusterId(), 4, 16, QLatin1Char('0')).toUpper();
	dbgCmd = QString("0x%1").arg(zclFrame.commandId(), 2, 16, QLatin1Char('0')).toUpper();

in the respective logs.

[manup]

Oh ok indeed that makes sense. Until the item.json display extensions are ready that's ok for now.