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

GeniusHub - Recent PR causes local access to fail #127023

Closed
RogerSelwyn opened this issue Sep 29, 2024 · 23 comments · Fixed by #129569
Closed

GeniusHub - Recent PR causes local access to fail #127023

RogerSelwyn opened this issue Sep 29, 2024 · 23 comments · Fixed by #129569

Comments

@RogerSelwyn
Copy link
Contributor

RogerSelwyn commented Sep 29, 2024

The problem

PR - #122330 - causes an error when starting up HA. My configuration uses local access which stores ip/username/password in the config entry. The change includes setting the unique_id to the Mac address, which is not stored in the config entry.

What version of Home Assistant Core has the issue?

core-2024.11.0b0

What was the last working version of Home Assistant Core?

core-2024.10.0

What type of installation are you running?

Home Assistant Container

Integration causing the issue

GeniusHub

Link to integration documentation on our website

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

Diagnostics information

Not available since integration does not load.

Config entry is as below

      {
        "created_at": "2024-09-29T08:22:53.002715+00:00",
        "data": {
          "host": "192.168.20.204",
          "username": "redacted",
          "password": "redacted"
        },
        "discovery_keys": {},
        "disabled_by": null,
        "domain": "geniushub",
        "entry_id": "01J8YE06TATA2MVAVNH2M3C430",
        "minor_version": 1,
        "modified_at": "2024-09-29T08:22:53.002719+00:00",
        "options": {},
        "pref_disable_new_entities": false,
        "pref_disable_polling": false,
        "source": "user",
        "title": "192.168.20.204",
        "unique_id": "B8:27:EB:90:F0:FB",
        "version": 1
      }

Example YAML snippet

No response

Anything in the logs that might be useful for us?

2024-09-29 08:27:03.503 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry 192.168.20.204 for geniushub
Traceback (most recent call last):
  File "/workspaces/core/homeassistant/config_entries.py", line 594, in async_setup
    result = await component.async_setup_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/core/homeassistant/components/geniushub/__init__.py", line 181, in async_setup_entry
    unique_id = entry.data[CONF_MAC]
                ~~~~~~~~~~^^^^^^^^^^
KeyError: 'mac'

Additional information

No response

@home-assistant
Copy link

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

Code owner commands

Code owners of geniushub 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 geniushub 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)


geniushub documentation
geniushub source
(message by IssueLinks)

@RogerSelwyn
Copy link
Contributor Author

When I look at a commit in that PR is states that the tests don't cover the change

https://github.com/home-assistant/core/pull/122330/files/d16851972fcb28a12c17e90877ff564f3d8180f3..14f7e00494d420ee0f571a9738bacd8fc3a6c003

@zxdavb zxdavb changed the title GeniusHub - Recent PR causes local access too fail GeniusHub - Recent PR causes local access to fail Oct 11, 2024
@RogerSelwyn
Copy link
Contributor Author

@joostlek - I see this issue is now in the beta released today. I'm not sure of what the intent of the PR was so can't recommend a fix.

@GeoffAtHome - Not sure if you are looking at these at all since @manzanotti has not looked at it

@joostlek
Copy link
Member

The intent was that previously users had to manually enter a mac address, which we don't want since it was in fact a user settable unique id. I think I made a small mistake in this PR and will create a fix for it. If you would be able to test it, that would be awesome

@joostlek
Copy link
Member

cd /config
curl -o- -L https://gist.githubusercontent.com/bdraco/43f8043cb04b9838383fd71353e99b18/raw/core_integration_pr | bash /dev/stdin -d geniushub -p 129569

@joostlek joostlek mentioned this issue Oct 31, 2024
19 tasks
@joostlek joostlek added this to the 2024.11.0 milestone Oct 31, 2024
@RogerSelwyn
Copy link
Contributor Author

I’ll be able to test later this evening - uk

@RogerSelwyn
Copy link
Contributor Author

The intent was that previously users had to manually enter a mac address, which we don't want since it was in fact a user settable unique id. I think I made a small mistake in this PR and will create a fix for it. If you would be able to test it, that would be awesome

It seems to start fine in my dev environment with that change. Should be good to go.

@joostlek
Copy link
Member

Awesome, while you're at it, do you mind testing a few more things?

@RogerSelwyn
Copy link
Contributor Author

Can do. Not sure that I have a completely standard setup here (e.g. cd /config wouldn't get you where you think), but I can give them a go. Easier if they are merged to dev...:-)

@joostlek
Copy link
Member

The bash code I sent before allows you to install PRs as custom component, so you can always delete it afterwards. I still have #126986 and #126983. The first one is meant to improve the naming scheme of the integration and it adds devices, be sure to generate translatons and it will also allow translated names. The second PR adds a modern data fetching mechanism, so ideally you would not notice any change for this :)

@RogerSelwyn
Copy link
Contributor Author

RogerSelwyn commented Oct 31, 2024

The bash code I sent before allows you to install PRs as custom component, so you can always delete it afterwards. I still have #126986 and #126983. The first one is meant to improve the naming scheme of the integration and it adds devices, be sure to generate translatons and it will also allow translated names. The second PR adds a modern data fetching mechanism, so ideally you would not notice any change for this :)

OK, I understand. Can you do the same for these two? Make it easier if I can just pull them. I've never generated those links for myself before, so would do it myself if I knew how. A learning day

@joostlek
Copy link
Member

The link itself is quite explanatory, the magic is in the end -p xxx is the PR number and -d xx is the domain of the integration, in this case geniushub

@RogerSelwyn
Copy link
Contributor Author

Or is it the same but with a different pr on the end. Not sure about the long string after bdraco.

@joostlek
Copy link
Member

https://gist.githubusercontent.com/bdraco/43f8043cb04b9838383fd71353e99b18/raw/core_integration_pr is the URL of the script :)

What this does is that it downloads a script, which contains the instructions to download the specific PR. (Always be sure to check the scripts you run, just in case)

@RogerSelwyn
Copy link
Contributor Author

#126986 fails with the error I reported, so applied your fix. It boots fine. I note that on the devices, all manufacturer's are Unknown and my Zones and Genius Hub have unknown models.

@RogerSelwyn
Copy link
Contributor Author

RogerSelwyn commented Oct 31, 2024

I don't see any errors from #126983 (apart from the same fix), but I haven't then gone and checked all the data is correct. I see some testing has been added recently, so that should pick it up. I don't think adding a coordinator should impact any update functions.

@joostlek
Copy link
Member

Do you maybe have discord?

@RogerSelwyn
Copy link
Contributor Author

Do you maybe have discord?

Nope. WhatsApp, Messenger, Zoom, etc. Not a Discord guy. I could maybe install it, can't be hard.

@RogerSelwyn
Copy link
Contributor Author

Or maybe not. You can mail me on firstname @ lastname org uk, we can connect up to whatsapp

@RogerSelwyn
Copy link
Contributor Author

I'm a liar, I do have discord, just don't use it

@joostlek
Copy link
Member

Hah, rather not via WhatsApp as that exposes more than I'd like.

We can otherwise just continue on GitHub :)

@RogerSelwyn
Copy link
Contributor Author

I'm RogTP on Discord. I'm sure you know better than me how to contact me, so I'll let you do that. I'm sure something will go bing when I get a message

@joostlek
Copy link
Member

Sent a message

@github-actions github-actions bot locked and limited conversation to collaborators Dec 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants