BASIC class mapping isn't complete

[GoogleCodeExporter]

My question is the following. When I use SetNodeLevel, the value named
"Basic" is updated and the light dims accordingly (as stated in the
doc). However, when I use the wall switch, only the value named
"Level" is updated, ending up in a desync between the two values.
Is this behavior correct, or should the "Basic" value be updated
automatically as well?

Does this mean I need to use the Manager::SetValue method instead of
SetNodeLevel to update the Dimmer?

Original issue reported on code.google.com by glsatz on 6 Dec 2011 at 7:16

[GoogleCodeExporter]

We *should* tie them together, and we do have the information to do it
as well (the command class that is mapped to basic is defined in
device_classes.xml).

Just need someone to write the code...

Mal

Original comment by glsatz on 6 Dec 2011 at 7:16

[GoogleCodeExporter]

Hi Mal,

Thanks for the hint about device_classes.xml. I looked around a bit in
the code this morning, and found out the following:

1) Only if there is no zwcfg_***.xml file, the "basic" mapping
parameter is loaded from device_classes.xml
2) It is stored as m_mapping in the basic command class! Someone had
already thought about this, but the property is only stored and never
used again.
3) It is then easy to use it in node.cpp for setnodelevel/on/off and
target the mapped CommandClass.
4) But the "basic" value does not get updated - should it be updated
as well? (2 messages are being sent if using value->set())

    Next
5) This mapping needs to be stored in the zwcfg_***.xml file, if this
is the basic CC. The WriteXML method was already thought that way
since it is Virtual in CommandClasses.h. However, many variables are
private and need to be changed to protected. Or better -Split the
WriteXML with 2 more helpers, one to write the CC and params, and one
to write instance and values (so we can easily update the parent's
methods instead of copying the whole thing).
6) Read this value at startup.

    Finally
7) Update basic when level is updated through the wall switch. Did not
investigate this yet.

Indeed, implementing item 3 already solved my problem as I only need
to read the "level" value now, and 5-6 will allow me to stop deleting
the config file each time I start the system:) However, the "basic"
value has no more sense (item 4) as it is not updated anymore.
Moreover, I have no idea if the code works with anything else than my
dimmers.

Cheers,
Yannick

Original comment by glsatz on 6 Dec 2011 at 7:16

[GoogleCodeExporter]

I put the mappings in the device_classes.xml, but I never got around
to adding in the code to support it.  Every command class that can be
mapped needs to have code that causes the Basic value's OnValueChanged
to be called (we need a new method - we can't use Basic::Set or
Basic::SetLevel, because these cause a message to be sent to the
device).

In Basic::SetLevel, we need to do the reverse - find the command class
that basic maps to and call the appropriate Set function.

This is fairly ugly, since it breaks a lot of the independence we had
between the command class implementations.  I probably put off doing
it in the hope that something neater would come along...

Mal

Original comment by glsatz on 6 Dec 2011 at 7:17

[GoogleCodeExporter]

609 has basic to sensor binary mapping. 

Original comment by glsatz on 5 Jan 2013 at 3:45

• Changed state: Started

[GoogleCodeExporter]

I did a patch for a mapping between basic and Switch_Binary as well as 
Switch_Multilevel.

Please note that I also updated CommandClass::UpdateBasic to allow setting 
basic to an arbitrary level (not only 0 and 255). This is required to 
synchronize Level and Basic on Dimmers. For binary switch I call UpdateBasic 
with either 255 or 0, which is the same as binary sensor.

However Issue 166 should be resolved for correct functionality: 
http://code.google.com/p/open-zwave/issues/detail?id=166

Original comment by matthias...@googlemail.com on 10 Feb 2013 at 2:34

Attachments:

• sync_basic.patch

[GoogleCodeExporter]

The latest repository version, r643 should support your SwitchBinary as well as 
SwitchMultilevel based off your patch. The code was simplified based on a 
conversation with Mal on his original intent:

The problem seems to exist because we're trying to update two ValueIDs.

When I first introduced the mapping idea, my intention was that we should
only create one ValueID - the one for the mapped class.  We would still
create a basic command class object, but it would not call CreateVars.

With only one ValueID, there is only one thing for the user to play with.
We then only have to deal with incoming Basic messages.  My plan was to have
Basic::HandleMessage check whether it was remapped and to call a method on
the mapped class (we would need a special handler for all mapped classes).

Mal

Original comment by glsatz on 15 Feb 2013 at 5:31