Skip to content
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

Scaffold device automation #26871

Merged
merged 1 commit into from Sep 27, 2019
Merged

Scaffold device automation #26871

merged 1 commit into from Sep 27, 2019

Conversation

balloob
Copy link
Member

@balloob balloob commented Sep 24, 2019

Description:

Scaffold script to bootstrap device automations.

  • python3 -m script.scaffold device_trigger
  • python3 -m script.scaffold device_condition
  • python3 -m script.scaffold device_action

Docs home-assistant/developers.home-assistant#321


async def async_get_triggers(hass: HomeAssistant, device_id: str) -> List[dict]:
"""List device triggers for NEW_NAME devices."""
registry = await entity_registry.async_get_registry(hass)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the script expose the ability to have both device and entity based triggers? Ex: in the case of stateless devices (remotes) there are no entities. Maybe pass the device to a private method to add device triggers, get the entities and then pass them to a private method to add entity triggers if necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a comment with some commented out code from the ZHA integration.

)


async def async_get_conditions(hass: HomeAssistant, device_id: str):
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above. Should this expose the ability to do this at both device and entity levels?

Copy link
Member Author

Choose a reason for hiding this comment

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

We should aim for device conditions to be a layer on top of existing state conditions. We should not have device conditions that get data that is not available in the state machine.

return test_is_on


async def async_get_actions(hass: HomeAssistant, device_id: str) -> List[dict]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above. Should this expose the ability to do this at both device and entity levels?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a comment to the generated code. Let me know if you think that it's enough.


# Add triggers for each entity that belongs to this integration
# TODO add your own triggers. It's ok to not have any triggers.
triggers.append(
Copy link
Contributor

@dmulcahey dmulcahey Sep 24, 2019

Choose a reason for hiding this comment

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

Should these blocks just be part of the comment above to illustrate how to do this? Or is there value in assuming/treating everything works like an on/off type of device? It looks like the script is designed to treat everything like a toggle entity.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was mainly to get tests going and show people how it should look.

You think that we should comment it out to emphasize that it's an example ?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you’re not worried that people will leave it in their implementations then leave it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's leave it in and see what the PRs do.


# Add conditions for each entity that belongs to this integration
# TODO add your own conditions. It's ok to not have any conditions.
conditions.append(
Copy link
Contributor

@dmulcahey dmulcahey Sep 24, 2019

Choose a reason for hiding this comment

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

Same as above. Should this just be part of the comment?


# Add actions for each entity that belongs to this integration
# TODO add your own actions. It's ok to not have any actions.
actions.append(
Copy link
Contributor

@dmulcahey dmulcahey Sep 24, 2019

Choose a reason for hiding this comment

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

Same as above. Should this just be part of the comment?

@balloob balloob mentioned this pull request Sep 24, 2019
5 tasks
@balloob balloob force-pushed the scaffold-device-automation branch 2 times, most recently from eb20b84 to ae25772 Compare September 24, 2019 23:31
@balloob
Copy link
Member Author

balloob commented Sep 24, 2019

I have updated the templates to match our new device automation organisation. It's now 3 templates : device_trigger, device_condition, device_action.

@MartinHjelmare MartinHjelmare moved this from Needs review to Review in progress in Dev Sep 26, 2019
@balloob balloob merged commit 77654da into dev Sep 27, 2019
Dev automation moved this from Review in progress to Done Sep 27, 2019
@delete-merged-branch delete-merged-branch bot deleted the scaffold-device-automation branch September 27, 2019 19:54
@lock lock bot locked and limited conversation to collaborators Sep 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants