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

Hangouts #16049

Merged
merged 36 commits into from Aug 24, 2018

Conversation

Projects
None yet
7 participants
@hobbypunk90
Contributor

hobbypunk90 commented Aug 18, 2018

Description:

Adds support for Google Hangouts as notify platform and automation trigger.

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

Example entry for configuration.yaml (if applicable):

hangouts:
  commands:
    - name: ping
      word: ping
  
notify:
  - name: erwin
    platform: hangouts
    default_conversations:
      - id: <conversation_id>
      - name: <conversation>

automation:
  - alias: 'Hangouts: Ping'
    trigger:
    - event_data: {'command': 'ping'}
      event_type: hangouts_command
      platform: event
    action:
    - data_template:
        target: 
          - id: '{{ trigger.event.data.conversation_id}}'
        message: 
          - text: 'pong'
      service: hangouts.send_message

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.

hobbypunk90 added some commits Jul 12, 2018

@homeassistant

This comment has been minimized.

homeassistant commented Aug 18, 2018

Hi @hobbypunk90,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

hobbypunk90 added some commits Aug 18, 2018

TARGETS_SCHEMA = vol.All(
vol.Schema({
vol.Exclusive(CONF_CONVERSATION_ID, 'id'): cv.string,
vol.Exclusive(CONF_CONVERSATION_NAME, 'id'): cv.string

This comment has been minimized.

@tjorim

tjorim Aug 19, 2018

Contributor

Is 'id' the correct 'group of exclusion'?

This comment has been minimized.

@hobbypunk90

hobbypunk90 Aug 19, 2018

Contributor

yes and no. the group is correct, but the naming is bad

@edif30

This comment has been minimized.

Contributor

edif30 commented Aug 19, 2018

I added this as a custom component to try out. I'm only testing the notify portion for alerts. Seems each message is being sent/received twice. I tested this with an automation and straight from the dev panel manually.

@hobbypunk90

This comment has been minimized.

Contributor

hobbypunk90 commented Aug 19, 2018

@edif30 can you please show me your relevant config (without password and email 😉) i tried it the last days and don't have a problem with double messages 🤔
i test it at the moment with a simple "ping" -> "pong" automation. no multiple messages sent.

@edif30

This comment has been minimized.

Contributor

edif30 commented Aug 19, 2018

Sure thing!

hangouts:
  email: !secret hangouts_email
  password: !secret hangouts_pw
      
notify:
  - name: ha_hangouts_main
    platform: hangouts
    default_conversations:
      - id: <id from entity>
      - name: HA Main

#
  - id: zwave_up
    alias: ZWAVE Up
    initial_state: 'on'
    trigger:
      platform: event
      event_type: zwave.network_ready
    action:
      - service: input_boolean.turn_on
        entity_id:
          - input_boolean.zwave_up
      - service: notify.ha_hangouts_main
        data:
          message:
            "ZWAVE UP!"

Like I mentioned in the previous post, I also tried sending a message manually via the dev panel and that shows 2 copies.

EDIT: I think I found the issue with the help from @dshokouhi. I had both "id" and "name". So it was sending the message twice.

@hobbypunk90

This comment has been minimized.

Contributor

hobbypunk90 commented Aug 19, 2018

ahhh, i think i see the problem 😁

default_conversations:
  - id: <id from entity>
  - name: HA Main

you entered the id of the conversation "HA Main" and the name, am i right?
That are 2 conversations, so you got the in every conversation, if it is the same you get it twice. 😉
you only need the the id OR the name of a conversation.
It is both possible, because direct conversations normally don't have a name. 😉

hobbypunk90 added some commits Aug 22, 2018

@@ -0,0 +1,12 @@
update_users_and_conversations:

This comment has been minimized.

@balloob

balloob Aug 23, 2018

Member

Should this just be called update ?

self.hass.states.async_set("{}.conversations".format(DOMAIN),
len(self._conversation_list.get_all()),
attributes=conversations)
# self.hass.bus.fire(EVENT_HANGOUTS_USERS_CHANGED, users)

This comment has been minimized.

@balloob

balloob Aug 23, 2018

Member

Remove?

_LOGGER.error("Hangouts failed to log in: %s", str(exception))
return False

hass.bus.async_listen(EVENT_HANGOUTS_CONNECTED,

This comment has been minimized.

@balloob

balloob Aug 23, 2018

Member

This needs to use the dispatcher too

This comment has been minimized.

@hobbypunk90

hobbypunk90 Aug 23, 2018

Contributor

are you sure? i fire the event with hass.bus.async_fire, so user can react if hangouts is connected (and also if its disconnected)

switched to dispatcher😁

SERVICE_UPDATE_USERS_AND_CONVERSATIONS,
bot.
async_handle_update_users_and_conversations,
schema=None)

This comment has been minimized.

@balloob

balloob Aug 23, 2018

Member

it has a schema, with no keys: vol.Schema({})


return await self.async_step_final()
except GoogleAuthError as err:
if err.__class__ is Google2FAError:

This comment has been minimized.

@balloob

balloob Aug 23, 2018

Member

if isinstance(err, Google2FAError):

def _on_connect(self):
_LOGGER.info('Connected!')
self._connected = True
self.hass.bus.async_fire(EVENT_HANGOUTS_CONNECTED)

This comment has been minimized.

@balloob

balloob Aug 23, 2018

Member

Should be dispatcher?

"""Handle disconnecting."""
_LOGGER.info('Connection lost!')
self._connected = False
self.hass.bus.async_fire(EVENT_HANGOUTS_DISCONNECTED)

This comment has been minimized.

@balloob

balloob Aug 23, 2018

Member

Should be dispatcher?

@balloob

Looking great! Could you add a quick test to make sure your config flow works? Here is an example

@hobbypunk90

This comment has been minimized.

Contributor

hobbypunk90 commented Aug 23, 2018

@balloob i will, but i'm not sure, how 🤔 i can test if it works if the login data are incorrect, but for a successful login we would need a real google login. And a 2FA for the 2FA question...

return await self.async_step_2fa()
msg = str(err)
if msg == 'Unknown verification code input':
errors['base'] = 'invalid_2fa_method'

This comment has been minimized.

@balloob

balloob Aug 23, 2018

Member

Can this happen ? I thought if we get a 2FAerror we already redirect to the 2fa step?

This comment has been minimized.

@hobbypunk90

hobbypunk90 Aug 23, 2018

Contributor

jep, this can happen...
if we get this error message, it is not our 2FAError, than the Google Account is secured the 2FA with asking on the phone to accept the new login, this is not supported in hangups.

GoogleAuthError, Google2FAError)
self._credentials = HangoutsCredentials(user_input[CONF_EMAIL],
user_input[CONF_PASSWORD])
self._refresh_token = HangoutsRefreshToken(None)

This comment has been minimized.

@balloob

balloob Aug 23, 2018

Member

These should only be stored on self if the call to get_auth is successful?

This comment has been minimized.

@hobbypunk90

hobbypunk90 Aug 23, 2018

Contributor

Both Classes are only Data Classes which are needed for get_auth to read/write the data.
on a successful login get_auth writes the token in HangoutsRefreshToken
and the credentials are stored to catch up the login data in the 2fa step.

@balloob

This comment has been minimized.

Member

balloob commented Aug 23, 2018

Nah, just mock any call to hangups. Something like this: (did not test this code)

flow = HangoutsFlowHandler()

with patch('hangups.get_auth', side_effect=Google2FAError):
    result = await flow.async_step_user({'email': 'bla', 'password': 'bla'})
    assert result['type'] == data_entry_flow.RESULT_TYPE_SHOW_FORM
    assert result['step_id'] == '2fa'
@balloob

This comment has been minimized.

Member

balloob commented Aug 24, 2018

Amazing work! 🎉

@balloob balloob merged commit ef0eab0 into home-assistant:dev Aug 24, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.03%) to 93.806%
Details

@wafflebot wafflebot bot removed the in progress label Aug 24, 2018

@hobbypunk90 hobbypunk90 deleted the hobbypunk90:hangouts branch Aug 24, 2018


async def async_setup(hass, config):
"""Set up the Hangouts bot component."""
config = config.get(DOMAIN, [])

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Aug 24, 2018

Member

Why default to empty list? Looks like the following line will fail in that case.

conversations[str(conv.id_)] = \
{'name': conv.name, 'users': users_in_conversation}

self.hass.states.async_set("{}.users".format(DOMAIN),

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Aug 24, 2018

Member

Shouldn't this be implemented as a sensor platform?

self.hass.states.async_set("{}.users".format(DOMAIN),
len(self._user_list.get_all()),
attributes=users)
self.hass.states.async_set("{}.conversations".format(DOMAIN),

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Aug 24, 2018

Member

See above.

@balloob balloob referenced this pull request Aug 29, 2018

Merged

0.77 #16256

cyberjacob pushed a commit to cyberjacob/home-assistant that referenced this pull request Sep 4, 2018

Hangouts (home-assistant#16049)
* add a component for hangouts

* add a notify component for hangouts

* add an extra message as title

* add support to listen to all conversations hangouts has

* move hangouts to package and add parameter documentation

* update .coveragerc and requirements_all.txt

* makes linter happy again

* bugfix

* add conversations parameter to command words

* Move the resolution of conversation names to conversations in own a function

* typo

* rename group of exclusion form 'id' to 'id or name'

* refactoring and use config_flow

* makes linter happy again

* remove unused imports

* fix not working regex commands

* fix translations

* cleanup

* remove step_init

* remove logging entry

* clean up events

* move constant

* remove unsed import

* add new files to .converagerc

* isort imports

* add hangouts_utils to ignored packages

* upadte doc and format

* fix I/O not in executor jon

* rename SERVICE_UPDATE_USERS_AND_CONVERSATIONS to SERVICE_UPDATE

* move EVENT_HANGOUTS_{CONNECTED,DISCONNECTED} to dispatcher

* add config flow tests

* Update tox.ini

@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.