OpenZWave / node-openzwave-shared

OpenZWave addon for Node.js (all versions) including management and security functions
Other
199 stars 113 forks source link

Value structure is incorrect #208

Closed vibr77-zz closed 5 years ago

vibr77-zz commented 6 years ago

Hello, After playing with this lib, I found something incorrect in the value structure for type 'list' with item

Let consider the following value extract from the zwave config file "

`

Which commands to send when PIR motion sensor triggered

`

On the event

zwave.on('value added', function(nodeid, comclass, value) {}

The content of value is

info: value_id=89-112-1-5, node_id=89, class_id=112, type=list, genre=config, instance=1, index=5, label=Command Options, units=, help=Which commands to send when PIR motion sensor triggered, read_only=false, write_only=false, min=1, max=2, is_polled=false, values=[Basic Set (default), Binary Sensor Report], value=Basic Set (default)

The field values only contains label and no value associated, There should be an associative array,

How to fix this ?

Thanks for your help Vincent

ekarak commented 6 years ago

It seems a bit confusing at first. A ValueType_List is the equivalent of an 'enum'. You typically call setValue() with one of the options provided (ie [Basic Set (default), Binary Sensor Report]) - you dont need to use the indices. What is your usecase?

vibr77-zz commented 6 years ago

Hello Ekarak, I used to play with the C lib of openzwave and the set a value with a type of list we use the value and not the label taken if the config file

if you look at the following ex `

The parameter determines the behaviour of tamper and how it reports. Tamper: Tamper alarm is reported in Sensor Alarm command class. Cancellation: Cancellation is reported in Sensor Alarm command class after the time period set in parameter 22 (Tamper Alarm Cancellation Delay). Orientation: Sensor's orientation in space is reported in Fibaro Command Class after the time period set in parameter 22. Vibration: The maximum level of vibrations recorded in the time period set in parameter 22 is reported. Reports stop being sent when the vibrations cease. The reports are sent in Sensor Alarm command class. Value displayed in the "value" field (0 - 100) depends on the vibrations force. Reports to the association groups are sent using Sensor Alarm command class. Default setting: 0 (Tamper)
                <Item label="Tamper" value="0" />
                <Item label="Tamper + Cancellation" value="1" />
                <Item label="Tamper + Orientation" value="2" />
                <Item label="Tamper + Cancellation + Orientation" value="3" />
                <Item label="Vibration" value="4" />
            </Value>`

The value expected if between 0 and 4 If openzwave doing the reversal for ex from 'Vibration' to 4 ?

Thanks Vincent

vibr77-zz commented 6 years ago

Another Example


<Value type="list" genre="config" instance="1" index="34" label="Reaction to alarms" units="" read_only="false" write_only="false" verify_changes="false" poll_intensity="0" min="1" max="63" vindex="6" size="1">
                    <Help>Type of transmitted control frame for association group 1, activated via input IN1. The parameter allows to specify the type of alarm frame or to force transmission of control commands (BASIC_SET)</Help>
                    <Item label="ALARM GENERIC" value="1" />
                    <Item label="ALARM SMOKE" value="2" />
                    <Item label="ALARM CO" value="4" />
                    <Item label="ALARM CO2" value="8" />
                    <Item label="ALARM HEAT" value="16" />
                    <Item label="ALARM WATER" value="32" />
                    <Item label="ALARM ALL" value="63" />
                </Value>

This a very common config list for one value, as you can use, each item is bound to a label and a value How can we have the same (an associative array) under the field values ?

vibr77-zz commented 6 years ago

Ok one workaround is modify the util.cc with the following

case OpenZWave::ValueID::ValueType_List: {
                std::string val;
                std::vector < std::string > items;
                std::vector < int32> itemsvalues;

                // populate array of all available items in the list
                OpenZWave::Manager::Get()->GetValueListItems(value, &items);
                OpenZWave::Manager::Get()->GetValueListValues(value, &itemsvalues);

                for (int i = 0; i != items.size(); i++){
                    items[i]=std::to_string(itemsvalues[i])+"|"+items[i];
                }

                AddArrayOfStringProp(valobj, values, items);
                // populated selected element
                OpenZWave::Manager::Get()->GetValueListSelection(value, &val);
                AddStringProp(valobj, value, val.c_str())
                break;
            }

Then in the values list there is now the "Value|label" it is easy to split the str to get the value and the label !

I am no expert in NAN but it works waiting for a less dirty solution and more integrated

vibr77-zz commented 6 years ago

Ok to be consistent with the setConfigParam one more modification the value val has to be the value and not the label thus changing from std:string to int32 and the output

case OpenZWave::ValueID::ValueType_List: {
                int32 val;
                std::vector < std::string > items;
                std::vector < int32> itemsvalues;

                // populate array of all available items in the list
                OpenZWave::Manager::Get()->GetValueListItems(value, &items);
                OpenZWave::Manager::Get()->GetValueListValues(value, &itemsvalues);

                for (int i = 0; i != items.size(); i++){
                    items[i]=std::to_string(itemsvalues[i])+"|"+items[i];
                }

                AddArrayOfStringProp(valobj, values, items);
                // populated selected element
                OpenZWave::Manager::Get()->GetValueListSelection(value, &val);
                //AddStringProp(valobj, value, val.c_str())
                AddIntegerProp(valobj, value, val);
                break;
            }
ekarak commented 6 years ago

can you please provide this as a pull request, also updating the documentation? apologies,I can't test this as I don't have "complex" devices,only a few switches (and 1 dimmer)

vibr77-zz commented 5 years ago

I have corrected the pull request to be compiled on every version I have tested the functional part it is working Vincent