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

XMPP async #17283

Merged
merged 13 commits into from
Oct 13, 2018
Merged

XMPP async #17283

merged 13 commits into from
Oct 13, 2018

Conversation

flowolf
Copy link
Contributor

@flowolf flowolf commented Oct 9, 2018

Description:

Move notify.XMPP from sleekxmpp to slixmpp to allow async functionality.

  • adds resource config option: lets users define the resource part of the sender address. see example below.
  • adds support for TLSv1.1 and TLSv1.2

Related issue (if applicable): fixes #17005

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#6640

Example entry for configuration.yaml (if applicable):

# Example configuration.yaml entry
notify:
  - name: NOTIFIER_NAME
    platform: xmpp
    sender: YOUR_JID
    password: YOUR_JABBER_ACCOUNT_PASSWORD
    recipient: YOUR_RECIPIENT
    room: roomXY@server.jabber.example.com  # (option)
    resource: HA-lakehouse  # (option)

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

will try slixmpp instead of aioxmpp

reasons:
echo bot example of aioxmpp had blocking behavior (slixmpp echo bot works fine)
closer API to sleekxmpp
less dependencies than aioxmpp
the joinMUC method changed from sleekxmpp to slixmpp
added debug messages
better name for cleanup callback
Copy link
Member

@fabaff fabaff left a comment

Choose a reason for hiding this comment

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

Thanks for working on this.

homeassistant/components/notify/xmpp.py Outdated Show resolved Hide resolved
homeassistant/components/notify/xmpp.py Outdated Show resolved Hide resolved
homeassistant/components/notify/xmpp.py Outdated Show resolved Hide resolved
homeassistant/components/notify/xmpp.py Outdated Show resolved Hide resolved
homeassistant/components/notify/xmpp.py Outdated Show resolved Hide resolved
@thundergreen
Copy link

I would highly appreciate also having httupuload integrated for xmpp notify ... Currently I use a cli command with
https://github.com/sezuan/uploadr

But slixmpp also has build in httupuload functionality. I know not really very much people use xmpp but I think it's worth to get the best out of it .

Furthermore I was thinking about a xmpp bit to interact with Hass ...well only ideas but httupuload would be great

@fabaff
Copy link
Member

fabaff commented Oct 10, 2018

@thundergreen, I think that we should keep this PR limited to the migration to slixmpp.

@flowolf flowolf changed the title [WIP] XMPP async XMPP async Oct 11, 2018
@thundergreen
Copy link

thundergreen commented Oct 12, 2018

Why not just pimp this component to a next upper level? Would be nice to modernize this component ..like this it's an alternative to telegram and co, slixmpp offers this so why not using it?

In terms of privacy and what this project aims to provide I think it's also time to give users with high privacy level a way to use a complete notifications platforms Without the need to build their very own custom platforms and services

@MartinHjelmare
Copy link
Member

@thundergreen one PR per feature.

@flowolf
Copy link
Contributor Author

flowolf commented Oct 12, 2018

@thundergreen I'd like to focus on the task at hand in this PR.
I'm already looking into HTTP upload with slixmpp. However this will be another PR.

Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Member

@fabaff fabaff left a comment

Choose a reason for hiding this comment

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

Thanks 🐦

@fabaff fabaff merged commit d4b0927 into home-assistant:dev Oct 13, 2018
@ghost ghost removed the in progress label Oct 13, 2018
@balloob balloob mentioned this pull request Oct 26, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate XMPP notify platform to asyncio
7 participants