Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Expose if a device supports dhw, solar thermal or ventilation capabilities #360

Merged
merged 30 commits into from
Nov 11, 2024

Conversation

CFenner
Copy link
Member

@CFenner CFenner commented Feb 9, 2024

This adds methods to check if a device supports general heating, dhw, solar thermal or vertilation capabilities.

This requires #356

Bildschirmfoto 2024-04-05 um 08 29 23

@CFenner CFenner marked this pull request as ready for review February 9, 2024 16:21
@CFenner
Copy link
Member Author

CFenner commented Mar 22, 2024

Can we bring this in, it would be essential to support ventilation devices in HA.

@@ -15,3 +15,19 @@ def __init__(self, service: ViCareService) -> None:
@handleNotSupported
def getSerial(self):
return self.service.getProperty("device.serial")["properties"]["value"]["value"]

@handleNotSupported
def isHeatingDevice(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of renaming this function in something like isDeviceHeatingEnabled ? Because I don't know which impact on this data point is made by changing the active mode of the Heating Device (e.g. DHW only during Summer)
I think the same renaming could be useful for the other functions as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a system where you could verify this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can test it later with my Oil Boiler and Heatbox2

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also a status

Bildschirmfoto 2024-04-03 um 11 13 50

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you are right. I have tested it, changing the mode to DHW Only doesn't affect the ´isEnabled` flag for heating on my System.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also verify if this influences the ["properties"]["active"]["value"] of DHW?
So does turning of the DHW (is that possible?) disable the flag?

Copy link
Contributor

@Nibot1 Nibot1 Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I disable DHW on my boiler, the feature heating.dhw changes isEnabled to false and removes the properties
{ "feature": "heating.dhw", "gatewayId": "xxxx", "deviceId": "0", "timestamp": "2024-04-04T16:01:14.844Z", "isEnabled": false, "isReady": true, "apiVersion": 1, "uri": "https://api.viessmann.com/iot/v1/features/installations/xxx/gateways/xxxxxx/devices/0/features/heating.dhw", "properties": {}, "commands": {} }
To disable dhw I need to reconfigure my boiler and remove the dhw temp. Sensor
So my Boiler acts as a device where DHW is possible but not enabled (probably isReady shows this)


@handleNotSupported
def isHeatingDevice(self):
return self.service.getProperty("heating")["isEnabled"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be nice to catch the PyVicareNotSupported error internally, rather than relying on the implementation of error handling from the Users?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, this should return false if it does not exists.

@CFenner CFenner marked this pull request as draft April 3, 2024 09:10
self.assertFalse(self.device.isHeatingDevice())

def test_isDomesticHotWaterDevice(self):
self.assertTrue(self.device.isDomesticHotWaterDevice())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason the vitovent says it is a dhw device. Maybe this is all not that accurate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SHuechtemann can you say why your Vitovent has an active DomesticHotWater flag?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk. Maybe the description is inacuarate/misleading as my vivovent has an active Heat exchanger and is capable to heat up the incoming air...

Copy link

@SHuechtemann SHuechtemann Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it has a bypass function to bypass the heat exchanger. Maybe this is what the flag is for, showing if bypass is active or inactive. This should be in the Response from the API, as the viessmann App also shows this.

@CFenner
Copy link
Member Author

CFenner commented Apr 3, 2024

@woehrl01 what shall I do about the failing test for newly added properties?

@woehrl01
Copy link
Collaborator

woehrl01 commented Apr 3, 2024

@CFenner either implement the methods or add them to the ignore list (I'm fine with the later)

@CFenner
Copy link
Member Author

CFenner commented Apr 4, 2024

There seem to be cases where "feature": "heating" is not available at all: #327 #345

@Nibot1
Copy link
Contributor

Nibot1 commented Apr 4, 2024

@CFenner my boiler also doesn't show up the "feature": "heating" what about testing the "feature": "heating.circuits" to solve this? I would believe that a device which is capable of heating must show a Circuit, and the devices which don't have heating capabilities won't show circuits.

@CFenner
Copy link
Member Author

CFenner commented Apr 4, 2024

Yes, I can check that.

I wonder if this could also be due to outdated api dumps.

@Nibot1
Copy link
Contributor

Nibot1 commented Apr 4, 2024

I don't think so because my API dumps are just a few minutes old.

@CFenner CFenner closed this Aug 15, 2024
@CFenner CFenner deleted the patch-7 branch August 15, 2024 15:24
@LarsTh86
Copy link

LarsTh86 commented Sep 3, 2024

Is there progress for the implementation of the ventilation? can I test something?

@CFenner CFenner restored the patch-7 branch October 5, 2024 06:56
@CFenner CFenner reopened this Oct 5, 2024
@CFenner CFenner changed the title Add ability to verify if a device supports heating, dhw, solar thermal or ventilation Add ability to verify if a device supports dhw, solar thermal or ventilation Oct 5, 2024
@CFenner CFenner added the enhancement New feature or request label Oct 7, 2024
@CFenner CFenner marked this pull request as ready for review October 7, 2024 18:42
@LarsTh86
Copy link

LarsTh86 commented Oct 7, 2024

How i can test this?

@CFenner CFenner changed the title Add ability to verify if a device supports dhw, solar thermal or ventilation feat: Expose if a device supports dhw, solar thermal or ventilation capabilities Nov 11, 2024
@CFenner CFenner merged commit 67a1fd4 into openviess:master Nov 11, 2024
9 checks passed
@CFenner CFenner deleted the patch-7 branch November 11, 2024 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants