Core: Empty services.yaml file for a lot of integrations

Created on 7 Oct 2019  路  43Comments  路  Source: home-assistant/core

Marco issue for missing services on-boarded documentation (thru developer tools/services tab in HA UI).
Easy fix. Perfect for #Hacktoberfest 馃槃

Edit of 09/09/2019: Not always so easy fix as there are some subtleties. Please read the Warning part at the bottom of the issue.

Home Assistant release with the issue:
0.101.0_dev0

Description of problem:
A lot of current integrations are registering services but provide and empty services.yaml file. It's mean users have no clue how to use the service by using the service tool integrated in HA, except going on online documentation website.
You will find below the current list of integrations with this scenario with a link on the documentation where you should find the data for filling the services.yaml file.

Integrations to be reviewed:

  • [x] arlo (https://www.home-assistant.io/integrations/arlo)
  • [ ] blackbird (https://www.home-assistant.io/integrations/blackbird)
  • [ ] bluesound (https://www.home-assistant.io/integrations/bluesound)
  • [ ] bluetooth_tracker (https://www.home-assistant.io/integrations/bluetooth_tracker)
  • [x] browser (https://www.home-assistant.io/integrations/browser)
  • [ ] cloudflare (https://www.home-assistant.io/integrations/cloudflare)
  • [ ] configurator (https://www.home-assistant.io/integrations/configurator)
  • [ ] demo (https://www.home-assistant.io/integrations/demo)
  • [ ] dominos (https://www.home-assistant.io/integrations/dominos)
  • [x] downloader (https://www.home-assistant.io/integrations/downloader)
  • [x] duckdns (https://www.home-assistant.io/integrations/duckdns)
  • [ ] econet (https://www.home-assistant.io/integrations/econet)
  • [ ] epson (https://www.home-assistant.io/integrations/epson)
  • [ ] flux (https://www.home-assistant.io/integrations/flux)
  • [ ] harmony (https://www.home-assistant.io/integrations/harmony)
  • [ ] html5 (https://www.home-assistant.io/integrations/html5)
  • [x] ifttt (https://www.home-assistant.io/integrations/ifttt)
  • [ ] joaoapps_join (https://www.home-assistant.io/integrations/joaoapps_join)
  • [ ] keyboard (https://www.home-assistant.io/integrations/keyboard)
  • [ ] lifx (https://www.home-assistant.io/integrations/lifx)
  • [ ] local_file (https://www.home-assistant.io/integrations/local_file)
  • [x] logbook (https://www.home-assistant.io/integrations/logbook)
  • [ ] matrix (https://www.home-assistant.io/integrations/matrix)
  • [x] media_extractor (https://www.home-assistant.io/integrations/media_extractor)
  • [ ] mill (https://www.home-assistant.io/integrations/mill)
  • [ ] monoprice (https://www.home-assistant.io/integrations/monoprice)
  • [ ] mysensors (https://www.home-assistant.io/integrations/mysensors)
  • [x] ness_alarm (https://www.home-assistant.io/integrations/ness_alarm)
  • [ ] nuheat (https://www.home-assistant.io/integrations/nuheat)
  • [ ] nuimo_controller (https://www.home-assistant.io/integrations/nuimo_controller)
  • [ ] nuki (https://www.home-assistant.io/integrations/nuki)
  • [ ] onkyo (https://www.home-assistant.io/integrations/onkyo)
  • [ ] onvif (https://www.home-assistant.io/integrations/onvif)
  • [ ] pilight (https://www.home-assistant.io/integrations/pilight)
  • [x] roku (https://www.home-assistant.io/integrations/roku)
  • [x] route53 (https://www.home-assistant.io/integrations/route53)
  • [x] sabnzbd (https://www.home-assistant.io/integrations/sabnzbd)
  • [ ] songpal (https://www.home-assistant.io/integrations/songpal)
  • [ ] soundtouch (https://www.home-assistant.io/integrations/soundtouch)
  • [x] squeezebox (https://www.home-assistant.io/integrations/squeezebox)
  • [x] stream (https://www.home-assistant.io/integrations/stream)
  • [ ] todoist (https://www.home-assistant.io/integrations/todoist)
  • [ ] wemo (https://www.home-assistant.io/integrations/wemo)
  • [ ] xiaomi_miio (https://www.home-assistant.io/integrations/xiaomi_miio)
  • [ ] yamaha (https://www.home-assistant.io/integrations/yamaha)

Integrations OK with an empty services.yaml file:
I will update this list according to our analysis and your comments in this issue.

  • [x] apns (https://www.home-assistant.io/integrations/apns) as name of the serivce is defined by user in the configuration.yaml file
  • [x] esphome (https://www.home-assistant.io/integrations/esphome) as name of the serivce is defined dynamically
  • [x] rest_command (https://www.home-assistant.io/integrations/rest_command) as name of the serivce is defined by user in the configuration.yaml file
  • [x] shell_command (https://www.home-assistant.io/integrations/shell_command) as the name of the service is defined by user un the configuration.yaml file

Integration with services describe in an other integration services.yaml file and need code rework:

  • [ ] channels (https://www.home-assistant.io/integrations/channels) as services are currently documented in media_player integration
  • [ ] facebox (https://www.home-assistant.io/integrations/facebox) as service are currently documented in images_processing` integration
  • [ ] icloud (https://www.home-assistant.io/integrations/icloud) as services are currently documented in device_tracker integration
  • [ ] neato (https://www.home-assistant.io/integrations/neato) as services are currently documented in vacuum integration

Warnings
This issue is not so simple as:

  • It could have good reasons to have an empty services.yaml file (for example if services are registered with user defined named like rest_command).
  • The services.yaml file in an integration folder describe services for the domain of this integration (defined in manifest.yaml file). It's mean that services registered by platforms added in this integration should be documented in the services.yaml file in the platform domain folder (example neato integration registers a specific service for the platform vacuum and the service documentation is in homeasssitant/components/vacuum/services.yaml

In consequence, please update only services documentation for integration you can test with your home-assistant install. Ideally provide a screenshot in your PR as proof of your validation.

There is some history in #23197

Hacktoberfest Help wanted

Most helpful comment

Just thinking out loud... Could it be interesting to actually review the services.yaml format and build a part of the documentation from that file instead of having to duplicate the information in the documentation and in the services.yaml file?
In this case it will always be up to date :)

All 43 comments

@oncleben31 I would like to take this issue :)

Cool. Avoid grouping your contributions in only one PR. I will update the issue according to the contributions.

Just thinking out loud... Could it be interesting to actually review the services.yaml format and build a part of the documentation from that file instead of having to duplicate the information in the documentation and in the services.yaml file?
In this case it will always be up to date :)

@RomRider good idea but this should be discussed with the core team. Perhaps you can open a dedicated topic on the forum or go on discord to start the discussion or open an issue in the architecture repo.

Note that for some of these integrations solving the problem isn't as easy as just filling in or moving the yaml to a new file. The service might be registered under the domain where the yaml description currently resides so the service registration needs to change too before moving the yaml description.

Just a request but please make sure everyone is properly testing the services.yaml and maybe even provide screenshots showing the service and the description so we all know its working. Some of these services need to have the correct name and need to be under the correct domain as pointed out by @MartinHjelmare

I realised that. I will update the issue description

I swear I've searched for past related issues. I missed the #23197, I don't know how.

I added the entries for the downloader component #27553

download_service

@oncleben31 shell_command is basically the same as rest_command in terms of services.
So you should properly move it to 'Integrations OK with an empty services.yaml file'.

And the services.yaml file of the todoist integration is not empty. Tested it on my system and the description is completely visible. Just so you know :)

Thank you to @springstan and @Mofeywalker. Initial list updated.

I added the entry for the media_extractor component.

Bildschirmfoto 2019-10-13 um 20 16 33

I also added the entry for the stream component. :)

Bildschirmfoto 2019-10-13 um 20 38 39

Documentation for local_file service local_file_update_file_path is already added to the services.yaml file of camera integration.

Should it be moved to local_file instead?

Added description for the route53 service.
Screenshot 2019-10-17 at 11 27 41

Documentation for channels services is already added to the services.yaml file of media_player integration.

Should it be moved to channels instead?

apns service names depend on the notifier name specified in configuration.

Documentation for icloud services is already added to the services.yaml file of device_tracker integration.

Should it be moved to icloud instead?

It looks like the update_records service from the cloudflare component doesn't need anything in its services.yml file as it's not taking any parameters, only info from the component configuration.

I add for squeezebox. #28247

image

I added for duckdns. #28248

image

Added for ness alarm #28250
Screenshot 2019-10-27 at 00 38 25
Screenshot 2019-10-27 at 00 38 10

Seems that the local_file component already gets its descriptions and example data from somewhere else.

Missed that one, my bad :sweat:

Onvif component service has defined in camera component. https://github.com/home-assistant/home-assistant/issues/27289#issuecomment-543115397

was just going to add the description to the facebox services.yaml and found out it's already part of the image_proccessing component at
https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/image_processing/services.yaml#L10

Thank you all for the contributions, I've updated the issue.
I've some doubts about local_file (#28330) and onvif (#28349) PR. @ZiroNL and @fabaff, according to my knowledge it shouldn't work without python code rework. Can you check the service description is working and sharing proof ?

@oncleben31
This is my local dev setup (Dutch)

afbeelding

And

afbeelding

@ZiroNL thank you. I still have a doubt according to @dshokouhi comments in another related PR.
To be sure it's working you should test with your modification and by removing the duplicate description in the services.yaml of the camera integration. Can you come back to us after testing this?

The service in bluetooth_tracker is in the device_tracker domain instead of the bluetooth_tracker domain. It would need a code update as part of the services.yaml update

Same issue for flux, epson, blackbird, bluesound, econet, html5, and harmony using the wrong domain. For harmony, it is two services that need updates, harmony_change_channel and harmony_sync

For anyone submitting changes here, please make sure before you decide to update the service domain check the UI and determine if it makes sense. A lot of services are under a certain domain so the drop down to select the entities is populated accordingly. Things like local_file and harmony and others allow the user to select the entity based on the domain. Changing the service domain gets rid of this filtering and shows all entities regardless of domain. It makes the UI less user friendly in this case and we should avoid that.

I proposed an architecture change to address this issue as referenced above

Per @balloob's comment here: https://github.com/home-assistant/home-assistant/pull/28890#issuecomment-558485691 we should not worry about entity discovery in the Dev Tools UI when considering moving services to the proper domain

OK! I have updated all custom services to the proper domain that I could find.

As far as I can tell, there are three integrations remaining with empty services.yaml that aren't explained in the original comment:
configurator
joaoapps_join
apns

apns and joaoapps_join need to have empty services.yaml's because they are registering services, but the service names are dynamic. Unclear what to do about configurator but I will leave that to someone else to figure out.

We can drop the services for configurator.

Is there anything left to do to solve this issue?

I believe this can be closed as the only integrations that are left with empty services.yaml are expected to have empty files

馃帀 Great job everyone ! 馃帀

Was this page helpful?
0 / 5 - 0 ratings