Add new sensor for fyta - #180328
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a diagnostic FYTA sensor intended to expose the plant sensor’s last update time.
Changes:
- Adds the FYTA sensor entity description and translation.
- Updates generated MDI frontend metadata.
- Extends diagnostic sensor coverage conceptually, but snapshots are missing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
homeassistant/components/fyta/sensor.py |
Defines the new diagnostic sensor. |
homeassistant/components/fyta/strings.json |
Adds its display name. |
pylint/plugins/pylint_home_assistant/generated/mdi_icons.py |
Changes generated frontend version metadata. |
Suppressed comments (2)
homeassistant/components/fyta/sensor.py:138
- Use the timestamp contract for this date-and-time value. The FYTA fixture provides
last_updatedas2023-01-10 10:10:00; declaring it asDATEmislabels a datetime and can make consumers parse or display it incorrectly. UseTIMESTAMPand normalize it to a timezone-aware datetime if the library returns a naive value.
device_class=SensorDeviceClass.DATE,
homeassistant/components/fyta/sensor.py:140
- Update the sensor snapshot to cover the new entity.
test_all_entitiescurrently has no entry/state snapshot for this sensor, so the key mismatch and the datetime device-class contract are not exercised.
value_fn=lambda plant: plant.last_updated,
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
homeassistant/components/fyta/sensor.py:138
- Expose this value as a timezone-aware timestamp instead of a date.
plant.last_updatedcontains a time component (for example, the FYTA fixture supplies2023-01-10 10:10:00), soDATEmisclassifies the value and can discard or misrender the update time; useTIMESTAMPand ensure the library value has the correct timezone.
device_class=SensorDeviceClass.DATE,
homeassistant/components/fyta/sensor.py:140
- Document the new sensor in the FYTA documentation and link that documentation PR here. The published FYTA “Supported functionality” sensor table enumerates each sensor but does not include
last_sensor_update, while this PR leaves the documentation link and checklist item empty.
FytaSensorEntityDescription(
key="last_sensor_update",
translation_key="last_sensor_update",
device_class=SensorDeviceClass.DATE,
entity_category=EntityCategory.DIAGNOSTIC,
value_fn=lambda plant: plant.last_updated,
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
homeassistant/components/fyta/sensor.py:135
- Add and regenerate the sensor snapshots for this new entity.
test_all_entitiessnapshots every registry entry and state, buttest_sensor.ambrhas nolast_sensor_updateentries, so the existing snapshot test fails when this entity is added.
FytaSensorEntityDescription(
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
homeassistant/components/fyta/sensor.py:138
- Use the timestamp device class and make the test fixtures timezone-aware.
last_updatedis a fulldatetime(the snapshots include a time, and the connector returns it with the configured timezone), whileDATErepresents a calendar date; this prevents Home Assistant and its frontend from handling the value as a timestamp.
device_class=SensorDeviceClass.DATE,
| ), | ||
| FytaSensorEntityDescription( | ||
| key="last_updated", | ||
| translation_key="last_sensor_update", |
There was a problem hiding this comment.
The sensor feels like it's more terminology of Home Assistant. What would it be in the context of Fyta? Last device update? Last plant update? I think it's better to name it to something related to Fyta that users can relate to. :)
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Proposed change
Add a new sensor to track when the FYTA-sensor has last been updated.
Cf. https://github.com/orgs/home-assistant/discussions/3270
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.To help with the load of incoming pull requests: