Remove friendly name usage from HAControls - #7268
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Android Controls (HAControls) pipeline to avoid reading friendly_name directly from entity attributes, instead building controls from EntitiesForDisplayManager / EntityDisplay* so names, areas, icons, and other display attributes are resolved consistently (including entity registry data).
Changes:
- Refactors
HaControlsProviderServiceto source items fromEntitiesForDisplayManager(snapshot + observe) and removes registry-handling helpers/structures. - Extends
EntityDisplayandEntityhelpers to expose additional display/control data needed by HAControls (number/media player/cover/vacuum controls,device_class,entity_picture). - Migrates individual control implementations to consume
EntityDisplayWithContextrather than rawEntityattributes.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| common/src/test/kotlin/io/homeassistant/companion/android/common/data/integration/EntityTest.kt | Adds unit tests covering new entity helpers (feature bitmask + domain control extraction + display attributes). |
| common/src/main/kotlin/io/homeassistant/companion/android/util/RegistriesDataHandler.kt | Removes obsolete registry lookup helper previously used by HAControls. |
| common/src/main/kotlin/io/homeassistant/companion/android/common/data/integration/Entity.kt | Adds supportsFeature and domain-specific *Controls helpers; restricts friendlyName visibility. |
| common/src/main/kotlin/io/homeassistant/companion/android/common/data/integration/display/EntityDisplay.kt | Extends EntityDisplay with additional attributes/controls and wires them from Entity. |
| common/src/main/kotlin/io/homeassistant/companion/android/common/data/integration/display/AlarmDisplay.kt | Uses the shared supportsFeature helper instead of direct bitmask casting. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/HaControlsProviderService.kt | Switches HAControls data source to EntitiesForDisplayManager, adds failed-item display handling, simplifies domain support gating. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/HaControlInfo.kt | Drops registry area from control metadata (area now comes from EntityDisplayWithContext). |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/HaControl.kt | Uses EntityDisplayWithContext for title/subtitle/state/icon rendering. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/CameraControl.kt | Uses entityPicture from display item for thumbnails. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/ClimateControl.kt | Uses resolved climate controls/modes from the display item. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/CoverControl.kt | Uses resolved cover controls and deviceClass from the display item. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/DefaultButtonControl.kt | Migrates to EntityDisplayWithContext for templates and domain naming. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/DefaultSliderControl.kt | Uses numberControls from the display item for range configuration. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/DefaultSwitchControl.kt | Migrates to EntityDisplayWithContext for templates and domain naming. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/FanControl.kt | Uses resolved fan controls from the display item. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/HaFailedControl.kt | Migrates to display item and uses rawState for notfound/exception rendering. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/LightControl.kt | Uses resolved light controls from the display item. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/LockControl.kt | Migrates to EntityDisplayWithContext for templates. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/MediaPlayerControl.kt | Uses resolved media player controls from the display item. |
| app/src/main/kotlin/io/homeassistant/companion/android/controls/VacuumControl.kt | Uses resolved vacuum controls from the display item (but currently retains shared mutable state). |
Comments suppressed due to low confidence (2)
app/src/main/kotlin/io/homeassistant/companion/android/controls/VacuumControl.kt:57
- performAction relies on the mutable supportsTurnOn state from the last rendered control. Use the action's templateId (systemId) to look up the capability and strip the optional server prefix before calling the HA service.
override suspend fun performAction(integrationRepository: IntegrationRepository, action: ControlAction): Boolean {
integrationRepository.callAction(
action.templateId.split(".")[0],
if (entitySupportsTurnOn) {
if ((action as? BooleanAction)?.newState == true) "turn_on" else "turn_off"
} else if ((action as? BooleanAction)?.newState == true) {
"start"
} else {
"return_to_base"
},
hashMapOf(
"entity_id" to action.templateId,
),
)
app/src/main/kotlin/io/homeassistant/companion/android/controls/HaControlsProviderService.kt:353
- sendControl catches all Exceptions, including CancellationException from a cancelled subscription, and then tries to send a failed control. Rethrow CancellationException so cancellation stops work promptly and doesn't emit extra controls after cancel().
val control = try {
domainToHaControl[if (failed) "ha_failed" else item.domain]?.createControl(
applicationContext,
item,
info,
)
} catch (e: Exception) {
Timber.e(e, "Unable to create control for ${item.domain} entity, sending error entity")
domainToHaControl["ha_failed"]?.createControl(
applicationContext,
failedItem(item.entityId, notFound = false),
info,
)
}
| val colorTint = when { | ||
| item.domain == LIGHT_DOMAIN && item.rawState == "on" -> R.color.colorDeviceControlsLightOn | ||
| item.domain == CAMERA_DOMAIN -> R.color.colorDeviceControlsCamera | ||
| item.domain == CLIMATE_DOMAIN && item.rawState == "heat" | ||
| -> R.color.colorDeviceControlsThermostatHeat | ||
|
|
||
| entity.state in listOf( | ||
| "off", | ||
| "unavailable", | ||
| "unknown", | ||
| ) -> R.color.colorDeviceControlsOff | ||
| item.rawState in listOf( | ||
| "off", | ||
| "unavailable", | ||
| "unknown", | ||
| ) -> R.color.colorDeviceControlsOff | ||
|
|
||
| else -> R.color.colorDeviceControlsDefaultOn | ||
| } | ||
|
|
||
| iconDrawable.setTint(ContextCompat.getColor(context, colorTint)) | ||
| control.setCustomIcon(iconDrawable.toAndroidIconCompat().toIcon(context)) | ||
| } | ||
| } else { | ||
| // Specific override for some domain icons to match HA frontend rather than provided device type | ||
| val iconOverride = listOf(MEDIA_PLAYER_DOMAIN, "number") | ||
| if (entity.domain in iconOverride) { | ||
| val icon = IconicsDrawable(context, entity.getIcon()).apply { sizeDp = 48 } | ||
| val tint = if (entity.isActive()) { | ||
| R.color.colorDeviceControlsDefaultOn | ||
| } else { | ||
| R.color.colorDeviceControlsOff | ||
| } | ||
| icon.setTint(ContextCompat.getColor(context, tint)) | ||
| control.setCustomIcon(icon.toAndroidIconCompat().toIcon(context)) | ||
| } | ||
| else -> R.color.colorDeviceControlsDefaultOn | ||
| } |
There was a problem hiding this comment.
This behavior predates the active state decision elsewhere. I think it might make sense to change but not in this PR considering the scope.
9fa116f to
e1ec1ea
Compare
| entity.domain == CAMERA_DOMAIN -> R.color.colorDeviceControlsCamera | ||
| entity.domain == CLIMATE_DOMAIN && entity.state == "heat" | ||
| -> R.color.colorDeviceControlsThermostatHeat | ||
| // Render the resolved icon to match the HA frontend rather than the provided device type |
There was a problem hiding this comment.
This is actually a behavior change, it used to only use the MDI icon if it was custom as Google's default device type icons are nicely animated when toggling them. For example when toggling a light you get an empty vs. filled lightbulb.
Considering how our icon support has grown and is basically matching the frontend I'll accept it however.
| var sentInitial = false | ||
| val error404 = HttpException(Response.error<ResponseBody>(404, byteArrayOf().toResponseBody())) | ||
|
|
||
| serverManager.webSocketRepository(serverId).getCompressedStateAndChanges(entityIds) |
There was a problem hiding this comment.
This function used to get the compressed state changes only for the selected entities, for responsiveness and speed and data use limitation. Now it fetches all. Is it worth adjusting observeInContext first so that it switches over to the compressed state changes?
| private const val FAILED_STATE_NOT_FOUND = "notfound" | ||
| private const val FAILED_STATE_EXCEPTION = "exception" |
| /** The `entity_picture` attribute of the entity, or null when it has none or it is blank. */ | ||
| fun Entity.entityPicturePath(): String? = (attributes["entity_picture"] as? String)?.takeIf { it.isNotBlank() } |
There was a problem hiding this comment.
If vacuum gets a class with just one vacuum-specific attribute, I think you should also introduce a class for camera controls. When taking a snapshot of a live stream or refreshing (controlling the camera) the entity picture is updated.
Summary
Remove friendly_name usage in HAControls it was accessing it directly through the attributes of the Entity. It uses instead the EntitiesForDisplayManager that gives everything required for the HaControls. I had to extend the EntityDisplay interface to add more attributes.
Checklist
Select exactly one option that describes AI usage in this contribution: