Setting value label bug

[kdschlosser]

If I set the label of a value with an instance higher then 1 it also sets the label for all other value instances of the same command class including the root instance.

I have a landscape lighting controller that has the multi channel cc. there are 8 values and the instance numbers are 1, 2, 3, 4, 5, 6, 7 and 8. they all have the same cc of COMMAND_CLASS_SWITCH_BINARY. they are all set with the default label of "Switch". I would like to be a little more descriptive as to what area of lights that instance controls. If I set instance 3 to "Deck Lights" all 8 instances change to "Deck Lights". this is less then ideal.

In the user config file it shows the 8 separate elements one for each instance. and each one has it's own attributes just like a normal value. but somewhere somehow they are all joined together.

Also is there a reason as to why there is no SetInstanceLabel? This could be useful for setting a location name if the node is a multi channel node. here is a use example. with that lighting controller it would be handy to be able to define a location for each of the zones.

[Fishwaldo]

Honestly - I don't think anybody actually uses the SetLabel calls at all... hence why you caught this. SetLabel will affect all instances. You differentate instances via the InstanceLabels, but as you point out there is no SetInstanceLabel call. At the moment the only option is via config files.

[petergebruers]

I think in 99% of all cases the "config file option" is a good one, and the config file can help to distinguish the different elements of a device. This means the config file refers to a property of the device, for example if it has two outputs those might be marked Q1 and Q2. I think it is the task of that upper layer to match Q1 with "Deck Lights" and Q2 with "Patio". I think it makes sense to have both pieces of information available to the user. After all, the output on that module is marked Q1 for everyone.

@kdschlosser if you want to test it, you'll have to add or edit the config or your ozwcache like this (do not edit the Value element of the xml):

  <CommandClass id="37" name="COMMAND_CLASS_SWITCH_BINARY">
    <Instance index="1" endpoint="1" label = "Output Q1"/>
    <Instance index="2" endpoint="2" label = "Pin A7"/>
  </CommandClass>

This will label them as:

Output Q1 Switch
Pin A7 Switch

I do have a corner case for "SetInstanceLabel" label though :wink:

I used my Z-Uno to test this and because it can make arbitrary devices, we cannot make a single config file that fits everyone's needs... I am perfectly happy with "Instance 1 Switch" as generic name for the device but it would be nice to be able to change that to "Relay 7" for example.

To solve that "uniqueness" problem, there was a request on the Z-Wave.me forum to define a property or even the type ID so gateway implementers can distinguish more than one type of of Z-Uno (sketch, implementation, variant or whatever you want to call them).

[Fishwaldo]

I do have a corner case for "SetInstanceLabel" label though

Yep - I can see a usecase for it as well... Just wonder how many actual applications would adopt it via the OZW Api :)

Most apps seem to maintain a internal node/entity map to OZW ValueID etc, and right now, any Set*Label call would be lost if the ozwcache file is removed etc. Ideally, if we beef up support in this space, I think we have to consider a persistant storage option on top of the cache.

(TBH, I was considering removing all the SetLabel/SetHelp etc type calls from OZW in a future release)

[kdschlosser]

the purpose for it I am thinking is as I pointed out above with a "zoned" device where the zones are not in the location of the actual node. even something like a multi relay node that controls lighting in more then a single room of the house. It is nice to be able to identify them by something other then "Instance 1: Switch", "Instance 2: Switch" ....

I did a test run with adding the SetInstanceLabel and it worked without issue.

Manager.cpp

//-----------------------------------------------------------------------------
//  Instances
//-----------------------------------------------------------------------------

//-----------------------------------------------------------------------------
// <Manager::SetInstanceLabel>
// Sets the user-friendly Instance label for the valueID
//-----------------------------------------------------------------------------
bool Manager::SetInstanceLabel(ValueID const &_id, string label)
{
    return SetInstanceLabel(_id.GetHomeId(), _id.GetNodeId(), _id.GetInstance(), label);
}

//-----------------------------------------------------------------------------
// <Manager::SetInstanceLabel>
// Sets the user-friendly Instance label for the cc and instance
//-----------------------------------------------------------------------------
bool Manager::SetInstanceLabel(uint32 const _homeId, uint8 const _node, uint8 const _instance, string label)
{
    if (Driver* driver = GetDriver(_homeId))
    {
        Internal::LockGuard LG(driver->m_nodeMutex);
        if (Node* node = driver->GetNode(_node))
        {
            node->SetInstanceLabel(_instance, (char *)label.c_str());
            return true;
        }
        OZW_ERROR(OZWException::OZWEXCEPTION_INVALID_NODEID, "Invalid Node passed to SetInstanceLabel");
        return false;
    }
    else
    {
        OZW_ERROR(OZWException::OZWEXCEPTION_INVALID_HOMEID, "Invalid HomeId passed to SetInstanceLabel");
        return false;
    }
    return false;
}

Manager.h

                        /**
             *\brief Set the Global Instance Label for an instance
             *\param _id The unique identifier of the value
             *\return true if successful
             */
            bool SetInstanceLabel(ValueID const &_id, string label);

            /**
             *\brief Set the Global Instance Label for an instance.
             *\param _homeId the HomeID for the network you are querying
             *\param _node The Node you are interested in.
             *\param _instance the Instance you are querying for
             *\return true if successful
             */
            bool SetInstanceLabel(uint32 const _homeId, uint8 const _node, uint8 const _instance, string label);

Node.cpp

void Node::SetInstanceLabel(uint8 const _instance, char *label)
{
    m_globalInstanceLabel[_instance] = string(label);
}

Node.h

                        /**
             * This function sets a Global Instance Label for all CommandClasses that don't define their
             * own labels
             */
            void SetInstanceLabel(uint8 const _instance, char *label);

[petergebruers]

It is nice to be able to identify them by something other then "Instance 1: Switch", "Instance 2: Switch" ....

I agree, but as @Fishwaldo said the "ozwcache" file is not really a database, and if a user has issues we sometimes ask to remove it and it should rebuild. The Name and Location CC already caused confusion because few device store that information.

Maybe check with HASS how that would affect "entity names" (and what would you want them to be?).

[kdschlosser]

I see where you are coming from. python-openzwave originally offered use of sqlite for storing information. Using any kind of a sql database to store information about a zwave network is a complete waste of resources. xml can be used yaml as well. a csv file would also work. there is not that much information to store when dealing with 255 max nodes.

I do believe there should be 2 files in openzwave one that is a map of the network and contains only static information. and there should be a cache file that houses any dynamic information. while I do understand that an application can offer things like what I stated about the labels and create their own "database" on the network.. then what is the point in caching any of the information in openzwave then? having to maintain 2 data sets for the same device is kind of loony tunes. the mechanics are there in openzwave. it already caches information about the network then what would be the harm in also storing use defined information as well? It is not really any different then storing dynamic values from the node. In the end they are still user set. If there is no want to offer any convenience with the API may as well pitch the majority of the API and just shoot for an API that will only interpret the serial data and send notifications about the data that has come in. The application would have to maintain a cache if they wanted to. this way everything is all in a single location. all of the code for handling this is also in a single spot. This might be the best way to go. only offer information contained within the device config files. and leave it up to an application to handle storing whatever data it is they decide is necessary.

[kdschlosser]

and I also only did the above to see how involved it was to be able to add the SetInstanceLabel method. As I have said in the past. I learn by doing so it is never a waste of time when I add or change code in openzwave that is not going to get used. It's also how I am familiarizing myself with where different bits and pieces are inside of openzwave.