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

Update to DoorBirdPy v2 (again) #14933

Merged
merged 19 commits into from Nov 1, 2018

Conversation

@Klikini
Contributor

Klikini commented Jun 11, 2018

Description:

Update to DoorBirdBy 2.0.2, which uses the new version (0.21) of the DoorBird LAN API.

  • No more clearing all other data on the device every startup
  • Motion event triggers available
  • External relay support
  • Manual registration clearing method

Pull request in home-assistant.github.io with documentation: home-assistant/home-assistant.io#5529

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.
@Klikini

This comment has been minimized.

Contributor

Klikini commented Jun 11, 2018

@oblogic7 I'm moving soon and I don't have a full HA instance set up right now for automation. Could you please test doorbell/motion events to with the new API version changes here?

@Klikini Klikini force-pushed the Klikini:doorbird-v2 branch from 656a6f1 to 05e58ba Jun 11, 2018

@Klikini Klikini referenced this pull request Jun 11, 2018

Merged

Doorbirdpy v2 #5529

2 of 2 tasks complete
@oblogic7

This comment has been minimized.

Contributor

oblogic7 commented Jun 12, 2018

Sure, no problem. I will try to do it tomorrow. Kids are down for the night and testing the doorbell would not be wise...

@syssi

This comment has been minimized.

Member

syssi commented Jun 12, 2018

@Klikini Could you fix/update the .coveragerc? The doorbird.py is listed already but it's indented by a tab instead of 4 spaces. I assume this will fix the coveralls error.

@oblogic7

This comment has been minimized.

Contributor

oblogic7 commented Jun 12, 2018

@Klikini It seems that the registered events override the Doorbird app notifications. Is this expected on the new API?

Edit: Also, I do not see the events in the log when monitored conditions are enabled. HA shows the events in the log by default, right?

@Klikini Klikini force-pushed the Klikini:doorbird-v2 branch from 05e58ba to f1f3777 Jun 12, 2018

@Klikini

This comment has been minimized.

Contributor

Klikini commented Jun 12, 2018

@oblogic7 That is not expected and not documented! I checked my device settings after the firmware upgraded and the only entries were for Home Assistant, none for the app. I did test the device setup and verified that HA was setting up the device schedules, and the app still notified me this morning, so I can't reproduce this issue.

I added a section to the component documentation that describes the HA "removal" process:

Home Assistant will register the monitored conditions with the device as schedule entries that correspond to favorites on startup. If you remove monitored conditions from your configuration, Home Assistant will attempt to remove these items from the device. However, in some cases, such as if the IP address of the machine running Home Assistant changes or if the device is renamed in your configuration, this will not work correctly and some data will be left in device storage.

This should not cause any problems, but if you would like to remove it, open a new browser window and navigate to {Home Assistant URL}/api/doorbird/clear/{DoorBird name}. Replace {Home Assistant URL} with the full path to your running instance, such as http://localhost:8123. Replace {DoorBird name} with the name specified in your configuration for the device you would like to clear, or how it appears in the Home Assistant UI if you have not specified one, such as DoorBird 1.

Please note that clearing device registrations will prevent the device from sending pushes to Home Assistant until you restart your instance with the component enabled. It could also affect other third-party applications you may use with your DoorBird device. It will not break the official mobile app in any way, so mobile push notifications will still work. Or so I thought?!

Do app notifications work again after following those instructions? And after starting this version of HA? After that if you want to go back to the stable HA version, follow the instructions again.

@oblogic7

This comment has been minimized.

Contributor

oblogic7 commented Jun 13, 2018

Tested this a couple of times now and can reliably reproduce the issue. I also found something else relating to registration of events. I verified the behavior using the schedule view in the official Doorbird app on iOS.

Here is what my push notifications schedule looks like before HA events are registered. Confirmed that the blue square means active since the app doesn't clarify if it is whitelist or blacklist behavior.

image

After I enable events for this device, the push notifications schedule appears to be cleared. Confirmed that notifications are no longer received to my device. Additionally, my chime no longer sounds in this state either.

image

The other interesting thing is that it appears that doorbell schedule is only applied to the 30 minute slot when HA event registration happens. I would expect all time slots to be enabled.

image

When both event types are registered, the schedule view for motion is not editable and has the same single time slot filled as doorbell events but using a gray box.

image

I'm going to dive into your changes and the API updates to see if I can sort out what is happening. I will let you know if I find anything.

@Klikini

This comment has been minimized.

Contributor

Klikini commented Jun 13, 2018

A step-by-step test

  1. I started with no schedule entries or favorites showing in API calls.

  2. I opened the Android app and confirmed that the push notification schedule was full and the relay schedule was empty, then saved my "changes":
    app1

  3. The schedule data then showed:

[
  {
    "input": "doorbell",
    "param": "1",
    "output": [
      {
        "event": "notify",
        "param": "",
        "schedule": {
          "weekdays": [
            {
              "from": "104400",
              "to": "104399"
            }
          ]
        }
      }
    ]
  }
]

The favorites list is still empty. I'm not sure why it's not showing 24/7 on weekdays, but only at certain times, even though the app has everything selected.

I can't even test my HA code anymore because I'm suddenly getting package errors when I run hass in its virtual environment and I don't have the energy to try to figure that out for the 50th time right now.

Even if the app has to register actions that aren't even in the API documentation ("notify"), the new API method should still work because it no longer erases the schedule data, it just adds HA's URLs and schedules to the existing data, and then will only delete its own URLs. The only time it would delete other things is when you access /clear.

Could you send your schedule.cgi and favorites.cgi output when push notifications work and then after starting hass when they don't so we can compare? Maybe it's an Android/iOS thing?

Also, here's my info.cgi output to compare versions:

{
  "BHA": {
    "RETURNCODE": "1",
    "VERSION": [
      {
        "FIRMWARE": "000111",
        "BUILD_NUMBER": "15222406",
        "WIFI_MAC_ADDR": "",
        "RELAYS": [
          "1"
        ],
        "DEVICE-TYPE": "DoorBird D101S"
      }
    ]
  }
}
@oblogic7

This comment has been minimized.

Contributor

oblogic7 commented Jun 13, 2018

Do you have some code you are using to get the output? No problem if you don't, but would save time from having to build something up to get the output you ask for.

@Klikini

This comment has been minimized.

Contributor

Klikini commented Jun 13, 2018

@oblogic7

This comment has been minimized.

Contributor

oblogic7 commented Jun 13, 2018

Ah, didn't know it supported basic auth. Good to know.

Here is the output for info.cgi

{
    "BHA": {
        "RETURNCODE": "1",
        "VERSION": [
            {
                "FIRMWARE": "000111",
                "BUILD_NUMBER": "15236052",
                "WIFI_MAC_ADDR": "",
                "RELAYS": [
                    "1"
                ],
                "DEVICE-TYPE": "DoorBird D202B"
            }
        ]
    }
}

Here is what the schedule looks like when I reset everything. I'm not sure where the motion schedule is from because it doesn't show in the app.

[
    {
        "input": "motion",
        "param": "",
        "output": [
            {
                "enabled": "1",
                "param": "1",
                "schedule": {
                    "weekdays": [
                        {
                            "to": "604800",
                            "from": "0"
                        }
                    ]
                },
                "event": "http"
            }
        ]
    }
]

I manually removed favorites, so those are clear.

Here is what the schedule and favorites look like after registering events via HA.

[
    {
        "output": [
            {
                "schedule": {
                    "weekdays": [
                        {
                            "to": "604800",
                            "from": "0"
                        }
                    ]
                },
                "enabled": "1",
                "event": "http",
                "param": "0"
            }
        ],
        "input": "doorbell",
        "param": "1"
    },
    {
        "input": "motion",
        "param": "",
        "output": [
            {
                "enabled": "1",
                "param": "1",
                "schedule": {
                    "weekdays": [
                        {
                            "to": "604800",
                            "from": "0"
                        }
                    ]
                },
                "event": "http"
            }
        ]
    }
]
    "http": {
        "0": {
            "title": "Home Assistant on http://192.168.1.13:8123/api/doorbird/front_door_button (doorbell events)",
            "value": "http://192.168.1.13:8123/api/doorbird/front_door_button/api/doorbird/doorbell"
        },
        "1": {
            "title": "Home Assistant on http://192.168.1.13:8123/api/doorbird/front_door_motion (motion events)",
            "value": "http://192.168.1.13:8123/api/doorbird/front_door_motion/api/doorbird/motion"
        }
    }
}

EDIT: I did confirm that I have latest version of iOS app.

@Klikini

This comment has been minimized.

Contributor

Klikini commented Jun 13, 2018

HA didn't delete the schedule that already existed, but that schedule doesn't look like it came from the app because its output event is http and not notify.


Now that I look at the API docs again, I found this:
image

But that still doesn't explain why mine works without the notify schedules and yours doesn't. Maybe try restarting your device and/or emailing DoorBird support?

@oblogic7

This comment has been minimized.

Contributor

oblogic7 commented Jun 13, 2018

@oblogic7

This comment has been minimized.

Contributor

oblogic7 commented Jun 13, 2018

If I manually populate and save the push notifications schedule in the iOS app the schedule.cgi endpoint now shows the following entry for doorbell.

    {
        "input": "doorbell",
        "param": "1",
        "output": [
            {
                "param": "0",
                "enabled": "1",
                "schedule": {
                    "weekdays": [
                        {
                            "to": "1799",
                            "from": "0"
                        }
                    ]
                },
                "event": "http"
            },
            {
                "event": "notify",
                "param": "",
                "schedule": {
                    "weekdays": [
                        {
                            "to": "104399",
                            "from": "104400"
                        }
                    ]
                }
            }
        ]
    },
@Klikini

This comment has been minimized.

Contributor

Klikini commented Jun 13, 2018

Yep, here's the full schedule.cgi output:

[
  {
    "input": "doorbell",
    "param": "1",
    "output": [
      {
        "event": "notify",
        "param": "",
        "schedule": {
          "weekdays": [
            {
              "from": "104400",
              "to": "104399"
            }
          ]
        }
      }
    ]
  },
  {
    "input": "motion",
    "param": "",
    "output": [
      {
        "event": "notify",
        "param": "",
        "schedule": {
          "weekdays": [
            {
              "from": "104400",
              "to": "104399"
            }
          ]
        }
      }
    ]
  }
]

I still get mobile pushes if I delete those, though.

@oblogic7

This comment has been minimized.

Contributor

oblogic7 commented Jun 13, 2018

And the http entries are added to the response if you register events from HA?

@Klikini

This comment has been minimized.

Contributor

Klikini commented Jun 13, 2018

I think in most cases it will be:

  1. User uses app to configure schedule
  2. User starts HA, which adds its own entries
  3. Both exist, device works with both
@Klikini

This comment has been minimized.

Contributor

Klikini commented Jun 13, 2018

They were added yesterday, yes. But again, my python setup broke apparently, so I'll have to fix that before I can run HA again.

@oblogic7

This comment has been minimized.

Contributor

oblogic7 commented Jun 13, 2018

I'm wondering if Android notifications do not use the schedule for some reason? That would explain why you still receive them with the entries removed. Working on an email to support now. Will run this behavior by them and see what they say. Seems like something is definitely not working as expected.

message = 'Clearing schedule for {}'.format(slug)
return web.Response(status=200, text=message)


This comment has been minimized.

@houndci-bot

houndci-bot Oct 24, 2018

blank line at end of file


class ConfiguredDoorbird():
class ConfiguredDoorBird(object):

This comment has been minimized.

@houndci-bot

houndci-bot Oct 24, 2018

expected 2 blank lines, found 1


_LOGGER.info("You may use the following event name for automations"
": %s_%s", DOMAIN, slug)
def get_doorstation_by_slug(hass, slug):

This comment has been minimized.

@houndci-bot

houndci-bot Oct 24, 2018

expected 2 blank lines, found 1

oblogic7 added some commits Oct 25, 2018

{'slug': slug})

message = 'Clearing schedule for {}'.format(slug)
return web.Response(status=200, text=message)

This comment has been minimized.

@houndci-bot

houndci-bot Oct 25, 2018

blank line at end of file

oblogic7 added some commits Oct 25, 2018

@oblogic7 oblogic7 referenced this pull request Oct 25, 2018

Merged

Doorbird auth and event data #6245

2 of 2 tasks complete
@oblogic7

This comment has been minimized.

Contributor

oblogic7 commented Oct 25, 2018

This should be good to go assuming no further changes are requested.

Changes from #6245 are included here now since they would have conflicted with the changes on this PR. Both this PR and $6245 include breaking changes for the Doorbird component, so better to group them anyway.

name = '{} {}'.format(self.name, SENSOR_TYPES[sensor_type]['name'])
slug = slugify(name)

url = '{}{}/{}?token={}'.format(hass_url, API_URL, slug,

This comment has been minimized.

@balloob

balloob Nov 1, 2018

Member

I know that this is an old PR but this doesn't make sense anymore. We now have a webhook component that does exactly this.

This comment has been minimized.

@oblogic7

oblogic7 Nov 1, 2018

Contributor

Do you have a link to some documentation or maybe some other components that have implemented via webhook component?

This comment has been minimized.

@balloob

balloob Nov 1, 2018

Member

IFTTT component.

@balloob

This comment has been minimized.

Member

balloob commented Nov 1, 2018

It's fine. It's not perfect, would be better using the webhook component, but it's ok to merge I guess.

@balloob balloob merged commit e9ae862 into home-assistant:dev Nov 1, 2018

5 checks passed

Hound No violations found. Woof!
WIP Legacy commit status override — see details
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 93.273%
Details

@wafflebot wafflebot bot removed the in progress label Nov 1, 2018

@WoLpH

This comment has been minimized.

Contributor

WoLpH commented Nov 1, 2018

@oblogic7 When testing it I get the following error:

Invalid config for [doorbird]: required key not provided @ data['doorbird']['token']. Got None.

Do you have any documentation about how/where to fetch the token? It's not in the documentation that was just merged.

@oblogic7

This comment has been minimized.

Contributor

oblogic7 commented Nov 1, 2018

@balloob balloob referenced this pull request Nov 9, 2018

Merged

0.82 #18335

@o0linus0o

This comment has been minimized.

o0linus0o commented Nov 30, 2018

Hi,
facing issues to load component after 0.82 (doorbird api v2). Debug error log gives me:

2018-11-30 19:59:19 ERROR (MainThread) [homeassistant.setup] Error during setup of component doorbird
Traceback (most recent call last):
  File "/usr/src/app/homeassistant/setup.py", line 148, in _async_setup_component
    component.setup, hass, processed_config)  # type: ignore
  File "/usr/local/lib/python3.6/concurrent/futures/thread.py", line 56, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/src/app/homeassistant/components/doorbird.py", line 108, in setup
    doorstation.update_schedule(hass)
  File "/usr/src/app/homeassistant/components/doorbird.py", line 199, in update_schedule
    self._register_event(url, sensor_type, schedule)
  File "/usr/src/app/homeassistant/components/doorbird.py", line 220, in _register_event
    if not self.webhook_is_registered(hass_url):
  File "/usr/src/app/homeassistant/components/doorbird.py", line 278, in webhook_is_registered
    favs = favs if favs else self.device.favorites()
  File "/usr/local/lib/python3.6/site-packages/doorbirdpy/__init__.py", line 206, in favorites
    return self.__request(self.__url("favorites.cgi", auth=False))
  File "/usr/local/lib/python3.6/site-packages/doorbirdpy/__init__.py", line 297, in __request
    body_data = json.loads(body_json)
  File "/usr/local/lib/python3.6/json/__init__.py", line 354, in loads
    return _default_decoder.decode(s)
  File "/usr/local/lib/python3.6/json/decoder.py", line 339, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/local/lib/python3.6/json/decoder.py", line 357, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

Doorbird config is OK:

doorbird:
  token: ABCDEFGHIJK
  devices:
   - host: 192.168.x.x
     username: xx
     password: xx
     name: frontdoor
     monitored_conditions:
      - doorbell
      - motion

Any hint on the JSONDecodeError?

@oblogic7

This comment has been minimized.

Contributor

oblogic7 commented Nov 30, 2018

@o0linus0o

This comment has been minimized.

o0linus0o commented Nov 30, 2018

Make sure your DoorBird user has API Operator permission.

On Fri, Nov 30, 2018 at 1:17 PM o0linus0o ***@***.***> wrote: Hi, facing issues to load component after 0.82 (doorbird api v2). Debug error log gives me: 2018-11-30 19:59:19 ERROR (MainThread) [homeassistant.setup] Error during setup of component doorbird Traceback (most recent call last): File "/usr/src/app/homeassistant/setup.py", line 148, in _async_setup_component component.setup, hass, processed_config) # type: ignore File "/usr/local/lib/python3.6/concurrent/futures/thread.py", line 56, in run result = self.fn(*self.args, **self.kwargs) File "/usr/src/app/homeassistant/components/doorbird.py", line 108, in setup doorstation.update_schedule(hass) File "/usr/src/app/homeassistant/components/doorbird.py", line 199, in update_schedule self._register_event(url, sensor_type, schedule) File "/usr/src/app/homeassistant/components/doorbird.py", line 220, in _register_event if not self.webhook_is_registered(hass_url): File "/usr/src/app/homeassistant/components/doorbird.py", line 278, in webhook_is_registered favs = favs if favs else self.device.favorites() File "/usr/local/lib/python3.6/site-packages/doorbirdpy/init.py", line 206, in favorites return self.__request(self.__url("favorites.cgi", auth=False)) File "/usr/local/lib/python3.6/site-packages/doorbirdpy/init.py", line 297, in __request body_data = json.loads(body_json) File "/usr/local/lib/python3.6/json/init.py", line 354, in loads return _default_decoder.decode(s) File "/usr/local/lib/python3.6/json/decoder.py", line 339, in decode obj, end = self.raw_decode(s, idx=_w(s, 0).end()) File "/usr/local/lib/python3.6/json/decoder.py", line 357, in raw_decode raise JSONDecodeError("Expecting value", s, err.value) from None json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0) Doorbird config is OK: doorbird: token: ABCDEFGHIJK devices: - host: 192.168.x.x username: xx password: xx name: frontdoor monitored_conditions: - doorbell - motion Any hint on the JSONDecodeError? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#14933 (comment)>, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbs0CfPZu7lGVnd08oLw5d1Pu6qny50ks5u0YQ0gaJpZM4UjcFL .

That's it - problem solved. Thanks a lot!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Nov 30, 2018

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