Add new CentriConnect component#166933
Conversation
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
There was a problem hiding this comment.
Pull request overview
This pull request adds a new CentriConnect/MyPropane component to Home Assistant that integrates tank monitoring devices through a cloud polling API. The integration provides a comprehensive sensor platform for monitoring tank levels, battery status, device location, and various diagnostics.
Changes:
- Adds complete CentriConnect/MyPropane integration with config flow for device authentication
- Implements 16 sensor entities with device information, tank metrics, and diagnostics
- Includes coordinator for polling the CentriConnect API at 6-hour intervals
- Provides diagnostics support for troubleshooting
- Adds comprehensive test coverage with snapshot validation
Reviewed changes
Copilot reviewed 24 out of 30 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/components/centriconnect/sensor.py | Main sensor platform with 16 entity descriptions for various device metrics |
| homeassistant/components/centriconnect/coordinator.py | Data coordinator handling API polling and data normalization |
| homeassistant/components/centriconnect/config_flow.py | Configuration flow for device authentication with error handling |
| homeassistant/components/centriconnect/init.py | Integration setup and teardown |
| homeassistant/components/centriconnect/entity.py | Base entity class with device info and unique ID generation |
| homeassistant/components/centriconnect/diagnostics.py | Diagnostics endpoint for troubleshooting |
| tests/components/centriconnect/ | Comprehensive test suite with snapshot tests and config flow tests |
| manifests/config files | Integration manifest, quality scale, icons, strings, and requirements |
Added suggested display precision to various sensor descriptions for better formatting.
joostlek
left a comment
There was a problem hiding this comment.
Hello!
Before the PR is reviewable, please remove the brand folder and diagnostics from the initial PR. The brands should be merged to our brands repository
|
Filed Brand PR: home-assistant/brands#10070 |
Will add back diagnostics in a later PR
| hass: HomeAssistant, entry: CentriConnectConfigEntry | ||
| ) -> bool: | ||
| """Unload CentriConnect/MyPropane API integration platforms and coordinator.""" | ||
| _LOGGER.info("Unloading CentriConnect/MyPropane API integration") |
| vol.Required(CONF_USERNAME): str, | ||
| vol.Required(CONF_DEVICE_ID): str, | ||
| vol.Required(CONF_PASSWORD): str, |
|
|
||
| async def async_step_user( | ||
| self, user_input: dict[str, Any] | None = None | ||
| ) -> config_entries.ConfigFlowResult: |
There was a problem hiding this comment.
| ) -> config_entries.ConfigFlowResult: | |
| ) -> ConfigFlowResult: |
| } | ||
|
|
||
|
|
||
| class CentriConnectConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): |
There was a problem hiding this comment.
| class CentriConnectConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): | |
| class CentriConnectConfigFlow(ConfigFlow, domain=DOMAIN): |
| except CentriConnectConnectionError: | ||
| errors["base"] = "cannot_connect" | ||
| except CentriConnectTooManyRequestsError: | ||
| errors["base"] = "cannot_connect" | ||
| except CentriConnectNotFoundError: | ||
| errors["base"] = "invalid_auth" | ||
| except CentriConnectEmptyResponseError: | ||
| errors["base"] = "unknown" | ||
| except CentriConnectDecodeError: | ||
| errors["base"] = "unknown" |
There was a problem hiding this comment.
These duplicate ones can be merged
| CentriConnectSensorEntityDescription( | ||
| key=CentriConnectSensorType.LATITUDE, | ||
| translation_key=CentriConnectSensorType.LATITUDE, | ||
| native_unit_of_measurement=DEGREE, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| entity_registry_enabled_default=False, | ||
| value_fn=lambda coord: coord.data.latitude, | ||
| ), | ||
| CentriConnectSensorEntityDescription( | ||
| key=CentriConnectSensorType.LONGITUDE, | ||
| translation_key=CentriConnectSensorType.LONGITUDE, | ||
| native_unit_of_measurement=DEGREE, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| entity_registry_enabled_default=False, | ||
| value_fn=lambda coord: coord.data.longitude, | ||
| ), |
There was a problem hiding this comment.
Why isn't lat/long/altitutde a device tracker?
| CentriConnectSensorEntityDescription( | ||
| key=CentriConnectSensorType.LTE_SIGNAL_LEVEL, | ||
| translation_key=CentriConnectSensorType.LTE_SIGNAL_LEVEL, | ||
| native_unit_of_measurement=PERCENTAGE, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| suggested_display_precision=0, | ||
| value_fn=lambda coord: ( | ||
| # The LTE signal level is estimated based on the LTE signal strength, | ||
| # with -140 dBm or below being 0% and -70 dBm or above being 100%. | ||
| min(1.0, max(((coord.data.lte_signal_strength + 140.0) / 70.0), 0.0)) * 100 | ||
| if coord.data.lte_signal_strength is not None | ||
| else None | ||
| ), | ||
| ), |
There was a problem hiding this comment.
- Either do in library
- I am not sure why we need a percentage as well
| async def async_setup_entry( | ||
| hass: HomeAssistant, | ||
| entry: CentriConnectConfigEntry, | ||
| async_add_entities: AddConfigEntryEntitiesCallback, | ||
| ) -> None: | ||
| """Set up CentriConnect sensor entities from a config entry.""" | ||
| async_add_entities( | ||
| CentriConnectSensor(entry.runtime_data, description) | ||
| for description in ENTITIES | ||
| if description.value_fn(entry.runtime_data) is not None | ||
| ) |
There was a problem hiding this comment.
Have this under the entity descriptions but above the sensor
| CentriConnectSensorEntityDescription( | ||
| key=CentriConnectSensorType.BATTERY_LEVEL, | ||
| translation_key=CentriConnectSensorType.BATTERY_LEVEL, | ||
| native_unit_of_measurement=PERCENTAGE, | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| device_class=SensorDeviceClass.BATTERY, | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| value_fn=lambda coord: ( |
There was a problem hiding this comment.
by using the device class, you don't need all that logic for battery icons
| "critical_level": "Critical Level", | ||
| "low_level": "Low Level", |
There was a problem hiding this comment.
our strings are in Sentence case
Proposed change
Add a new component for CentriConnect/MyPropane tank monitors
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: