Skip to content

Adds useful attributes to RainMachine programs and zones#14087

Merged
balloob merged 13 commits intohome-assistant:devfrom
bachya:rainmachine-2
May 8, 2018
Merged

Adds useful attributes to RainMachine programs and zones#14087
balloob merged 13 commits intohome-assistant:devfrom
bachya:rainmachine-2

Conversation

@bachya
Copy link
Copy Markdown
Contributor

@bachya bachya commented Apr 26, 2018

Description:

This PR adds several useful attributes to RainMachine programs and zones, including:

  • Restrictions
  • Cycles
  • Capacities
  • Sun Type
  • Soil Type
  • Vegetation Type

Additionally, under the hood, the process of migrating common RainMachine data/logic into the component created by #14225 starts.

Related issue (if applicable): N/A

Pull request in home-assistant.github.io with documentation (if applicable): N/A

Example entry for configuration.yaml (if applicable):

rainmachine:
  ip_address: 192.168.1.100
  password: abc123

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:
- [ ] Documentation added/updated in home-assistant.github.io

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 dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
- [ ] New files were added to .coveragerc.

@bachya bachya changed the title WIP: Adds useful attributes to RainMachine programs and zones Adds useful attributes to RainMachine programs and zones Apr 26, 2018
@bachya bachya changed the title Adds useful attributes to RainMachine programs and zones WIP: Adds useful attributes to RainMachine programs and zones May 1, 2018
@bachya bachya changed the title WIP: Adds useful attributes to RainMachine programs and zones Adds useful attributes to RainMachine programs and zones May 1, 2018
Comment thread homeassistant/components/rainmachine.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should just use mac and type. I don't know what entity id is ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

MAC and type aren't sufficient – in that case, every program would have the same unique ID (e.g., "abcdef1234_program"). Same with zones. entity_id is the ID of that specific program or zone, which gives results like "abcdef1234_program_2". Let me know if I'm thinking about this wrong or doing the job in a confusing manner.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That makes sense. Got confused because Home Assistant also has the concept of entity ids.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should never have relative time in the attributes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

10-4.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should only store information in the attributes that is relevant in further explaining the state.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just because we have the data, doesn't mean that we should add it to our state machine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've been a tad over-zealous. 😄 Question: what about attributes that don't relate directly to the state, but would still be useful in influencing the state? Example: I have an attribute for soil type; it'd be great to use this in my automations for the switch itself (e.g., "If the temperature goes above 80 degrees, stop watering zones with Clay Loam soil").

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That can stay. I guess that is similar to things like max_temp for climate devices.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @balloob. I've removed all frivolous attributes and all attributes that (a) aren't directly related to the state and (b) aren't useful in influencing the state through automation. Let me know your thoughts?

@callback
def _program_updated(self):
"""Update state, trigger updates."""
self.schedule_update_ha_state(True)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use async_schedule_update_ha_state.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Got it. For my own knowledge, this is because the @callback decorator declares that anything in the method body is safe for the event loop, yes?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes! When the dispatcher calls this method it uses the core method async_add_job which checks if the function/method has been decorated with callback, and if so, schedules a call to it on the event loop. Otherwise it will be run in the thread pool (if it's not a coroutine or coroutine function).

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Looks good!

@balloob balloob merged commit f516cc7 into home-assistant:dev May 8, 2018
@bachya bachya deleted the rainmachine-2 branch May 8, 2018 22:49
@balloob balloob mentioned this pull request May 28, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
…ant#14087)

* Starting to add attributes

* All attributes added to programs

* Basic zone attributes in place

* Added advanced properties for zones

* Working to move common logic into component + dispatcher

* We shouldn't calculate the MAC with every entity

* Small fixes

* Small adjustments

* Owner-requested changes

* Restart

* Restart part 2

* Added ID attribute to each switch

* Collaborator-requested changes
@home-assistant home-assistant locked and limited conversation to collaborators Sep 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants