home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
70.97k stars 29.63k forks source link

Luci integration reports false away detection after 2023.5 upgrade #92580

Open dziadeck47 opened 1 year ago

dziadeck47 commented 1 year ago

The problem

The status of devices integrated via built-in OpenWRT (Luci) device tracker started fluctuating between away-home on release 2023.5 while it was perfectly stable/rational up till 2023.4.2 No other changes were made to either device nor configuration

image

What version of Home Assistant Core has the issue?

core-2023.5.0

What was the last working version of Home Assistant Core?

core-2023.4.2

What type of installation are you running?

Home Assistant OS

Integration causing the issue

OpenWrt (luci)

Link to integration documentation on our website

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

Diagnostics information

see screen above and

image image

Example YAML snippet

device_tracker:
  - platform: luci
    host: 192.168.1.1
    username: !secret luci_username
    password: !secret luci_password
    new_device_defaults:
      track_new_devices: false

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 1 year ago

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

Code owner commands Code owners of `luci` 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 luci` Removes the current integration label and assignees on the issue, add the integration domain after the command.

(message by CodeOwnersMention)


luci documentation luci source (message by IssueLinks)

protyposis commented 1 year ago

I can confirm, it's constantly fluctuating and started with the upgrade to 2023.5.0.

There's nothing conspicuous in the logs.

protyposis commented 1 year ago

This issue was introduced by a dependency update of openwrt-luci-rpc in https://github.com/home-assistant/core/pull/91358, containing fix https://github.com/fbradyirl/openwrt-luci-rpc/pull/53 for issue https://github.com/fbradyirl/openwrt-luci-rpc/issues/37.

I documented more details in https://github.com/fbradyirl/openwrt-luci-rpc/issues/37#issuecomment-1537424114.

protyposis commented 1 year ago

As a temporary workaround, I duplicated the luci integration to the custom_integrations folder and updated the manifest.json by adding "version": "1.0.0" (mandatory field for custom integrations) and downgrading the required openwrt-luci-rpc version back to 1.1.11. Presence detection now works again like in HA 2023.4.

dziadeck47 commented 1 year ago

How did you 'duplicate the luci integration'? I was not able to find the files on my instance (Hasos install) I did find the corresponding folder in the HA source code(on github) -> copied it (with 3 'text' files inside) to my local instance 'config/custom_components' folder -> amended 2 lines of the manifest as you describe ->restarted HA so that the custom component takes over the 'normal' integration (i left the config.yaml, knowndevices and everything else untouched) Didnt help - did I miss something (are there some other places in the github source I should amend or sth?)

protyposis commented 1 year ago

Yes, I also copied it from this repo. I also renamed the folder, the domain in manifest.json and the device_tracker/platform name in configuration.yaml from luci to luci_legacy. Wasn't sure if that is required, but now seems like it is :)

dziadeck47 commented 1 year ago

Thanks for the tip. I did reneme the folder and the domain in the manifest.json image Restarted HA so it could find the new 'integration' Then amended the config.yaml with the new platform: image But it does not recognise it - I get "Platform error device_tracker.luci_legacy - Integration 'luci_legacy' not found." on restart - did I omit something? It works for you I understand

protyposis commented 1 year ago

You need to restart twice. The first time for HA to recognize the custom component, the second time to apply the config change. That's the same for all custom integrations.

You'll know it works when the logs contain the Could not determine OpenWRT version, defaulting to version 18.06 error (#89133).

dziadeck47 commented 1 year ago

Ahh, I did not add the comma after 'requirements' and before adding the 'version' parameter in the manifest.json, hence HA did not recognise the integration and I could not use it in the config.yaml It kicked in after correcting it and double reboot (once to so HA recognises the intergation and I can use it in config.yaml, 2nd after amending config.yaml) Let me see it it improves the situation Thanks for the help!

dziadeck47 commented 1 year ago

Works like charm (as it used to) Thanks! Hope it gets repaired in the next official release

issue-triage-workflows[bot] commented 1 year ago

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.

protyposis commented 1 year ago

Issue still exists...

dziadeck47 commented 1 year ago

YEs, the integration was not changed whatsoever since the issue was reported, so I am still using the 'custom' luci integration with "requirements": ["openwrt-luci-rpc==1.1.11"]

codyc1515 commented 10 months ago

Appears it may be this commit specifically https://github.com/fbradyirl/openwrt-luci-rpc/pull/53/commits/9d40417a515291c0711e8dcd8c7c58dbbc46b486

fyhertz commented 10 months ago

Same problem here with OpenWrt 22.03 and home assistant 2023.10.3.

However using openwrt-luci-rpc==1.1.16 with only_reachable=False seem to be enough.

I've created a custom integration like @protyposis but instead of patching the openwrt-luci-rpc version I've patched _upate_info in device_tracker.py like so:

    from pathlib import Path
    from openwrt_luci_rpc import Constants

    def _update_info(self):
        """Check the Luci router for devices."""
        result = self.router.get_all_connected_devices(only_reachable=False)

        _LOGGER.debug("Luci get_all_connected_devices returned: %s", result)

        host_hints = self.router.router._call_json_rpc(
            Constants.LUCI_RPC_SYS_PATH.format(self.router.router.host_api_url),
            'net.host_hints'
        )
        host_hints = {k: v["name"] for k,v in host_hints.items() if "name" in v}

        last_results = []
        for device in result:
            hostname = None
            if device.mac in host_hints:
                hostname = str(Path(host_hints.get(device.mac)).stem)
            last_results.append(device._replace(hostname=hostname))

        self.last_results = last_results

This patch also uses host_hints RPC call to set hostnames instead of this rpc call used in openwrt-luci-rpc. You can ignore this modification if you are fine with the current behavior of the luci integration. I wasn't happy with it because it fails to set the hostname when device reports it through DHCP.

marsp88 commented 7 months ago

Thank you guys! I was able to solve the issue with devices fluctuating, but now one device (an Iphone) is always shown as home, even when it has not been on the wifi for hours. My openwrt is a build from december and HA is 2024.1.2. It's the same for both solutions in this threat.

jimb0b commented 6 months ago

Thank you @fyhertz , that fixed my problem. Given the openwrt python library suggests not using this on any OpenWRT devices newer than 18.06 ( https://github.com/fbradyirl/openwrt-luci-rpc/blob/master/openwrt_luci_rpc/openwrt_luci_rpc.py#L138 ) and that this version was released in 2018(!) I feel your suggestion should be pulled into the codebase, worth raising a PR for? (I've only just started looking through the HA code so not sure what the official process is to get that sorted)

Edit - just as I said that, my devices are fluctuating again :( (will double check to see if I've done something wrong)

edit2 - @fyhertz I originally just changed the "only_reachable" part as described however it turned out my devices were still not being updated. I had to also update the for loop, taking your entire example, and it looks like it works now - thanks.

issue-triage-workflows[bot] commented 3 months ago

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.

SWW13 commented 3 months ago

We are still waiting for a proper fix...

loyukfai commented 1 month ago

Ummm... After a recent re-install I moved from ubus integration (https://www.home-assistant.io/integrations/ubus/) to LuCI and encountered this bug.

Any temporary fix I can use? Or should I switch back to ubus?

(Besides, can we setup a new user for LuCI integration as well? Using root for everything feels a bit uncomfortable.)

Thanks in advance! This is very annoying.

jimb0b commented 1 month ago

@loyukfai please refer to @fyhertz post above - I had to reapply it after this months update for some reason. Essentially

  1. Copy the luci integration (you may need to exec in via docker depending on your install method) to custom_components/luci_legacy (or similar).
  2. Edit the domain field in luci_legacy/manifest.json to be "luci_legacy"
  3. Update configuration.yaml to use this new domain (luci -> luci_legacy)
  4. Update luci_legacy/device_tracker.py as per @fyhertz post above
loyukfai commented 1 month ago

Thanks! I need some help. Am running HASS, where should I create the custom_components folder? I tried creating it in merged and after 2 reboot HA still say it cannot find luci_legacy.

jimb0b commented 1 month ago

No worries, have you got ssh enabled? If you ssh into your instance, that folder should already be there (under config/custom_components). Or you could install the vs code server addon which should also show you the filesystem layout?

loyukfai commented 1 month ago

It seems ssh isn't enabled. Connection refused.

Just wondering, this bug is present on the ubus integration as well? Perhaps I can just switch to that... (Besides, it feels better cause ubus integration recommends creating a separate user insteda of using root as in LuCI.)

Cheers.

jimb0b commented 1 month ago

Hmm, not sure to be honest - I can't remember why I don't use the ubus integration (I can only assume I tried it and didn't have great success!). Re: ssh, theres tutorials on the internet / addons for HA which should enable it for you!

loyukfai commented 1 month ago

Thanks. To be honest, if I keep on learning and doing these stuffs, I may just try to upstream @fyhertz fix to master myself.

I had ubus integration working before, but I like the fact that the LuCI version will theoretically survive an OpenWrt upgrade (for the ubus version you need to manually make some changes for every upgrade although OpenWrt upgrades are far and few between anyway.)

But if the ubus integration has this same problem, there is no point in switching... except for the potential(?) security issue.

Cheers.

Edit: I have a feeling that the ubus command line version settings may work for LuCI as well, but I don't really have time to look into it at the moment.

loyukfai commented 1 month ago

OK, I switched back to ubus integration and everything seems to work well.

After some observations, it seems the ubus Wiki's way to configure a separate account will work for LuCI integration as well.

After all, it's my understanding that LuCI integration is just a wrapper for ubus, which means the ACL entry still needs to be manually added every time OpenWrt is upgraded (that's, if you create a separate account for presence detection integration).

Cheers.

koichirose commented 3 weeks ago

I'm also experiencing this issue and fixed it by changing the code to only_reachable=False. It's been working flawlessly since and it's also pretty fast at detecting when my devices (Android phones) are away.

What's the rationale behind only_reachable=True for presence detection?