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
Add flow and rain sensor support to Hydrawise #116303
Add flow and rain sensor support to Hydrawise #116303
Conversation
Hey there @dknowles2, @ptcryan, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs tests added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once tests are added.
e0bd1a9
to
f86a0d2
Compare
All comments have been addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏻
Thanks!
def _get_zone_next_cycle(sensor: HydrawiseSensor) -> datetime | None: | ||
if (next_run := sensor.zone.scheduled_runs.next_run) is not None: | ||
return dt_util.as_utc(next_run.start_time) | ||
return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Euh, as far as I know, if the date time has timezone data, we still convert it to UTC internally, do I'm wondering if this adds any value now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the other way around. The problem is when the date time does not have any timezone data. Then we'll run into:
ValueError: Invalid datetime: sensor.raised_beds_next_cycle provides state '2024-05-07 12:55:00', which is missing timezone information
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address the comments in a new PR. Thanks!
key="is_watering", | ||
translation_key="watering", | ||
device_class=BinarySensorDeviceClass.RUNNING, | ||
value_fn=lambda watering_sensor: watering_sensor.zone.scheduled_runs.current_run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use parenthesis around the lambda and indent it so that it's easier to distinguish the lambda from the other entity description parameters.
self._attr_unique_id = f"{self._device_id}_{description.key}" | ||
self._attr_device_info = DeviceInfo( | ||
identifiers={(DOMAIN, self._device_id)}, | ||
name=controller.name if zone is None else zone.name, | ||
name=self.zone.name if zone_id is not None else controller.name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
Breaking change
Proposed change
Add flow and rain sensor support to Hydrawise
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: