Skip to content
This repository was archived by the owner on Oct 11, 2022. It is now read-only.

load available programs from api and add service calls for appliance programs #49

Closed
cycl0psg wants to merge 4 commits into
DavidMStraub:masterfrom
cycl0psg:master
Closed

load available programs from api and add service calls for appliance programs #49
cycl0psg wants to merge 4 commits into
DavidMStraub:masterfrom
cycl0psg:master

Conversation

@cycl0psg
Copy link
Copy Markdown

@cycl0psg cycl0psg commented Jan 4, 2020

  • load available programs from api for each appliance instead of having them written in the code

Added handling of appliance specific sensors:

  • add appliance specific sensors and binary_sensors
  • include sensors and binary_sensors device_class and unit implementation
  • usage of homeassistant constants for device classes and sensor units

- add appliance specific sensors and binary_sensors
- include sensors and binary_sensors device_class and unit implementation
- usage of homeassistant constants for device classes and sensor units
@DavidMStraub
Copy link
Copy Markdown
Owner

DavidMStraub commented Jan 6, 2020

Thanks a lot for the PR!

Can you please provide some description so I don't have to dig everything from the source code?

Note that the PR in Home Assistant is currently under review and it's probably quite complicated to include such extensive changes at this point in the review process (especially since some requested changes over there are not yet done). I'd be very grateful if you could help me getting the PR merged.

Edit: link to the PR: home-assistant/core#29214

Create service calls for available programs with specific parameters fetched from api.
Requires this PR to be merged in the homeconnect api DavidMStraub/homeconnect#4

The parameter list for each call is fetched from the api but, since the service descriptions are written in the service.yaml file, services not detailed in this file will have no call parameters specifications detailed unless someone add them.
I've added the one for the appliance I own
@cycl0psg cycl0psg changed the title load available programs from api load available programs from api and add service calls for appliance programs Jan 12, 2020
@cycl0psg
Copy link
Copy Markdown
Author

Create service calls for available programs with specific parameters fetched from api.
Requires this PR to be merged in the homeconnect api DavidMStraub/homeconnect#4

The parameter list for each call is fetched from the api but, since the service descriptions are written in the service.yaml file, services not detailed in this file will have no call parameters specifications detailed unless someone add them.
I've added the one for the appliance I own

@cycl0psg
Copy link
Copy Markdown
Author

cycl0psg commented Jan 12, 2020

I've also rebased the PR to your last changes to avoid conflicts. I'll keep it updated until it can be merged

@DavidMStraub
Copy link
Copy Markdown
Owner

@cycl0psg, I need to apologize for not having reviewed your PR yet. In the last couple of weeks I tried to fix the outstanding bug DavidMStraub/homeconnect#3, to no avail so far, and didn't have the energy to do anything in addition. I hope I can do it soon.

@cycl0psg
Copy link
Copy Markdown
Author

Don't worry, I have no rush

@DavidMStraub
Copy link
Copy Markdown
Owner

Hi, a couple of comments:

  • the function format_key was removed due to a request in the upstream PR but it seems to still be present in your code.
  • if I understand correctly, you get rid of hard-coded programs and fetch the available programs from the API. However, the reason I didn't do that was that this requires the device to be switched on during initialization of the component, which we cannot assume. Unfortunately, a device that is switched off does not report its available programs. Did you somehow manage to work around that?

device.entities += entity_list
entities += entity_list
if device.has_programs:
services = device.get_programs_services()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line prevents me to have an HC integration up and running (I have a Bosch dishwasher).
Here is the detailed error:

2020-04-06 11:56:54 ERROR (MainThread) [homeassistant.components.sensor] Error while setting up homeconnect platform for sensor
Traceback (most recent call last):
  File "/home/ligio/workspace/home-assistant/homeassistant/helpers/entity_platform.py", line 179, in _async_setup_platform
    await asyncio.wait_for(asyncio.shield(task), SLOW_SETUP_MAX_WAIT)
  File "/usr/lib/python3.7/asyncio/tasks.py", line 442, in wait_for
    return fut.result()
  File "/home/ligio/.homeassistant/custom_components/homeconnect/sensor.py", line 41, in async_setup_entry
    async_add_entities(await hass.async_add_executor_job(get_entities), True)
  File "/usr/lib/python3.7/concurrent/futures/thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/home/ligio/.homeassistant/custom_components/homeconnect/sensor.py", line 29, in get_entities
    services = device.get_programs_services()
  File "/home/ligio/.homeassistant/custom_components/homeconnect/api.py", line 229, in get_programs_services
    options = self.appliance.get_program_options(p)
AttributeError: 'HomeConnectAppliance' object has no attribute 'get_program_options'

@cycl0psg
Copy link
Copy Markdown
Author

cycl0psg commented Apr 6, 2020

  • the function format_key was removed due to a request in the upstream PR but it seems to still be present in your code.

At the moment I don't remember the reason for this but I think it can be handled in another way

  • if I understand correctly, you get rid of hard-coded programs and fetch the available programs from the API. However, the reason I didn't do that was that this requires the device to be switched on during initialization of the component, which we cannot assume. Unfortunately, a device that is switched off does not report its available programs. Did you somehow manage to work around that?

I only have an Oven which even when is off is probably always connected so I didn't notice that behavior. Anyway we can leave the hard-coded programs as a fallback in case the lookup don't succeed or try and create a cache of the discovered programs and use those.

I'm not able to do anything for the time being because I'm quarantined away from home and I can't have access to development or appliance. I don't know for how long so I have to ask you to be patient and wait for some weeks until I can get back home.
If you prefer to not have the pr pending you can reject it and I'll reopen it in the future

@cycl0psg cycl0psg closed this Apr 6, 2020
@cycl0psg cycl0psg reopened this Apr 6, 2020
@badguy99
Copy link
Copy Markdown
Contributor

Just wondering if more work is going to be done on this PR, as I'd love to be able to make use of service calls to set delayed start time after remote start is enabled on my dishwasher.
Thanks for the work so far on it!

@DavidMStraub
Copy link
Copy Markdown
Owner

Sorry about that - the PR in Core, that was based on the version before this PR, took such a huge amount of time and required so many changes, that I did not have time or energy to give as much attention to this as it deserved.

But I agree that service calls are an important feature.

I would suggest to start from scratch and with only one feature at a time, in particular not mixing the available programs and service calls. @cycl0psg, would you be interested in that after waiting so long?

@badguy99
Copy link
Copy Markdown
Contributor

No need to apologise - what you have done with this is amazing! I know what a time suck these things can be, and how hard it can be to make time for everything.
I've got an automation triggering based on what you have done so far - it detects power on and door closed and remote start enabled, and then sends me a notification to tell me what delayed start time to set at the moment. Getting that final step automated with service calls will be the icing on the cake.

@Pe-MaKer
Copy link
Copy Markdown

would you be interested in that after waiting so long?

Hi @DavidMStraub ,
I've been using your integration since December. We' ve been talking about it in the HA community. Thank you for your support and all the work you have invested.

There is very little left on my wish list:

@DavidMStraub DavidMStraub mentioned this pull request May 28, 2020
@DavidMStraub
Copy link
Copy Markdown
Owner

Since I've started implenting service calls, I will close this now.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants