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

Remove use of deprecated last_event for button events #388

Conversation

idekker
Copy link
Contributor

@idekker idekker commented Aug 9, 2024

Use button_report.event instead.

@idekker
Copy link
Contributor Author

idekker commented Aug 9, 2024

Fixes home-assistant/core#123282

@idekker
Copy link
Contributor Author

idekker commented Aug 12, 2024

Could borrow a FoH switch and successfully tested the fix

@Asom-Velz
Copy link

@idekker Sorry for the question but in the section Assignees says No one assigned.
I don't have knowledge of how the pull requests are working but should not be mentioned the code owners there (balloob & marcelveldt) ?

@idekker
Copy link
Contributor Author

idekker commented Aug 14, 2024

@Asom-Velz Not sure what is wrong. Checks complains about no labels being assigned, but I cannot do that. I guess one of the owners will pick it up when they're available.
Depending on how you've setup HA, you can temporarily use my aiohue branch. I'm running the container and simply add a folder share to a locally checked out version of aiohue: <local path to aiohue dir in aiohue clone>:/usr/local/lib/python3.12/site-packages/aiohue

@idekker
Copy link
Contributor Author

idekker commented Aug 16, 2024

@marcelveldt Please help to review this PR. Many are waiting for events from Friends of Hue switches to be fixed.

@marcelveldt
Copy link
Collaborator

@marcelveldt Please help to review this PR. Many are waiting for events from Friends of Hue switches to be fixed.

Not so cool to ping like this. I'm still on holiday. I had planned to pick this up on Monday.

@idekker
Copy link
Contributor Author

idekker commented Aug 16, 2024

I saw you're actively pushing to music-assistant, so I thought you're back already and you mentioned before a PR would be reviewed if someone could make a fix...

@Asom-Velz

This comment was marked as off-topic.

@marcelveldt
Copy link
Collaborator

I saw you're actively pushing to music-assistant, so I thought you're back already and you mentioned before a PR would be reviewed if someone could make a fix...

hehe, well, in holiday mode it means I'm doing LESS open source work, not NO open source work :-)
But I'm back, picking up my backlog!

Copy link
Collaborator

@marcelveldt marcelveldt left a comment

Choose a reason for hiding this comment

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

This is fine like this. It will only cause issues for folks that didn't update their Hue bridge for a very long time but imo that is an edge case safe to ignore in this case.

@marcelveldt
Copy link
Collaborator

Thanks for the fix @idekker !
Any more fixes you want me to look at now or should we just bump the lib now ?

@marcelveldt marcelveldt merged commit 9fcaf4a into home-assistant-libs:main Aug 21, 2024
4 of 5 checks passed
@idekker
Copy link
Contributor Author

idekker commented Aug 21, 2024

Not for aiohue. I found another use of the deprecated last_event for relative rotary in the HA integration, but for as long it is not remove it doesn't break anything for anyone.

Ah, yeah, backwards compatibility. Maybe an idea to add button_report to the event when (just) last_event is there. Then at least upstream code doesn't have to deal with deprecated stuff anymore.

@idekker idekker deleted the remove_use_of_deprecated_last_event branch August 21, 2024 16:12
@marcelveldt
Copy link
Collaborator

Thanks for your help. Feel free to ping me on discord next time you need my help and I'm not responding. Its sometimes hard to get track of the dozens of repositories I'm involved with.

@Asom-Velz
Copy link

Hi @idekker @marcelveldt

Thank you both for your efforts to fix this issue.

I have a question what is the time frame that this fix will pop-up as an update to Home Assistant cause some integrations start to require the latest HA version 2024.8 in order to upgrade but I am stuck with the HA version 2024.7.3 which is still working with the FoH switches !!

Regards

@idekker
Copy link
Contributor Author

idekker commented Aug 22, 2024

I see a tag is created already, but not yet integrated in HA core. I'll see if I can create a PR for it tonight.

@Asom-Velz
Copy link

@idekker
Ok Thank you for the info 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants