audiconnect / audi_connect_ha

Adds an audi connect integration to home assistant
MIT License
226 stars 98 forks source link

fix: Treat SunRoof as Window #367

Closed coreywillwhat closed 5 months ago

coreywillwhat commented 5 months ago

Summary

The sunRoof should be treated as a window.

358 fixed the sunroof becoming unavailable, but during testing showed it was always returning Open. This fixes that and groups it with the other windows, following the same logic.

Technical

Tested and working as expected

Kolbi commented 5 months ago

I still don't know why it should always be open for sun roof.

value": "3" if status[0] == "closed" else "0",

Closed = 3 Open = 0 ?

Your change now only passes 0 (= Open)?

cdnninja commented 5 months ago

The sensor is HA is looking for true false but also reverts to 0,1 being valid as well. Anything outside or 0,1 doesn't map to true or false. Not sure that helps, I haven't looked closely at this.

coreywillwhat commented 5 months ago

return self._vehicle.fields.get("STATE_SUN_ROOF_MOTOR_COVER") != "3" So if sensor != closed (3) return True (Open) If sensor = closed (3) return False

As far as I can tell, the gods forged this logic alongside fire and the wheel. I can't fight it or tell you why its designed this way.

coreywillwhat commented 5 months ago

if you directly pass 3 to HA, wouldn't it be considered True (Open)? I would think then if passing 0 it would be returning false. Maybe I didn't try closing the sunroof, I thought I had. Its possible the values for yours @Kolbi were just inverted, rather than always "Open".

cdnninja commented 5 months ago

I had this pain in my Kia integration too. Kia uses numbers that actually mean more than I thought surface level.

A example that's made up 0 may mean closed 1 tilt 2 partial open 3 fully open.

I had this with seats thinking it was just true false but only to find later some cars report the power level.

coreywillwhat commented 5 months ago

I'm pretty sure you're right and the numbers were used in an old API and "3" was introduced around an issue with a 2 door car that didn't report rear doors/windows. The logic was probably adjusted at some poitn with an API change where, like now, the API is actually reporting "closed" and "open".

Might be worth a clean up at some point. That said, this code is tested and working. Tried open, close, and tested other windows with the any window open logic and its all working as expected.

Given the back and forth I'll wait for either of you to approve/merge.

Kolbi commented 5 months ago

Sorry I had no sun roof to test and have a lack of an testing environment. Sorry my home base is not Python :) I merged the PR.

coreywillwhat commented 5 months ago

no worries at all, thanks @Kolbi

Kolbi commented 5 months ago

return self._vehicle.fields.get("STATE_SUN_ROOF_MOTOR_COVER") != "3" So if sensor != closed (3) return True (Open) If sensor = closed (3) return False

Now i understand this point. :) I thought "return self._vehicle.fields.get("STATE_SUN_ROOF_MOTOR_COVER")" returns the value from self._vehicle.fields.get("STATE_SUN_ROOF_MOTOR_COVER") but it doesn't :D it returns True or False :)

Kolbi commented 5 months ago

My entity binary_sensor.audi_windows is not available anymore.

I guess because: 2024-04-12 08:27:17.252 DEBUG (MainThread) [custom_components.audiconnect.audi_models] Data that will be used for _tryAppendStateWithTs: [...] 'windows': [{'name': 'frontLeft', 'status': ['closed']}, {'name': 'frontRight', 'status': ['closed']}, {'name': 'rearLeft', 'status': ['closed']}, {'name': 'rearRight', 'status': ['closed']}, {'name': 'roofCover', 'status': ['unsupported']}, {'name': 'sunRoof', 'status': ['unsupported']}], [...]

Because I don't have an entity sun_roof at all: https://github.com/audiconnect/audi_connect_ha/blob/d4470877d4e4481df0326da564c797ea46610469/custom_components/audiconnect/audi_models.py#L398

So we need to modify: https://github.com/audiconnect/audi_connect_ha/blob/06d588d9512a36fca37b04f9bd9a24dc3b343d6c/custom_components/audiconnect/audi_connect_account.py#L1076 and https://github.com/audiconnect/audi_connect_ha/blob/06d588d9512a36fca37b04f9bd9a24dc3b343d6c/custom_components/audiconnect/audi_connect_account.py#L1093