kjoconnor / pyhatchbabyrest

Python library to control the Hatch Baby Rest device.
MIT License
19 stars 9 forks source link

Home Assistant support #7

Open ViViDboarder opened 2 years ago

ViViDboarder commented 2 years ago

This is just a pointer to the draft PR I have to get support merged into Home Assistant.

https://github.com/home-assistant/core/pull/77381

Depends on #6

kjoconnor commented 1 year ago

Cut a release here and on pypi. Thanks and good luck with HA support, very much looking forward to it!

ViViDboarder commented 1 year ago

Thanks! Not sure how familiar you are with the Home Assistant architecture, but if you have time for a review it might help speed things along.

kjoconnor commented 1 year ago

Anything I can help with to get it merged? Seems like a lot of requirements to getting it into core!

ViViDboarder commented 1 year ago

Thanks. I think I was passing most of them but because it was taking so long I pushed an update for manual setup so I could run it as a custom component but that new flow needs tests.

Any feedback would be great so then next time I push a diff it can hopefully be the last.

ViViDboarder commented 1 year ago

@kjoconnor I just shared some findings with my extended testing of the Home Assistant integration

https://github.com/home-assistant/core/pull/77381#issuecomment-1496302790

In short, I'm noticing the app scheduling not working well when the integration is running. This involves periodically calling refresh_data(). I'm not sure why that would be causing an issue or if it's supposed to send some timing values along with the refresh requests. Any ideas?

kjoconnor commented 1 year ago

Weird - I've never used the scheduling function in the app. I wonder if the software gets momentarily interrupted by having to answer refresh_data() and the app has some functionality for correcting the clock drift while using the app?

Do you have any BLE sniffer available? If not I'll see if I can get mine running over the next few days and try it out. Are the repro steps roughly 1) set up a schedule using the Hatch app 2) run refresh_data() pretty often, and after that I should see the schedule drift?

ViViDboarder commented 1 year ago

I expect so. I think I had Home Assistant polling every 30s or so. Over a day it was typically drifting a few min. A few weeks later it would be off by 30+ min.

What can I use to sniff BLE? I found this article, which could possibly guide me to doing it with my current bluetooth card. https://hackaday.com/2021/03/23/a-crash-course-on-sniffing-bluetooth-low-energy/

kjoconnor commented 1 year ago

That looks like a good article. I ended up ordering one of these to be able to use it with Wireshark but I've had middling results so far - it seems to only capture some traffic at times, but it's possible I haven't found the right configuration yet.

kjoconnor commented 1 year ago

I ended up trying to make a custom component to use via HACS for Home Assistant: https://github.com/kjoconnor/ha-hatch-baby-rest

It kind of works, but the connection drops out a lot. I think it's some combo of: 1) I'm doing things in pyhatchbabyrest that HA doesn't like, like updating state when attempting to just set the brightness or something 2) I'm testing on an old Raspberry Pi which sucks, I may try it next with ESPHome 3) pyhatchbabyrest itself is a bit wasteful, we could probably be smarter about how often to update 4) The device itself kind of stinks. Not very responsive BLE, poorly designed APIs (you have to set the color and brightness together all the time instead of being able to just set one or the other)

In any case - give it a shot, it does in fact work and maybe you will have better results on the BLE connectivity front. Maybe we can work together on fixing it up and just release it as a custom component.

image
kjoconnor commented 1 year ago

Whoops and it was a private repo, just made it public.

ViViDboarder commented 1 year ago

Yea. I had the same issue running my component from my PR after manually copying it into my custom components. Flaky and causing the time offset issue. The latter is what I consider a bigger blocker to merging. I can tell that there are 3-4 neighbors with a Hatch Rest, and I am pretty sure it was throwing off their clocks too.

kjoconnor commented 1 year ago

The power switch on and off seems to be mostly working OK, so I may just reduce my custom component to that, or comment the other components out with a warning or something. I think in general the BLE stack on the Hatch itself kind of stinks so there's only so much we can do.

ViViDboarder commented 1 year ago

You're not seeing the time drift that I saw? It could be something quirky with my adapter or something then. I had it refreshing every 30s by calling refresh_data(), and since that refreshes every attribute via a data update coordinator, it shouldn't matter much how many entities are used.

kjoconnor commented 1 year ago

I don’t use any of the on device scheduling so I don’t ever know what the clock is, sorry!

The on/off functionality has been working well for me on a couple weeks of use.

On Fri, Aug 11, 2023 at 12:18 Ian @.***> wrote:

You're not seeing the time drift that I saw? It could be something quirky with my adapter or something then. I had it refreshing every 30s by calling refresh_data() https://github.com/kjoconnor/pyhatchbabyrest/blob/master/pyhatchbabyrest/pyhatchbabyrestasync.py#L83, and since that refreshes every attribute via a data update coordinator, it shouldn't matter much how many entities are used.

— Reply to this email directly, view it on GitHub https://github.com/kjoconnor/pyhatchbabyrest/issues/7#issuecomment-1675041060, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFSQYNUGQJOVW5O3EJLYF3XUZLNDANCNFSM57XUSOYA . You are receiving this because you were mentioned.Message ID: @.***>

trail-coffee commented 9 months ago

@ViViDboarder are you still having the time drift? Looking at my bluetooth logs from ios, the hatch is sending me the time and i'm updating the hatch pretty often. Maybe this function needs to be implemented?

ViViDboarder commented 9 months ago

So I haven't been running the integration lately. It would be great to implement that feature! Do you have any dumps of the command so that it could be implemented?

ViViDboarder commented 9 months ago

I found out how to read the time, indicated from the byte at index 0 == "T", the next 4 bytes can be read into a bigendian int containing the epoch. I'm not sure the command that needs to be sent though.

trail-coffee commented 9 months ago

@ViViDboarder I think i got it here add time function

edit: wow, never noticed those headers were T, C, S, and P....

ViViDboarder commented 9 months ago

Yea, I noticed those headers when I printed out the whole byte array and casted anything in the ASCII alpha range to chars. Made it easy for me to guess it was going to be ST, but I didn't guess the format was going to be a dt string rather than the epoch in the same format it was read. Oh well!

Particpant commented 1 month ago

I'd been watching the progress of a previous PR to get the Hatch Rest into core HA and was pretty disappointed to see the PR abandoned. I get it- it looks pretty hard to get through the code reviews for core HA, but was wondering if either of you would be able to get what you had into HACS? Even half-baked support would be pretty nice and appreciated!

kjoconnor commented 1 month ago

Yep - I have one here: https://github.com/kjoconnor/ha-hatch-baby-rest

Right now I've disabled all of the entities besides the on/off switch, and even then it doesn't work sometimes. I think the BLE stack on Hatch is not great and is easily overwhelmed, but I haven't tested much more than that. Let me know if you end up making any progress!

ViViDboarder commented 1 month ago

The review process wasn’t really the blocker, but rather getting the stack to work as expected. It works well as a CLI tool for running adhoc commands, but when running frequent polling things get wonky.

I think I have an integration branch with everything (lights, sound, power, etc) already, but I may need to rebase.

I wouldn’t recommend using it though.

trail-coffee commented 1 month ago

Edit: @ViViDboarder I see your PR in HA. that probably should've been my starting point. looks like you got a bunch of comments on it, so maybe it was close to approval? I need to figure out how to test a custom component...

@ViViDboarder, @kjoconnor I'm giving it a shot in the homeassistant core devcontainer (no idea if this is the best way). I've got the ConfigFlow coming up and discovering the device. I catch a Bleak error at device.refresh_data(). I'm thinking this might be one of your bluetooth stack complaints.

looking at the official airthings integration library, the _update_device() function looks like a good start to make some adjustments to refresh_data(). The multiple connection retries with bleak-retry-connector seems like a possible solution.

hoping to give it a shot if i can figure out how to get a custom pypi repo into my devcontainer (first time working with those).

you can see I don't quite have debugging figured out here (code errors out at the refresh_data):

"""Config flow for Hatch Rest."""

import dataclasses
import logging
from typing import Any

from bleak import BleakError
from pyhatchbabyrest import PyHatchBabyRestAsync
import voluptuous as vol

from homeassistant.components.bluetooth import (
    BluetoothServiceInfoBleak,
    async_ble_device_from_address,
    async_discovered_service_info,
)
from homeassistant.config_entries import ConfigFlow, ConfigFlowResult
from homeassistant.const import CONF_ADDRESS, CONF_SENSOR_TYPE

from .const import DOMAIN, MANUFACTURER_ID

_LOGGER = logging.getLogger(__name__)

SERVICE_UUIDS = [
    "0000180a-0000-1000-8000-00805f9b34fb",
]

# Much of this is sourced from the Switchbot official component
def format_unique_id(address: str) -> str:
    """Format the unique ID for a Hatch Baby Rest."""
    return address.replace(":", "").lower()

def short_address(address: str) -> str:
    """Convert a Bluetooth address to a short address."""
    results = address.replace("-", ":").split(":")
    return f"{results[-2].upper()}{results[-1].upper()}"[-4:]

@dataclasses.dataclass
class Discovery:
    """A discovered bluetooth device."""

    name: str
    discovery_info: BluetoothServiceInfoBleak
    device: PyHatchBabyRestAsync

class HatchBabyRestConfigFlow(ConfigFlow, domain=DOMAIN):
    """Handle a config flow for Hatch Rest."""

    VERSION = 1

    def __init__(self) -> None:
        """Initialize the config flow."""
        self._discovered_device: Discovery | None = None
        self._discovered_devices: dict[str, Discovery] = {}

    async def async_step_bluetooth(
        self, discovery_info: BluetoothServiceInfoBleak
    ) -> ConfigFlowResult:
        """Handle the bluetooth discovery step."""
        _LOGGER.debug("Discovered Hatch Baby Rest %s", discovery_info.as_dict())
        await self.async_set_unique_id(format_unique_id(discovery_info.address))
        _LOGGER.debug("Set unique ID")
        self._abort_if_unique_id_configured()
        _LOGGER.debug("Made it past the abort line")

        try:
            _LOGGER.debug("Trying")
            ble_device = async_ble_device_from_address(
                self.hass, discovery_info.address, connectable=True
            )
            _LOGGER.debug("Made it to PyHatchBabyRestAsync with %s", ble_device.address)
            device = PyHatchBabyRestAsync(ble_device, scan_now=False, refresh_now=False)
            _LOGGER.debug("Made device")
        except BleakError as err:
            _LOGGER.error(
                "Error connecting to and getting data from %s: %s",
                discovery_info.address,
                err,
            )
        except Exception as e:
            _LOGGER.debug(e)
            return self.async_abort(reason="unknown")

        try:
            await device.refresh_data()
            _LOGGER.debug("Refreshed data")
        except BleakError as err:
            _LOGGER.error(
                "Error connecting to and getting data from %s: %s",
                discovery_info.address,
                err,
            )
        except Exception as e:
            _LOGGER.debug(e)
            return self.async_abort(reason="unknown")

        self._device_name = device.name
        self._discovered_device = Discovery(
            name=device.name, discovery_info=discovery_info, device=device
        )

        self.context["title_placeholders"] = {
            "name": self._device_name,
            "address": short_address(discovery_info.address),
        }

        return await self.async_step_bluetooth_confirm()

    async def async_step_bluetooth_confirm(
        self, user_input: dict[str, Any] | None = None
    ) -> ConfigFlowResult:
        """Confirm discovery."""
        if user_input is not None:
            return await self._async_create_entry_from_discovery(user_input)

        self._set_confirm_only()
        return self.async_show_form(
            step_id="bluetooth_confirm",
            description_placeholders={
                "name": self.context["title_placeholders"]["name"]
            },
        )

    async def async_step_user(
        self, user_input: dict[str, Any] | None = None
    ) -> ConfigFlowResult:
        """Handle the user step to pick discovered device."""
        if user_input is not None:
            address = user_input[CONF_ADDRESS]
            await self.async_set_unique_id(
                short_address(address), raise_on_progress=False
            )
            self._abort_if_unique_id_configured()
            discovery = self._discovered_devices[address]

            self.context["title_placeholders"] = {"name": discovery.name}

            self._discovered_device = discovery

            return await self._async_create_entry_from_discovery(user_input)

        current_addresses = self._async_current_ids()
        for discovery_info in async_discovered_service_info(self.hass):
            address = discovery_info.address
            if address in current_addresses or address in self._discovered_devices:
                continue

            if MANUFACTURER_ID not in discovery_info.manufacturer_data:
                continue

            try:
                ble_device = async_ble_device_from_address(discovery_info.address)
                device = PyHatchBabyRestAsync(
                    ble_device, scan_now=False, refresh_now=False
                )
                await device.refresh_data()
            except Exception:
                return self.async_abort(reason="unknown")
            name = device.name
            self._discovered_devices[address] = Discovery(name, discovery_info, device)

        if not self._discovered_devices:
            return self.async_abort(reason="no_devices_found")

        titles = {
            address: name for (address, discovery) in self._discovered_devices.items()
        }
        return self.async_show_form(
            step_id="user",
            data_schema=vol.Schema({vol.Required(CONF_ADDRESS): vol.In(titles)}),
        )

    async def _async_create_entry_from_discovery(
        self, user_input: dict[str, Any]
    ) -> ConfigFlowResult:
        address = self._discovered_device.discovery_info.address
        name = self._device_name
        return self.async_create_entry(
            title=name,
            data={
                **user_input,
                CONF_ADDRESS: address,
                CONF_SENSOR_TYPE: "switch",  # is this even required? I have other platforms supported
            },
        )
ViViDboarder commented 1 month ago

You can replace the requirement in the manifest with one using the Git URL of your pyhatch fork instead: https://pip.pypa.io/en/stable/topics/vcs-support/

ViViDboarder commented 1 month ago

The Bleak issue you’re encountering may not be related and could instead be something to do with exposing the Bluetooth device to the container. Not sure if that’s pre configured.

trail-coffee commented 1 month ago

I'll take a look at it after the kids go to bed, I've got the container privileged and mounting dbus. It works at least enough to configure the Bluetooth integration and discover the Hatch.

ViViDboarder commented 1 month ago

Sounds good! That’s one of the struggles, isn’t it? Anyone sufficiently motivated to work on this has kids, and therefore diminished time to work on it.

trail-coffee commented 1 month ago

Got it working with quick and dirty code below, pulled in the logger from hass to make things easier for me as well

    async def refresh_data(self):
        self.logger.debug("Refreshing data.")
        self.device = await self._ensure_scan()

        self.logger.debug("Starting client.")
        async with BleakClient(self.device) as client:
            if client.is_connected:
                try:
                    raw_char_read = await client.read_gatt_char(CHAR_FEEDBACK)
                except BleakError as e:
                    self.logger.error(e)
            else:
                try:
                    await client.connect()
                    raw_char_read = await client.read_gatt_char(CHAR_FEEDBACK)
                except BleakError as e:
                    self.logger.error(e)

        self.logger.debug("Checking response.")
Particpant commented 1 month ago

@trail-coffee if you've got a repo I can pull from, happy to do additional testing as needed.

Particpant commented 1 month ago

The review process wasn’t really the blocker, but rather getting the stack to work as expected. It works well as a CLI tool for running adhoc commands, but when running frequent polling things get wonky.

I think I have an integration branch with everything (lights, sound, power, etc) already, but I may need to rebase.

I wouldn’t recommend using it though.

Aah, from the outside it definitely looked like the whole "hey, pull out all the features", etc, etc dance that perhaps got to be too much. Happy to pull & test whatever you've got (or @trail-coffee's code, whichever is best) if I can pull it into my local install.

ViViDboarder commented 1 month ago

These are my branches:

I think the only thing you’d need to do is rebase the light branch on the main one, and then merge the light and select siren branch’s into the main one. Then that would have all features. Still, the main issue is that it’s not very responsive and frequently goes unavailable.

IamTheFij commented 1 month ago

I wrote a tool to make it easier to manage custom components via the command line, and added the ability to install components from a forked core repo to make it easier to test unreleased core components.

pip install unhacs
unhacs --config <path to ha config dir> add --forked-component <giturl> --branch <branch>

For example, if you're in your ha config dir, unhacs add --forked-component https://github.com/ViViDboarder/home-assistant --branch hatchrest