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

Minor update to manual alarm_control_panel to publish the arming/pending duration #41872

Closed
wants to merge 5 commits into from

Conversation

jcooper-korg
Copy link

@jcooper-korg jcooper-korg commented Oct 15, 2020

Add new state_duration attribute when in arming or pending state, representing the number of seconds until the state change.

Proposed change

In order to show a countdown timer in an alarm panel control card, the card needs a way to know the duration of the current alarm state (when the alarm enters the arming/pending states). There is no other way for the card to obtain this time, since it depends on the alarm panel configuration settings for the Home/Away/etc states.
So when the alarm enters the pending/arming state, it simply adds an attribute "state_duration" to the device_state_attributes.

This is a very minor change, but enables a very nice improvement in the alarm control panel GUI.

An example custom alarm card with full config example, explanation and screenshots is at https://github.com/jcooper-korg/AlarmPanel

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

Full example config is at https://github.com/jcooper-korg/AlarmPanel

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

…resenting the number of seconds until the state change
@homeassistant
Copy link
Contributor

Hi @jcooper-korg,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@jcooper-korg
Copy link
Author

not sure what to do about it failing flake8. it says:

remote: Internal server error
fatal: unable to access 'https://gitlab.com/pycqa/flake8/': The requested URL returned error: 500

Copy link
Contributor

@Bre77 Bre77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@jcooper-korg jcooper-korg changed the title Minor update to manual alarm_control_panel to publish a the arming/pending duration Minor update to manual alarm_control_panel to publish the arming/pending duration Oct 15, 2020
@jcooper-korg
Copy link
Author

jcooper-korg commented Oct 15, 2020

I pushed a couple trivial commits to force it to rerun the flake8 test. but it's still failing due to some server issue. please let me know if there's some other way to retrigger the flake8 test to get this passing!
Update: looks like flake worked after this last commit. now all checks are passing

Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, we won't accept this PR.

return {
ATTR_PREVIOUS_STATE: self._previous_state,
ATTR_NEXT_STATE: self._state,
ATTR_STATE_DURATION: state_duration.total_seconds(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't set config values in the state machine.

@MartinHjelmare
Copy link
Member

I'll close here now. Still, thanks for your contribution!

@jcooper-korg
Copy link
Author

@MartinHjelmare , thanks for your review.

You said

We don't set config values in the state machine.

This change is not intended to set a config value. It is publishing the duration of the current state as an attribute (in device_state_attributes), so that it can be accessed from a client like a Lovelace alarm panel card.

The duration value is chosen by the alarm state machine, depending on its internal state. Yes, the duration ultimately did come from its config settings, but it had to choose the value based on internal alarm state.

Could you please advise how else this value could be obtained to accomplish the same goal?
Thanks again!

@MartinHjelmare
Copy link
Member

I don't have any suggestions at the moment.

@jcooper-korg
Copy link
Author

I don't have any suggestions at the moment.

Can you then please cite a more detailed reason why this was rejected? You said "We don't set config values in the state machine", but that's not what this change is doing, IIUC. I'd like to understand what you're saying so I can try to figure out a different way to accomplish it.
Thanks again,
John

@MartinHjelmare
Copy link
Member

The alarm times are configured. We don't allow storing configuration in the state machine.

@jcooper-korg
Copy link
Author

Can you please point me to documentation/advice on how to access config values from a javascript Lovelace card?
I assumed it should be accessible in the hass object passed into my card's set hass(hass), but I haven't been able to figure it out (and haven't received any replies to my queries in the dev forums, nor found answers by search).

e.g. if my configuration.yaml has the following, I'd need a way to extract the values for armed_away/arming_time and delay_time.

alarm_control_panel:
  - platform: manual
    name: House
    armed_away:
      arming_time: 60
      delay_time: 30

Thanks again,
John

@frenck
Copy link
Member

frenck commented Nov 12, 2020

@jcooper-korg If you have development questions, please join our Discord server. We have developer channels over there to provide developer support.

Thanks 👍

@jcooper-korg
Copy link
Author

Will do, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants