Adding helper for get and set values

[turbokongen]

Description:
Ref: #5619 (review)
I'm not sure if this is any helpful at all, only gaining 6 less lines of code.
Discussion open :)

[mention-bot]

@turbokongen, thanks for your PR! By analyzing the history of the files in this pull request, we identified @devdelay, @armills and @leoc to be potential reviewers.

[emlove]

I think this is better than what's there right now, but I'm going to use this as a discussion for a further refactoring I've been mulling over for a while. The root use-case for these loops are Entities that need to reference more than one ZWave Value. Currently, entities are tied to one value, which is passed in when the entity is created.

Right now we're looping over all the node values every time a different value needs to be looked up. The way I'd ultimately like to implement this is to refactor the discovery code to search for all the values that an entity needs. Right now in DISCOVERY_COMPONENTS, each item is a description of one ZWave value that will trigger the creation of the underlying entity. I think we could come up with a way to expand these definitions to include additional values that should also be passed to the entity if they are discovered on the network. I implemented something of a first-pass at this for ZWave color lights, but this should be generalized in the ZWave component code.

This way the ZWave entities will get a reference to the extra values they need as soon as its discovered by the ZWave network, and they won't need to do the search each time they need it.

[emlove]

member should be dropped here, since it's expecting a reference to the Value object.

[turbokongen]

So this is expecting the object, and not the value.data?

[turbokongen]

Done 👍

[emlove]

Right, this is where it's used.

[emlove]

My preference would be to make this an internal method, and add wrapper functions get_value and set_value. I think it's quicker to read and understand self.get_value/self.set_value in the code instead of self.value_handler.

[turbokongen]

That was my initial thought, but since the code is nearly the same for both, I found it better to have an argument.

[emlove]

I just mean something small like this, just to make it easier to read, but still keeping all the code common. Up to you, though.

def get_value(**kwargs)
    return self._value_handler(method='get', **kwargs)

[turbokongen]

I didn't think of it that way :)
I will do that. 👍

[houndci-bot]

continuation line over-indented for visual indent

[houndci-bot]

continuation line over-indented for visual indent

[houndci-bot]

continuation line over-indented for visual indent

[houndci-bot]

continuation line over-indented for visual indent

[houndci-bot]

continuation line over-indented for visual indent

[houndci-bot]

continuation line over-indented for visual indent

[andrey-git]

This was releasing either the 'Open' or 'Down' button

[turbokongen]

Since we now get the value_id of a valid entry for the node, we don't need both.
https://github.com/turbokongen/home-assistant/blob/f44c55d1012a7b5eb5058197b21c271d50e79e68/homeassistant/components/cover/zwave.py#L81

[andrey-git]

I wasn't clear: Previously it was releasing both buttons: Open for open and Down for close (maybe should have released all 4 names).

But now you are only releasing _open, so I would expect stop_cover not to work while the cover is closing.

[turbokongen]

The way the releasbutton command in OZW works is not like the naming would suggest.
releasebutton triggers a stop command, and pressbutton triggers a start command.

https://github.com/OpenZWave/open-zwave/blob/master/cpp/src/Manager.cpp#L3028

[turbokongen]

Also, before I had Open or Down as stop alternatives, because it would work on either type of devices regardless if they had Open, Close or Up, Down label pairs.

[andrey-git]

Are you saying the ID doesn't matter?
value->ReleaseButton is called on a specific ID.

I don't have a cover device to test this. Does this work for you for stopping both open & close movement?

[turbokongen]

I don't have a cover device either, but yes. Calling the release button on either open,close or up,down labels will stop the level change. That is how the old code was made. 👍

[andrey-git]

Why did you drop the rounding?

[turbokongen]

Changes must have been made to OZW since I made the HVAC/climate component. Then it was not working without rounding. Now it responds perfectly to the set .5 intervals.

[andrey-git]

Did you drop the temperature rounding on purpose?

[turbokongen]

Yes, rounding is already done here:https://github.com/turbokongen/home-assistant/blob/f44c55d1012a7b5eb5058197b21c271d50e79e68/homeassistant/components/climate/__init__.py#L697

[turbokongen]

The last committ, I need somebody to test if that really works, as this is just based on how I read the code.

[andrey-git]

@turbokongen I don't have a cover device :(

[balloob]

🐬 ok to merge when you have resolved the conflicts.

[balloob]

@armills I like that idea, should we move that discussion to an issue?

[emlove]

@balloob I'll probably just open a PR at some point with at least a proof of concept, and we can discuss it there.