feat: add action buttons for charge port, hazard, valet, and windows#1647
feat: add action buttons for charge port, hazard, valet, and windows#1647blka wants to merge 2 commits into
Conversation
|
I thought windows was added as a cover entity? Looks like some of these should be switches? Or would it be too slow from a response standpoint for things like hazard? |
| "open all windows", | ||
| ) | ||
|
|
||
| async def async_close_all_windows(self, vehicle_id: str): |
There was a problem hiding this comment.
Is this worthwhile vs the existing window command?
There was a problem hiding this comment.
Good question. The existing set_windows service (via cover entity) controls individual windows with granular state. These button convenience methods are for the common case of "open all / close all / vent all" — one tap instead of adjusting each window cover individually.
The cover entity is the right UX for per-window control. The buttons are for quick bulk actions. Both use the same underlying set_windows_state API call, just with different WindowRequestOptions.
If you'd prefer these as switches instead of buttons (since they have a toggling feel for open/close), I can convert. But hazard lights and valet mode are fire-and-forget with no toggle state — buttons are the right entity type for those.
There was a problem hiding this comment.
Whenever on off exists with state it should not be a button.
There was a problem hiding this comment.
Hi @cdnninja, I've addressed all your feedback from the latest review:
-
exists_fnon button descriptions — Addedexists_fntoHyundaiKiaButtonDescriptionso entities only show for vehicles that support them. Hazard lights usevehicle.supports_window_control is not None, windows usevehicle.front_left_window_is_open is not None. No more hardcodedifblocks inasync_setup_entry. -
Charge port → switch — Moved from button to switch.
ev_charge_port_door_is_openhas clear on/off state, so a switch is the correct entity type.on_fn= open,off_fn= close. -
Valet mode → switch — Moved from button to switch.
valet_mode_activehas clear on/off state.on_fn= activate,off_fn= deactivate. -
Windows stay as buttons — Windows have 3 states (closed/vent/open), so they don't map cleanly to binary on/off.
vent_all_windowsis also incremental — each press vents a bit more. Buttons are the right fit. -
Hazard lights stay as buttons — Fire-and-forget action with no readable state. No on/off to toggle.
-
Clutter concern — The
exists_fnpattern ensures entities only appear for vehicles that actually support each feature. Charge port switch appears only ifev_charge_port_door_is_open is not None, valet switch only ifvalet_mode_active is not None, etc.
Branch has been rebased onto latest master (conflicts resolved with the new charging schedule switches from #1710).
Ready for re-review.
|
Rebased onto master. Changes since last review:
Regarding the window buttons vs cover entity question: the cover entity ( For hazard lights, valet mode, and charge port — these are fire-and-forget actions with no toggle state, so buttons are the correct entity type. Switches would imply a state that can be read back, which these don't have. Marking as ready for review. |
Mark entities from pending PRs as WIP so the README accurately reflects what's currently available vs what's pending merge: - V2L/V2X status sensors (PR Hyundai-Kia-Connect#1650) - Schedule charge / off-peak switches (PR Hyundai-Kia-Connect#1650) - Climate entity (PR Hyundai-Kia-Connect#1644) - Action buttons: charge port, hazard, valet, windows (PR Hyundai-Kia-Connect#1647) - set_navigation service (PR Hyundai-Kia-Connect#1665)
|
@cdnninja if we are ok with all the fixes, could you take care or this PR? |
Mark entities from pending PRs as WIP so the README accurately reflects what's currently available vs what's pending merge: - V2L/V2X status sensors (PR Hyundai-Kia-Connect#1650) - Schedule charge / off-peak switches (PR Hyundai-Kia-Connect#1650) - Climate entity (PR Hyundai-Kia-Connect#1644) - Action buttons: charge port, hazard, valet, windows (PR Hyundai-Kia-Connect#1647) - set_navigation service (PR Hyundai-Kia-Connect#1665)
cdnninja
left a comment
There was a problem hiding this comment.
A few more comments. I am a bit worried with some of these items adding clutter or adding them for those that can't use them due to model of car. How do we make sure this only shows for people who can use it?
| "open all windows", | ||
| ) | ||
|
|
||
| async def async_close_all_windows(self, vehicle_id: str): |
There was a problem hiding this comment.
Whenever on off exists with state it should not be a button.
Per review feedback on Hyundai-Kia-Connect#1647: - Charge port: replace open/close buttons with single switch (value_fn=ev_charge_port_door_is_open, on=open, off=close) - Valet mode: add as switch (value_fn=valet_mode_active) instead of start/stop buttons - Windows (open/close/vent): keep as buttons — 3-state (closed/vent/open) doesn't fit switch on/off - Hazard lights: keep as buttons (fire-and-forget, no state) - Add exists_fn to HyundaiKiaButtonDescription, remove hardcoded if-else checks from async_setup_entry - Remove charge port and valet button translations, add switch ones
- 9 new buttons: open/close charge port, hazard lights, hazard+horn, start/stop valet mode, open/close/vent all windows - Conditional creation for charge port and window buttons - Add window convenience methods to coordinator
Per review feedback on Hyundai-Kia-Connect#1647: - Charge port: replace open/close buttons with single switch (value_fn=ev_charge_port_door_is_open, on=open, off=close) - Valet mode: add as switch (value_fn=valet_mode_active) instead of start/stop buttons - Windows (open/close/vent): keep as buttons — 3-state (closed/vent/open) doesn't fit switch on/off - Hazard lights: keep as buttons (fire-and-forget, no state) - Add exists_fn to HyundaiKiaButtonDescription, remove hardcoded if-else checks from async_setup_entry - Remove charge port and valet button translations, add switch ones
bdb3146 to
a923adf
Compare
Summary
Adds 9 new action buttons for vehicle control operations.
New buttons
open_charge_portopen_charge_portvehicle.ev_charge_port_door_is_open is not Noneclose_charge_portclose_charge_portvehicle.ev_charge_port_door_is_open is not Nonestart_hazard_lightsstart_hazard_lightsstart_hazard_lights_and_hornstart_hazard_lights_and_hornstart_valet_modestart_valet_modestop_valet_modestop_valet_modeopen_all_windowsopen_all_windowsvehicle.front_left_window_is_open is not Noneclose_all_windowsclose_all_windowsvehicle.front_left_window_is_open is not Nonevent_all_windowsvent_all_windowsvehicle.front_left_window_is_open is not NoneFire-and-forget buttons (hazard, valet) are always created because they have no readable state. Conditional buttons only appear when the vehicle supports the related feature.
Changes
button.py: Add 9 button descriptions withpress_actionmapping to coordinator methodscoordinator.py: Addasync_open_all_windows(),async_close_all_windows(),async_vent_all_windows()usingWindowRequestOptionsandWINDOW_STATEstrings.json+translations/en.json: Translation keys for 9 buttonsTesting
Validated against live Hyundai EU API (Santa Fe HEV):
ev_charge_port_door_is_open is None)front_left_window_is_open=0)open_charge_port,close_charge_port,start_hazard_lights,start_hazard_lights_and_horn,start_valet_mode,stop_valet_mode,set_windows_stateDepends on: #1641 (bug fixes), #1642 (cover — window coordinator methods)
Test plan