core icon indicating copy to clipboard operation
core copied to clipboard

Lyric Honeywell integration - fails with leak detector sensors

Open tskirvin opened this issue 1 year ago • 13 comments

The problem

When trying to add Lyric Honeywell integration, it fails for the WLD3 water leak sensor. This appears to be an ongoing problem, based on the device not providing a macID (it instead offers a deviceID, which looks like it should be on all items). It sure feels like a simple try statement around the two calls to macID would solve this?

This is probably a repeat of 88471 (and 52528 before that), but I can't re-open that.

What version of Home Assistant Core has the issue?

core-2024.4.3

What was the last working version of Home Assistant Core?

(none)

What type of installation are you running?

Home Assistant Container

Integration causing the issue

lyric

Link to integration documentation on our website

https://www.home-assistant.io/integrations/lyric/

Diagnostics information

2024-05-12 19:25:35.876 ERROR (MainThread) [homeassistant.components.climate] Error adding entity None for domain climate with platform lyric
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 7
50, in _async_add_entity
    device = dev_reg.async_get(self.hass).async_get_or_create(
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/device_registry.py", line 6
57, in async_get_or_create
    connections = _normalize_connections(connections)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/device_registry.py", line 1
224, in _normalize_connections
    (key, format_mac(value)) if key == CONNECTION_NETWORK_MAC else (key, value)
          ^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/device_registry.py", line 3
79, in format_mac
    if len(to_test) == 17 and to_test.count(":") == 5:
       ^^^^^^^^^^^^
TypeError: object of type 'NoneType' has no len()

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

Sample json from the logs of a particular water sensor:

{'devices': [{'waterPresent': False, 'currentSensorReadings': {'time':          '2024-05-12T12:46:18', 'temperature': 20.85, 'humidity': 57.7}, 'currentAlarms':[], 'lastCheckin': '2024-05-12T12:46:20', 'lastDeviceSettingUpdatedOn':         '0001-01-01T00:00:00', 'batteryRemaining': 29, 'isRegistered': True,            'hasDeviceCheckedIn': True, 'isDeviceOffline': False, 'firstFailedAttemptTime': '0001-01-01T00:00:00', 'failedConnectionAttempts': 0, 'wifiSignalStrength': -48,'time': '2024-05-12T12:46:19', 'deviceClass': 'LeakDetector', 'deviceType':     'Water Leak Detector', 'deviceID': 'a481b5a7-47cf-4a68-9374-b181b7f55452',      'deviceInternalID': 316664, 'userDefinedDeviceName': 'Laundry', 'backend': {},  'isAlive': True, 'isUpgrading': False, 'isProvisioned': True, 'deviceSettings': {'temp': {'high': {'limit': 37}, 'low': {'limit': 7}}, 'humidity': {'high':     {'limit': 70}, 'low': {'limit': 20}}, 'userDefinedName': 'Laundry',             'buzzerMuted': False, 'checkinPeriod': 24, 'currentSensorReadPeriod': 60},      'service': {'mode': 'Up'}, 'deviceRegistrationDate':
'2017-07-06T20:52:20.1266667', 'deviceVariant': 'WLD3'}]}

tskirvin avatar May 13 '24 01:05 tskirvin

Hey there @timmo001, mind taking a look at this issue as it has been labeled with an integration (lyric) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of lyric can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Renames the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign lyric Removes the current integration label and assignees on the issue, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


lyric documentation lyric source (message by IssueLinks)

home-assistant[bot] avatar May 13 '24 01:05 home-assistant[bot]

I looked over the code and it appears that it only supports thermostats and room sensors. Other devices in the ecosystem like water leak sensor, security, and camera don't appear to be supported. So I'm guessing that this would need be a feature request but I'll let the developer comment as I am not one.

I have water leak sensor too so I would vote for the feature request.

wcookjm avatar Jun 01 '24 18:06 wcookjm

That's why I included the yaml snippet; it was closer than I expected! The right fields look to be there.

tskirvin avatar Jun 03 '24 19:06 tskirvin

I don't think timmo has been able to dedicate any time to support the integration. I was hoping to request Water Leak devices added to the aiolyric package, but issues are getting marked as stale and auto closed.

Looks like 88471 and 52528 were in support of similar instances with non-thermostat devices, but also had to be resolved by someone other than the code owner.

Seems any additional support of the Lyric package for non-thermostat devices may need to come from someone else in the community.

rewardone avatar Jun 06 '24 01:06 rewardone

I'm possibly willing to go at least test my "this is fairly easy" theory, but only if there's somebody that can take my pull commit if necessary. Is there anybody?

tskirvin avatar Jun 07 '24 19:06 tskirvin

Timmo did accept a pull request in aiolyric at the end of last year, so I'd say it's a possibility.

rewardone avatar Jun 07 '24 20:06 rewardone

Another vote for this feature. I'm interested in these sensors simple because my home owners insurance has a "Connected Home" discount if you have at least two of these sensors. I already have other leak sensors that can turn off my main water valve, but they won't accept sensors/automation that actually do something, only ones that will email me if there is leak. I guess their idea of a connected home is from 1995.

I'm still planning on buying them, but if they were integrated into HA, I could add them to other areas of my home to increase reliability.

tamorgen avatar Jul 06 '24 00:07 tamorgen

I'm possibly willing to go at least test my "this is fairly easy" theory, but only if there's somebody that can take my pull commit if necessary. Is there anybody?

If we can get this working I'm pretty sure we can scare up some Home Assistant people to help out if @timmo001 is not available to merge fixes.

It looks like the water sensors return a deviceID that is a GUID and the current Lyric component is relying on macID as a dictionary key for storing devices. I don't have a thermostat, so I can't tell for sure, but the API documentation makes it look as though thermostats have a deviceID as well? So maybe if that dictionary is re-keyed to use deviceID then these devices can just be added as sensors?

k7hpn avatar Jul 09 '24 02:07 k7hpn

I did make a run at it, and it was (unsurprisingly) not as simple as I'd like. I have to admit that switching over to deviceID whole-sale might work better, but that'd actually affect existing users and I didn't see a good enough test suite to do it anyway.

So - I think it still could be done. I still think it's worth doing. I'm not sure I have a quick solution.

tskirvin avatar Jul 09 '24 19:07 tskirvin

@tskirvin, if you need testers, let us know. I just got two of these sensors in today, and going through the Lyric enrollment process, it shows the devices on the Lyric page to enroll, but then when it comes back to HA, the devices aren't there. I'm sure this is the same behavior others are seeing.

tamorgen avatar Jul 11 '24 14:07 tamorgen

Hello, no solution yet?

Regards

tco69 avatar Jul 29 '24 08:07 tco69

I have a problem in the end of Lyric enrollment process. After I select the device from honeywell api site and then link to HA the enrollment open HA and ask in whitch House to add, i select the house but then after a while ha go on error saying 500 internal server error : server got itself in trouble.

robitsrl avatar Aug 07 '24 03:08 robitsrl

I also have two of these water sensors (the Resideo "L1 WiFi Water Leak Detector"), which I also got for the "Connected Home" discount on my homeowner's insurance, which requires us to use this specific brand of water sensor. HA integration would be very nice. Nothing shows up in HA when the integration is added. But I'm able to see the following using the Honeywell API manually:

[ { "locationID": 5555555, "name": "Home", "country": "US", "zipcode": "55555", "devices": [ { "waterPresent": false, "currentSensorReadings": { "time": "2024-09-20T15:00:37", "temperature": 25.39, "humidity": 56.5 }, "currentAlarms": [], "lastCheckin": "2024-09-20T15:00:42", "lastDeviceSettingUpdatedOn": "2024-09-20T15:00:42", "batteryRemaining": 100, "isRegistered": true, "hasDeviceCheckedIn": true, "isDeviceOffline": false, "deviceOfflineTime": "2024-09-22T03:00:42", "firstFailedAttemptTime": "0001-01-01T00:00:00", "failedConnectionAttempts": 0, "wifiSignalStrength": 0, "time": "2024-09-20T15:00:42", "deviceClass": "LeakDetector", "deviceType": "Water Leak Detector", "deviceID": "74d0c5c2-5cab-4425-936b-855c132b9d75", "deviceInternalID": 6676829, "userDefinedDeviceName": "Water Heater", "backend": {}, "isAlive": true, "isUpgrading": false, "isProvisioned": true, "deviceSettings": { "temp": { "high": { "limit": 37 }, "low": { "limit": 7 } }, "humidity": { "high": { "limit": 70 }, "low": { "limit": 20 } }, "userDefinedName": "Water Heater", "buzzerMuted": false, "checkinPeriod": 24, "currentSensorReadPeriod": 60 }, "service": { "mode": "Up" }, "deviceRegistrationDate": "2024-09-20T14:59:57.01", "deviceVariant": "L1_SmartWaterSensor_Retail" }, { "waterPresent": false, "currentSensorReadings": { "time": "2024-09-20T15:06:40", "temperature": 25.26, "humidity": 57.8 }, "currentAlarms": [], "lastCheckin": "2024-09-20T15:06:44", "lastDeviceSettingUpdatedOn": "2024-09-20T15:06:44", "batteryRemaining": 100, "isRegistered": true, "hasDeviceCheckedIn": true, "isDeviceOffline": false, "deviceOfflineTime": "2024-09-22T03:06:44", "firstFailedAttemptTime": "0001-01-01T00:00:00", "failedConnectionAttempts": 0, "wifiSignalStrength": 0, "time": "2024-09-20T15:06:44", "deviceClass": "LeakDetector", "deviceType": "Water Leak Detector", "deviceID": "fec2b1a8-dc96-4c24-b6a5-f5e6a0eaad72", "deviceInternalID": 6676850, "userDefinedDeviceName": "Master Bathroom", "backend": {}, "isAlive": true, "isUpgrading": false, "isProvisioned": true, "deviceSettings": { "temp": { "high": { "limit": 37 }, "low": { "limit": 7 } }, "humidity": { "high": { "limit": 70 }, "low": { "limit": 20 } }, "userDefinedName": "Master Bathroom", "buzzerMuted": false, "checkinPeriod": 24, "currentSensorReadPeriod": 60 }, "service": { "mode": "Up" }, "deviceRegistrationDate": "2024-09-20T15:06:02.9333333", "deviceVariant": "L1_SmartWaterSensor_Retail" } ], "users": [ { "userID": 5555555, "username": "[email protected]", "firstname": "FirstName", "lastname": "LastName", "created": 1726844205, "deleted": -62135596800, "activated": true, "connectedHomeAccountExists": true, "locationRoleMapping": [ { "locationID": 5555555, "role": "Adult", "locationName": "Home", "status": 1 } ], "isOptOut": "False", "isCurrentUser": true } ], "timeZoneId": "Eastern", "timeZone": "Eastern Standard Time", "ianaTimeZone": "America/New_York", "daylightSavingTimeEnabled": true, "geoFenceEnabled": false, "predictiveAIREnabled": false, "comfortLevel": 0, "geoFenceNotificationEnabled": false, "geoFenceNotificationTypeId": 13, "configuration": { "faceRecognition": { "enabled": false, "maxPersons": 2, "maxEtas": 2, "maxEtaPersons": 1, "schedules": [ { "time": [ { "start": "15:00:00", "end": "17:00:00" } ], "days": [ "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday" ] } ] } } } ]

theminor avatar Sep 20 '24 17:09 theminor

Any news to get water leak working ? No one is manteining it ?

robitsrl avatar Sep 24 '24 15:09 robitsrl

Been looking for a way to integrate this for a while now, looks like still no luck?

phrankinstein avatar Sep 24 '24 15:09 phrankinstein

Anybody looking into this?

I noticed Homebridge has a plug-in for Resideo, and they list WiFi Water Leak & Freeze Detector as a supported device. I don't think that's the same as the L1 WiFi Water Leak Detector, but they may be similar enough.

https://github.com/homebridge-plugins/homebridge-resideo

I haven't had a chance to play with it, but I might be able to get the device imported to Homebridge, and from there into HAAS.

tamorgen avatar Dec 09 '24 16:12 tamorgen

I have the same issue - I have several of the Resideo First Alert L1 WiFi Water Leak and Freeze Detectors (provided through the Honeywell Lyric integration) and wanted them in HA. I have a Homebridge instance running as well, and was able to use the above listed Homebridge-Resideo plugin to add the sensors to Home Assistant via the HomeKit Device integration. They work just fine this way, just wish I could add them directly to HA instead of bouncing through Homebridge.

aacentric avatar Dec 29 '24 15:12 aacentric

I have modified the built-in code to support the Water Leak sensors. I don't see any indication that mine are Homekit compatible.

You can see my modifications, do a diff, and choose to load the custom_component into your instance here

I will not work on building a pull request to the core integration because I believe, ultimately, that the AIO Lyric library needs to be updated to support the devices before the integration can be 'properly' updated.

The code is not great, but it seems to function.

rewardone avatar Dec 31 '24 16:12 rewardone

Awesome, that worked perfectly. Thank you!

-Aaron On Dec 31, 2024 at 8:39 AM -0800, rewardone @.***>, wrote:

I have modified the built-in code to support the Water Leak sensors. I don't see any indication that mine are Homekit compatible. You can see my modifications, do a diff, and choose to load the custom_component into your instance here I will not work on building a pull request to the core integration because I believe, ultimately, that the AIO Lyric library needs to be updated to support the devices before the integration can be 'properly' updated. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

aacentric avatar Dec 31 '24 22:12 aacentric

Yes, that worked for me too (once I updated to a more recent version of HA). Thanks!

tskirvin avatar Jan 02 '25 22:01 tskirvin

Note: I'm not closing this because it's still a bug, even if @rewardone's patch does what it's supposed to do.

tskirvin avatar Jan 02 '25 22:01 tskirvin

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

@rewardone any chance you could submit a Pull Request to integrate your patch?

theminor avatar Apr 09 '25 22:04 theminor

@rewardone any chance you could submit a Pull Request to integrate your patch?

I had not planned on it because it's likely that the PR will never be merged by the current maintainer. It might be prudent to eventually fork the AIO Lyric library, support leak sensors 'natively', and then add them to the integration in a consistent fashion.

Since my code is published, I am not stopping anyone from forking my patch and submitting it as a PR if someone has that hope. It might be a possibility that HA devs might need to add a maintainer to this integration or allow ownership transfer.

I also do not believe my patch has any of the required test coverage/etc. that is required for an integration of this quality (which I can no longer find, but I thought it was quite high on the quality scale).

rewardone avatar Apr 18 '25 00:04 rewardone