Add default code to alarm_control_panel #20062
Conversation
src/state-control/alarm_control_panel/ha-state-control-alarm_control_panel-modes.ts
Outdated
Show resolved
Hide resolved
src/state-control/alarm_control_panel/ha-state-control-alarm_control_panel-modes.ts
Outdated
Show resolved
Hide resolved
gjohansson-ST
force-pushed
the
alarm_control_panel-default-code
branch
from
b31c3d1 to
57868d5
Compare
There was a problem hiding this comment.
Code looks good. 👍 I've did a test and everything works as expected. Just there's a small difference in how the card behaves when a default code is correctly set:
- In the more info dialog, you don't show the prompt anymore to enter the alarm code.
- In the alarm card panel, you still see the code display while it's not really needed. It should be hidden there, if it's not needed to fill it in.
gjohansson-ST
force-pushed
the
alarm_control_panel-default-code
branch
from
57868d5 to
b7ed985
Compare
I did an effort to try to figure it out but I hope maybe to get some help here. |
|
Either implement the firstUpdated or connectedCallback in the lit component: I would probably suggest to do connectedCallback, since firstUpdated is called right after the render and obviously would trigger a new render. |
|
Thanks for the help @silamon |
|
Core PR approved |
|
Still a few things to decide on in my opinion:
|
|
I'm also adding the label back to indicate we should "wait for backend" before we can merge. |
I guess that makes sense. The card is a bit odd without anything really in it.
Can you provide some guidance how to fix that. As I have no clue otherwise.
Isn't that an error coming from the respective integration?
That label is very confusing as the core PR is done and ready to merge, we are waiting for the frontend PR |
The design is ok. User can use other card if they don't have code (e.g. tile card)
We should be able to detect the changes using this code: protected updated(changedProps: PropertyValues<typeof this>): void {
super.updated(changedProps);
if (changedProps.has("hass")) {
const oldHass = changedProps.get("hass");
const entityId = this._config?.entity;
if (entityId) {
if (oldHass?.entities[entityId] !== this.hass?.entities[entityId]) {
this._loadEntityRegistryEntry();
}
}
}
} |
I just checked and yes, it's an error coming from the implementing integration. |
gjohansson-ST
force-pushed
the
alarm_control_panel-default-code
branch
from
63f941b to
3121f26
Compare
WalkthroughWalkthroughThe updates introduce Changes
Sequence Diagram(s) (Beta)Setting Alarm Mode with Code ProtectionsequenceDiagram
participant User
participant UI as User Interface
participant AlarmPanel as Alarm Control Panel
participant Backend as Backend Service
User ->> UI: Select Alarm Mode
UI ->> AlarmPanel: Request to set mode with code
AlarmPanel ->> UI: Prompt for code
User ->> UI: Enter code
UI ->> AlarmPanel: Send code and mode
AlarmPanel ->> Backend: Validate code and set mode
Backend -->> AlarmPanel: Mode set confirmation
AlarmPanel -->> UI: Update UI with new mode
UI -->> User: Display mode set confirmation
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (5)
Additional Context UsedBiome (49)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (7)
src/state-control/alarm_control_panel/ha-state-control-alarm_control_panel-modes.ts (2)
Line range hint
54-54: Consider removing the non-null assertion operators (!) where possible to enhance code safety by handling potential null or undefined values explicitly.Also applies to: 55-55, 56-56, 71-71, 72-72, 75-75, 86-86, 87-87, 95-95, 97-97, 131-131
Line range hint
93-93: Specify a more explicit type instead of usinganyto improve type safety and maintainability.- const mode = (ev.detail as any).value as AlarmMode; + const mode = (ev.detail as CustomEventDetail).value as AlarmMode;src/panels/lovelace/cards/hui-alarm-panel-card.ts (2)
Line range hint
163-163: Multiple non-null assertions are used throughout the file. Consider validating these values before use to prevent runtime errors.Also applies to: 164-164, 169-169, 170-170, 175-175, 180-180, 181-181, 281-281, 297-297, 302-302, 303-303, 309-309, 310-310, 316-316, 317-317, 318-318
Line range hint
282-282: Avoid using template literals when not necessary as it can lead to performance overhead.- ? this.hass!.localize(`ui.card.alarm_control_panel.clear_code`) + ? this.hass!.localize("ui.card.alarm_control_panel.clear_code")src/panels/config/entities/entity-registry-settings-editor.ts (3)
Line range hint
283-283: Avoid using non-null assertions.- this.hass.connection._entityRegistry?.refresh(); + this.hass.connection._entityRegistry.refresh();Non-null assertions can lead to runtime errors if the object is actually null. It's safer to handle potential nulls explicitly or ensure that the object is always defined.
Line range hint
757-757: Use template literals for string concatenation.- "ui.dialogs.entity_registry.editor.stream.stream_orientation_" + num.toString(); + `ui.dialogs.entity_registry.editor.stream.stream_orientation_${num}`; - this.hass.localize("ui.dialogs.entity_registry.editor.change_device_area_link") + ")"; + `${this.hass.localize("ui.dialogs.entity_registry.editor.change_device_area_link")})`;Template literals provide a more readable and concise way to handle string concatenation, especially when variables are involved.
Also applies to: 833-834
Line range hint
845-846: Use optional chaining to safely access properties.- this.hass.devices[this.entry.device_id].area_id + this.hass.devices[this.entry.device_id]?.area_idOptional chaining is a safer way to access nested properties, especially when there's a possibility that an intermediate property could be undefined.
src/state-control/alarm_control_panel/ha-state-control-alarm_control_panel-modes.ts
Outdated
Show resolved
Hide resolved
|
Retested, but it keeps reloading the registry again right now, eventually slowing down the frontend. |
I changed it again but I guess you tested it before. |
No, I see. There's nothing that triggers the update and it's impossible from the more-info dialog to retrigger a card update at this time. Using a subscription on the entity does the trick, but I'm unsure if it would be accepted and the most performable solution for this case: |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
src/panels/lovelace/cards/hui-alarm-panel-card.ts (2)
Line range hint
178-178: Avoid using non-null assertions without checks.Consider adding checks before non-null assertions or handling potential nulls to prevent runtime errors.
Also applies to: 179-179, 184-184, 185-185, 190-190, 195-195, 198-198, 201-201, 202-202, 305-305, 321-321, 326-326, 327-327, 333-333, 334-334, 340-340
Line range hint
306-306: Avoid unnecessary template literals.- return this.hass!.localize(`component.alarm_control_panel.entity_component._.state.${entityState}`) || entityState; + return this.hass!.localize("component.alarm_control_panel.entity_component._.state." + entityState) || entityState;
I adapted the code to your proposal and it works like a charm. |
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range comments (7)
src/state-control/alarm_control_panel/ha-state-control-alarm_control_panel-modes.ts (2)
Line range hint
51-51: Specify a type forev.detailto avoid usingany.- const mode = (ev.detail as any).value as AlarmMode; + interface Detail { + value: AlarmMode; + } + const mode = ((ev.detail as Detail).value);
Line range hint
53-53: Avoid using non-null assertions. Instead, ensure that the variables are defined before use.- if (mode === this.stateObj!.state) return; + if (this.stateObj && mode === this.stateObj.state) return; - const oldMode = this._getCurrentMode(this.stateObj!); + if (this.stateObj) { + const oldMode = this._getCurrentMode(this.stateObj); + } - .disabled=${this.stateObj!.state === UNAVAILABLE} + if (this.stateObj) { + .disabled=${this.stateObj.state === UNAVAILABLE} + }Also applies to: 55-55, 89-89
src/data/alarm_control_panel.ts (2)
Line range hint
21-28: The enum declaration should not beconstas it is not a constant value but a type that can be extended or implemented.- export const enum AlarmControlPanelEntityFeature { + export enum AlarmControlPanelEntityFeature {
Line range hint
52-52: Avoid using non-null assertions. Instead, ensure that the variables are defined before use.- hass!.callService("alarm_control_panel", `alarm_${action}`, { + if (hass) { + hass.callService("alarm_control_panel", `alarm_${action}`, { + entity_id: entity, + code, + }); + } else { + console.error("Hass is undefined"); + }src/panels/lovelace/card-features/hui-alarm-modes-card-feature.ts (2)
Line range hint
99-99: Specify a type forev.detailto avoid usingany.- const mode = (ev.detail as any).value as AlarmMode; + interface Detail { + value: AlarmMode; + } + const mode = ((ev.detail as Detail).value);
Line range hint
101-101: Avoid using non-null assertions. Instead, ensure that the variables are defined before use.- if (mode === this.stateObj!.state) return; + if (this.stateObj && mode === this.stateObj.state) return; - const oldMode = this._getCurrentMode(this.stateObj!); + if (this.stateObj) { + const oldMode = this._getCurrentMode(this.stateObj); + } - setProtectedAlarmControlPanelMode(this, this.hass!, this.stateObj!, mode); + if (this.hass && this.stateObj) { + setProtectedAlarmControlPanelMode(this, this.hass, this.stateObj, mode); + } else { + console.error("Hass or StateObj is undefined"); + } - .label=${this.hass.localize("ui.card.alarm_control_panel.disarm")} + if (this.hass) { + .label=${this.hass.localize("ui.card.alarm_control_panel.disarm")} + } - .disabled=${this.stateObj!.state === UNAVAILABLE} + if (this.stateObj) { + .disabled=${this.stateObj.state === UNAVAILABLE} + }Also applies to: 103-103, 118-118, 140-140, 170-170
src/panels/lovelace/cards/hui-alarm-panel-card.ts (1)
Line range hint
177-177: Avoid using non-null assertions and template literals unnecessarily. Instead, ensure that the variables are defined before use and use string concatenation where interpolation is not needed.- this.hass!.localize("state.default.unavailable") + if (this.hass) { + this.hass.localize("state.default.unavailable") + } - this.hass!.localize( + if (this.hass) { + this.hass.localize( `component.alarm_control_panel.entity_component._.state.${entityState}` ) || entityState; + } - const val = (e.currentTarget! as any).value; + if (e.currentTarget) { + const val = (e.currentTarget as HTMLButtonElement).value; + } - callAlarmAction( - this.hass!, - this._config!.entity, - (e.currentTarget! as any).action, - input?.value || undefined - ); + if (this.hass && this._config && e.currentTarget) { + callAlarmAction( + this.hass, + this._config.entity, + (e.currentTarget as HTMLButtonElement).action, + input?.value || undefined + ); + }Also applies to: 178-178, 183-183, 184-184, 184-184, 201-201, 204-204, 207-207, 208-208, 306-306, 307-307, 322-322, 327-327, 328-328, 334-334, 334-334, 335-335, 335-335, 341-341, 342-342
src/state-control/alarm_control_panel/ha-state-control-alarm_control_panel-modes.ts
Show resolved
Hide resolved
src/state-control/alarm_control_panel/ha-state-control-alarm_control_panel-modes.ts
Show resolved
Hide resolved
|
@gjohansson-ST I updated the PR to have it ready for today's beta.
|
Thanks @piitaya 👍 |
Proposed change
Adds capabilities to add a default code to
alarm_control_panelentitiesCore PR: home-assistant/core#112540
Architecture decision: home-assistant/architecture#892
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
New Features
default_codeproperty for alarm control panel entities, enhancing security and user control.Improvements
Refactor