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

Add missing hangouts data/image to notify service #17576

Merged
merged 2 commits into from Oct 22, 2018

Conversation

Projects
None yet
5 participants
@quazzie
Contributor

quazzie commented Oct 18, 2018

Description:

Missed adding support for hangouts data/image to notify service

Related issue (if applicable): fixes #17513

Example entry for configuration.yaml (if applicable):

notify:
  - name: hangouts_me
    platform: hangouts
    default_conversations:
      - id: "xxxx"

Checklist:

  • The code change is tested and works locally.
add missing hangouts image_file/url to notify services
Missed adding support for hangouts image to notify service

@fabaff fabaff changed the title from add missing hangouts data/image to notify service to Add missing hangouts data/image to notify service Oct 21, 2018

@MartinHjelmare

Looks good!

@MartinHjelmare MartinHjelmare merged commit 42a4447 into home-assistant:dev Oct 22, 2018

3 checks passed

Hound No violations found. Woof!
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wafflebot wafflebot bot removed the in progress label Oct 22, 2018

@quazzie quazzie deleted the quazzie:patch-5 branch Oct 23, 2018

@edif30

This comment has been minimized.

Contributor

edif30 commented Oct 28, 2018

@quazzie

Since this change, it breaks using hangouts via notify.

2018-10-28 10:03:29 ERROR (MainThread) [homeassistant.core] Invalid service data for hangouts.send_message: expected a dictionary for dictionary value @ data['data']. Got None

And here is the automation I used to test which fails.

#
  - id: ha_startup
    alias: HA Startup
    initial_state: 'on'
    trigger:
      platform: homeassistant
      event: start
    action:
      - service: notify.ha_hangouts_main
        data:
          title: "HA STARTED!!"
          message:
            "HA Started!"

I don't need an image url but for some reason, this is now failing if I don't have a data field there with something. I don't see this in any changes on any release or considered a breaking change. Docs do not look like this is now considered "required". But shouldn't be required because not every message will have a data image_url.

@quazzie

This comment has been minimized.

Contributor

quazzie commented Oct 29, 2018

Uhm, it works for me.

notify:
  - name: myhangouts
    platform: hangouts
    default_conversations:
      - id: "xxxxx"
- initial_state: 'on'
  trigger:
    platform: homeassistant
    event: start
  action:
    - service: notify.myhangouts
      data:
        title: "HomeAssistant"
        message: "HomeAssistant started!"
@edif30

This comment has been minimized.

Contributor

edif30 commented Oct 29, 2018

I'll try again and see what happens. What version are you running?

@quazzie

This comment has been minimized.

Contributor

quazzie commented Oct 29, 2018

Home Assistant 0.81.0 on hassio

@edif30

This comment has been minimized.

Contributor

edif30 commented Oct 29, 2018

ok, issue I had was on .1. But I'll re-test later on after work.

@MartinHjelmare

This comment has been minimized.

Member

MartinHjelmare commented Oct 29, 2018

Please open an issue if you suspect a bug.

Merged PRs should not be used for support or bug reports. Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Oct 29, 2018

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