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

Improve Home Assistant Support for Notification Listener #5

Merged

Conversation

paquiro
Copy link
Contributor

@paquiro paquiro commented Oct 4, 2024

This pull request enhances the startNotificationListener function by adding support for Home Assistant. Key changes include:

  • Made hass an optional parameter for seamless integration with HA.
  • Adjusted async handling with asyncio.run_coroutine_threadsafe() and asyncio.ensure_future() to run coroutines both in and out of Home Assistant.
  • Fixed an issue with handling persistent IDs when notifications are received.
  • Refactored token registration and notification acknowledgment to ensure compatibility with HA's event loop.

These changes improve the flexibility and stability of the notification listener across different environments.

Can you merge it to a new branch? So we can test it better. I don't have permission for that.

@AfonsoFGarcia

@AfonsoFGarcia AfonsoFGarcia changed the base branch from main to event-loop-main October 5, 2024 07:29
Comment on lines +180 to +181
# TODO I commented this because it fails for me in Home Assistant
#run_in_event_loop(blueConAPIClient.__notificationInfoStorage.storePersistentId(idstr))
Copy link
Owner

Choose a reason for hiding this comment

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

This needs to be fixed before merging to main, as without persisting the IDs that were received, they will not be sent with the login request and FCM will send them again to be processed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I agree. At least we have a good entry point to be testing where we receive notifications to Home Assistant

@AfonsoFGarcia AfonsoFGarcia merged commit 1de4dcb into AfonsoFGarcia:event-loop-main Oct 5, 2024
@AfonsoFGarcia
Copy link
Owner

Merged to event-loop-main.

You have pre-releases of both the library (0.5.0a1 on pypy) and the integration (0.7.0-alpha.1) to test.

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

Successfully merging this pull request may close these issues.

2 participants