search

cgarwood / homeassistant-zwave_mqtt

Limited Pre-Release of the new OZW1.6 Z-Wave component. Currently has limited platform support. Check the README for more details.
72 stars 8 forks source link

Thermostat has multiple climate entities #116

Closed kpine closed 4 years ago

kpine commented 4 years ago

My thermostat which supports COMMAND_CLASS_THERMOSTAT_MODE has 4 separate climate entities, one for each setpoint. These should be combined into a single climate entity, as is done in HA core Z-Wave.

image

marcelveldt commented 4 years ago

Can you provide me with a mqtt dump please so I can test this ?

kpine commented 4 years ago

Sure, but the problem is the discovery schema. Compare the one here vs. HA core.

marcelveldt commented 4 years ago

Sure, but the problem is the discovery schema. Compare the one here vs. HA core.

May be true but I'd still like to receive the dump ;-)

kpine commented 4 years ago

You're too fast! Here it is.

mqtt_dump.txt

marcelveldt commented 4 years ago

Thanks, this will allow me to reproduce as I do not own climate devices.

kpine commented 4 years ago

FYI, here's what's missing: https://github.com/home-assistant/core/blob/d43617c41d6507f2d2b77aadf4fa1ebaf0058b14/homeassistant/components/zwave/discovery_schemas.py#L95

marcelveldt commented 4 years ago

Ah, I guess that was added to the core zwave after this project was initiated ;-)

kpine commented 4 years ago

I think that's true. I should have remembered to mention it before.

Also, this core issue should be fixed: https://github.com/home-assistant/core/issues/30180

marcelveldt commented 4 years ago

I think that's true. I should have remembered to mention it before.

Also, this core issue should be fixed: home-assistant/core#30180

I'd rather focus on getting this project stable but feel free to submit a PR for that specific issue.

joshtbernstein commented 4 years ago

@marcelveldt Do you still need a mqtt dump from me also? Thanks to @kpine for opening this issue.

marcelveldt commented 4 years ago

@marcelveldt Do you still need a mqtt dump from me also? Thanks to @kpine for opening this issue.

If you own a different thermostat than kpine: yes please

kpine commented 4 years ago

I'd rather focus on getting this project stable but feel free to submit a PR for that specific issue.

Instead, I should have said, please don't break those thermostat types when fixing this, because they aren't broken here yet.

marcelveldt commented 4 years ago

I'd rather focus on getting this project stable but feel free to submit a PR for that specific issue.

Instead, I should have said, please don't break those thermostat types when fixing this, because they aren't broken here yet.

Haha, true. Let's try to make this version a lot better!

cgarwood commented 4 years ago

Ah yeah, the climate change in the current core component came after I copied the discovery schema's over to this project and this project was never updated to include them. I had actually forgotten about that until now.

marcelveldt commented 4 years ago

Ah yeah, the climate change in the current core component came after I copied the discovery schema's over to this project and this project was never updated to include them. I had actually forgotten about that until now.

No problem, I'm working through it now. Unfortunately it wasn't as easy as copying the discovery schema.

joshtbernstein commented 4 years ago

@marcelveldt Here you go! Please let me know what else I can do to help with this :-)

mqtt_dump.txt

Dinth commented 4 years ago

Im also seeing this. Screenshot from 2020-05-01 12-50-32 https://gofile.io/?c=rrUiJ2

joshtbernstein commented 4 years ago

@marcelveldt I just pulled down the master using HACS and I'm not seeing any change, I still have two climate entities. I removed and readded the integration and no change. Anything I can do to help troubleshoot?

marcelveldt commented 4 years ago

@marcelveldt I just pulled down the master using HACS and I'm not seeing any change, I still have two climate entities. I removed and readded the integration and no change. Anything I can do to help troubleshoot?

No, just have some patience ;-)

joshtbernstein commented 4 years ago

No, just have some patience ;-)

Sorry, I'll be patient!

marcelveldt commented 4 years ago

I just published version .15 to HACS, please have a test with that. As we're already implementing the code into core Hass it would be great to get some feedback if this issue is resolved.

WARNING: version .15 contained a change to the internal id matching so it is normal that you need to rename your entities/devices again.

kpine commented 4 years ago

Looks good to me. My CT32 with thermostat mode has a single climate entity now.

There is something wrong with the preset behavior though, but I need to look more. Will log that separately if it turns out to be an issue.

marcelveldt commented 4 years ago

As the originally reported issue is resolved, I'll close this issue. The climate platform will be merged to the core implementation with these adjustments included.

marcelveldt commented 4 years ago

This issue will be fixed in the core implementation so please keep an eye on the next beta version of Home Assistant. Thanks for your help testing and reporting the issue.