Fix Telegram Bot send file to multiple targets, snapshots of HA cameras, variable templating, digest auth by azogue · Pull Request #7771 · home-assistant/core

azogue · 2017-05-25T18:55:53Z

Description:

Fix some problems introduced with the changes in telegram_bot and notify.telegram of 0.45 and 0.45.1, and some enhancements:

Allow templating in url, file, caption, longitude, latitude fields.
Fix sending HA camera snapshots by retrying the HA API request (it returns some 500's before working for some cameras in Home Assistant, not all)
Fix send file to multiple locations
Easier inline keyboards from yaml files.
HttpDigest authentication for url’s

Related issues:

fix answering to callback queries (KeyError)
fixes Location information not being sent to Telegram #7703 Location information not being sent to Telegram
fixes Cannot send specific jpg via telegram-bot #7413 Cannot send specific jpg via telegram-bot
fixes Telegram does not send photo if URL requires HttpDigestAuth #7395 Telegram does not send photo if URL requires HttpDigestAuth

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

Notification Examples for `configuration.yaml`:

telegram_bot:
  platform: polling
  api_key: !secret telegram_api
  allowed_chat_ids:
    - !secret telegram_bot_chat_id_admin
    - !secret telegram_bot_group

notify:
  - platform: telegram
    name: telegram_bot
    chat_id: !secret telegram_bot_chat_id_admin

script:
  # Telegram Bot component
  test_telegram_bot_location:
    alias: Test Telegram Bot multi-target location template
    sequence:
      - service: telegram_bot.send_location
        data:
          caption: 'Test location'
          target:
            - !secret telegram_bot_chat_id_admin
            - !secret telegram_bot_group
          latitude: '{{ states.device_tracker.iphone.attributes.latitude|float }}'
          longitude: '{{ states.device_tracker.iphone.attributes.longitude|float }}'
          disable_notification: true

  test_telegram_bot_document:
    alias: Test Telegram Bot multi-target document
    sequence:
      - service: telegram_bot.send_document
        data:
          caption: 'Test document'
          target:
            - !secret telegram_bot_chat_id_admin
            - !secret telegram_bot_group
          file: '/home/homeassistant/.homeassistant/home-assistant.log'
          disable_notification: true

  test_telegram_bot_image:
    alias: Test Telegram Bot multi-target image
    sequence:
      - service: telegram_bot.send_photo
        data:
          target:
            - !secret telegram_bot_chat_id_admin
            - !secret telegram_bot_group
          caption: 'Test image'
          url: 'http://localhost:8123{{ states.camera.ha_camera.attributes.entity_picture}}'
          disable_notification: true

  # Notify component
  test_notify_telegram_location:
    alias: Test notify.telegram location template
    sequence:
      - service: notify.telegram_bot
        data:
          message: "Not used here but needed..."
          data:
            location:
              latitude: '{{ states.device_tracker.iphone.attributes.latitude|float }}'
              longitude: '{{ states.device_tracker.iphone.attributes.longitude|float }}'

  test_notify_telegram_image:
    alias: Test notify.telegram multiple image
    sequence:
      - service: notify.telegram_bot
        data:
          message: "Not used here but needed..."
          data:
            photo:
              - url: 'https://upload.wikimedia.org/wikipedia/en/5/58/Penny_test.jpg'
                caption: 'Test img 1. C1'
              - url: 'http://localhost:8123{{ states.camera.ha_camera.attributes.entity_picture}}'
                caption: 'Test img 1. C2'

Checklist:

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

Documentation added/updated in home-assistant.github.io
Tests have been done manually to verify that the new code works. (There aren't tests for the Telegram components)

To make a row of InlineKeyboardButtons you pass: - a list of tuples like: `[(text_b1, data_callback_b1), (text_b2, data_callback_b2), ...] - a string like: `/cmd1, /cmd2, /cmd3` - or a string like: `text_b1:/cmd1, text_b2:/cmd2` Example: ```yaml data: message: 'TV is off' disable_notification: true inline_keyboard: - TV ON:/service_call switch.turn_on switch.tv, Other:/othercmd - /help, /init ```

- Allow templating for caption, url, file, longitude and latitude fields - Fix send a file to multiple targets - Load data with some retrying for HA cameras, which return 500 one or two times sometimes (generic cams, always!). - Doc in services for new inline keyboards yaml syntax: `Text button:/command`

mention-bot · 2017-05-25T18:55:55Z

@azogue, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fabaff, @sander76 and @MartinHjelmare to be potential reviewers.

MartinHjelmare · 2017-05-25T22:57:45Z

            kwargs = dict(service.data)
            _render_template_attr(kwargs, ATTR_MESSAGE)
            _render_template_attr(kwargs, ATTR_TITLE)
+            _render_template_attr(kwargs, ATTR_URL)


Use a for loop here.

MartinHjelmare · 2017-05-25T22:59:18Z

-                _LOGGER.warning("BAD TARGET %s, using default: %s",
+            try:
+                chat_ids = [int(t) for t in target
+                            if int(t) in self.allowed_chat_ids]


Chat ids in target have already been validated and coerced to integers.

It's true, this is old, I'm going to simplify it

MartinHjelmare · 2017-05-25T23:00:15Z

+            try:
+                chat_ids = [int(t) for t in target
+                            if int(t) in self.allowed_chat_ids]
+                if len(chat_ids) > 0:


if chat_ids:

MartinHjelmare · 2017-05-25T23:01:38Z

+                if len(chat_ids) > 0:
+                    return chat_ids
+                _LOGGER.warning("Unallowed targets: %s", target)
+            except (ValueError, TypeError):


When could this happen? Both target and allowed chat ids have been validated as list of integers before this.

MartinHjelmare · 2017-05-25T23:04:17Z

+                    return chat_ids
+                _LOGGER.warning("Unallowed targets: %s", target)
+            except (ValueError, TypeError):
+                _LOGGER.warning("Bad target data: %s, using default: %s",


Shouldn't the warning about default be included in the above warning instead? The warning here should never happen.

MartinHjelmare · 2017-05-25T23:08:44Z

+        params = self._get_msg_kwargs(kwargs)
+        caption = kwargs.get(ATTR_CAPTION)
+        func_send = self.bot.sendPhoto if is_photo else self.bot.sendDocument
        file = load_data(


Pick another variable name than file. It's reserved in python.

This one is still here. 😉

fixed! 😓

MartinHjelmare · 2017-05-25T23:15:22Z



-def load_data(url=None, file=None, username=None, password=None):
+def load_data(url=None, file=None, username=None, password=None,


Pick another parameter name than file. It's reserved in python.

MartinHjelmare · 2017-05-25T23:17:20Z

+                        params["auth"] = HTTPDigestAuth(username, password)
+                    else:
+                        params["auth"] = HTTPBasicAuth(username, password)
+                req = requests.get(url, **params)


Now there's no request without authentication. Is that intended?

authentication params are included only when are present:

params = {"timeout": 15} if username is not None and password is not None: if authentication == HTTP_DIGEST_AUTHENTICATION: params["auth"] = HTTPDigestAuth(username, password) else: params["auth"] = HTTPBasicAuth(username, password)

What do you mean?

You're correct. I missed that point, sorry.

- Don't use `file` as variable name. - For loop - Simplify filter allowed `chat_id`s.

MartinHjelmare

Nice!

azogue · 2017-05-26T10:48:42Z

@MartinHjelmare, sorry, I found one more when testing the examples in the doc.

UltraSub · 2017-05-26T11:14:36Z

Nice! Is there any chance this will go into a 45.2 release?

MartinHjelmare · 2017-05-26T14:38:25Z

@azogue Ready to merge?

azogue · 2017-05-26T14:54:36Z

Yes!

azogue added 6 commits May 24, 2017 22:16

fix double template rendering when messages come from notify.telegram

834fb02

fix 'chat' information not present in callback queries

6bd7369

fix send file to multiple targets

235f9cb

HttpDigest authentication as proposed in home-assistant#7396

c5fa282

homeassistant added platform: telegram_bot.__init__ cla-signed labels May 25, 2017

MartinHjelmare reviewed May 25, 2017

View reviewed changes

review changes

e86dab3

- Don't use `file` as variable name. - For loop - Simplify filter allowed `chat_id`s.

azogue changed the title ~~Fix callbacks~~ Fix Telegram Bot send file to multiple targets, snapshots of HA cameras, variable templating, digest auth May 26, 2017

This was referenced May 26, 2017

Cannot send specific jpg via telegram-bot #7413

Closed

Update docstrings and log messages #7709

Merged

Don't use file as variable name!

12582a4

homeassistant added the cla-signed label May 26, 2017

make params outside the while loop

e0ced86

MartinHjelmare approved these changes May 26, 2017

View reviewed changes

fix chat_id validation when editing sent messages

2366387

azogue mentioned this pull request May 26, 2017

telegram doc update: digest authentication and inline keyboards home-assistant/home-assistant.io#2714

Merged

MartinHjelmare added this to the 0.45.2 milestone May 26, 2017

MartinHjelmare merged commit 910020b into home-assistant:dev May 26, 2017

azogue mentioned this pull request May 29, 2017

Cannot send any image with telegram after Upgrade to 0.45.0 #7705

Closed

balloob mentioned this pull request Jun 2, 2017

0.46 #7866

Merged

azogue deleted the fix-callbacks branch June 4, 2017 12:02

home-assistant locked and limited conversation to collaborators Sep 4, 2017



		def load_data(url=None, file=None, username=None, password=None):
		def load_data(url=None, file=None, username=None, password=None,

Uh oh!

Conversation

azogue commented May 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Notification Examples for configuration.yaml:

Checklist:

Uh oh!

mention-bot commented May 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

azogue commented May 26, 2017

Uh oh!

UltraSub commented May 26, 2017

Uh oh!

MartinHjelmare commented May 26, 2017

Uh oh!

azogue commented May 26, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

azogue commented May 25, 2017 •

edited

Loading

Notification Examples for `configuration.yaml`: