Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Add config option to disable Arm at Home for the alarm panel #122

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ProtoxiDe22
Copy link

I've started using the integration, but in my use case of the alarm panel, i will never use the "Home" arming mode, as such, i thought it could be nice to have an option to disable it in the config flow and having only "Arm Away" and "Disarm" in the Panel.

this PR does exactly that by adding an option in the config flow, and works for both the mail panel and the Areas subpanels. The default configuration does not modify the current behavior of the integration

image

image

I've only added the string for the English language, so that probably needs to be addressed, i added the Italian string as that is my native language, but i would like to hear what's the correct practice for this repo, should we leave the string as is and wait for contributions from native speaking people or should we translate the string with an online service?

@ProtoxiDe22 ProtoxiDe22 changed the title Add config option to disable Arm at Home for the alarm panel [FEATURE] Add config option to disable Arm at Home for the alarm panel Aug 8, 2024
@petrleocompel
Copy link
Owner

This is very user specific scenario. I understand your needs but question is if this what you are asking for is not only an template of alarm_control_panel -> https://www.home-assistant.io/integrations/alarm_control_panel.template/
where you would disable the specific mode.

So in the end there would be no real reason to implement this. I know this seems "easy" to just put it inside of the integration but question is if it is necessary.

@petrleocompel petrleocompel self-assigned this Aug 9, 2024
@petrleocompel petrleocompel self-requested a review August 9, 2024 07:28
@petrleocompel petrleocompel added the question Further information is requested label Aug 9, 2024
@petrleocompel
Copy link
Owner

It different from issue #90 where FR is to disable all arming functionality.

i had inverted the two capabilities
@ProtoxiDe22
Copy link
Author

This is very user specific scenario. I understand your needs but question is if this what you are asking for is not only an template of alarm_control_panel -> https://www.home-assistant.io/integrations/alarm_control_panel.template/ where you would disable the specific mode.

So in the end there would be no real reason to implement this. I know this seems "easy" to just put it inside of the integration but question is if it is necessary.

I see your point, but i do think implementing this in the integration is more elegant and easy for the final user, in my experience creating template entities is kinda tedious and it would be expecially so if an user needs panels for multiple areas, i don't think giving this as an option is a bad idea.

It different from issue #90 where FR is to disable all arming functionality.

as for #90 i guess if you want, for feature completeness i could also add the option to disable ARM_AWAY, however, there's no feature in the alarm control panel entity to enable/disable disarming of the system, so i don't know what could be done about that.

@ProtoxiDe22
Copy link
Author

ProtoxiDe22 commented Aug 9, 2024

In any case, in all of my infinite wisdom, i had the logic backwards yesterday and didn't notice due to lack of sleep. the feature was supposed to remove the option ARM_HOME, was removing the option ARM_AWAY instead. The last commit fixes that

@petrleocompel
Copy link
Owner

I understand the user point. We will see where will go with this. But I will have to add options to disable arming and disarming completely to make it complete. 1 disable option is not a great. Does not make sense in long run.

I will try to take a look over a weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants