Skip to content

Implement optional External trigger for ZoneMinder switch#6836

Closed
anpetrov wants to merge 1 commit into
home-assistant:devfrom
anpetrov:dev
Closed

Implement optional External trigger for ZoneMinder switch#6836
anpetrov wants to merge 1 commit into
home-assistant:devfrom
anpetrov:dev

Conversation

@anpetrov
Copy link
Copy Markdown
Contributor

@anpetrov anpetrov commented Mar 28, 2017

Currently ZoneMinder switch allows triggering recording by switching
monitor state. Unfortunately changing monitor state over web API seem
to be really slow, because some cameras require long warm-up periods.
As result if you get substantial delay and might even miss an event
that was associated with the trigger. Fortunately ZM provides 'Nodect'
monitor mode where camera is kept warm and it can start recording
immediately. This way you will have a chance to see how your S.W.A.T
team actually captures the imaginary invader approaching your nuclear
facility monitored by homeassistant+zoneminder.

Signed-off-by: Andrey Petrov andrey.petrov@gmail.com

Description:

Pull request in home-assistant.github.io with documentation: home-assistant/home-assistant.io#2350 home-assistant/home-assistant.io#2360

Example entry for configuration.yaml (if applicable):

<see documentation PR>

Checklist:

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

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

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@mention-bot
Copy link
Copy Markdown

@anpetrov, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Khabi, @fabaff and @pschmitt to be potential reviewers.

@homeassistant
Copy link
Copy Markdown
Contributor

Hi @anpetrov,

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!

Comment thread homeassistant/components/zoneminder.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

statement ends with a semicolon

Comment thread homeassistant/components/zoneminder.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

indentation is not a multiple of four

Comment thread homeassistant/components/zoneminder.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

statement ends with a semicolon

Comment thread homeassistant/components/zoneminder.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

statement ends with a semicolon

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line missing indentation or outdented

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line missing indentation or outdented

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line unaligned for hanging indent

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line unaligned for hanging indent

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line unaligned for hanging indent

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

Comment thread homeassistant/components/zoneminder.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (84 > 79 characters)

Currently ZoneMinder switch allows triggering recording by switching
monitor state. Unfortunately changing monitor state over web API seem
to be really slow, because some cameras require long warm-up periods.
As result if you get substantial delay and might even miss an event
that was associated with the trigger. Fortunately ZM provides 'Nodect'
monitor mode where camera is kept warm and it can start recording
immediately. This way you will have a chance to see how your S.W.A.T
team actually captures the imaginary invader approaching your nuclear
facility monitored by homeassistant+zoneminder.

Signed-off-by: Andrey Petrov <andrey.petrov@gmail.com>
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
vol.Required(CONF_COMMAND_ON): cv.string,
vol.Required(CONF_COMMAND_OFF): cv.string,
vol.Optional('ext_trigger_time', default=60): cv.time,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please use constants.

vol.Required(CONF_HOST): cv.string,
vol.Optional(CONF_SSL, default=DEFAULT_SSL): cv.boolean,
vol.Optional(CONF_PATH, default=DEFAULT_PATH): cv.string,
vol.Optional('trigger_port', default=DEFAULT_TRIGGER_PORT): cv.port,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please use a constant.

import socket
sock = socket.socket()
try:
sock.connect((ZM['host'], ZM['trigger_port']))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is crossing the boundary of protocol specific code that is allowed to be in Home Assistant. Please extract the Zoneminder logic into it's own Python library.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

do you mean to shove all that into PyPi package?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 31, 2017

Noticed your docs pr already got merged. I reverted it and have updated the link in your PR description to point at the new docs PR: home-assistant/home-assistant.io#2360

@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 30, 2017

This PR seems to have gone stale. Closing it. You can reopen it when you're ready to finish it.

@balloob balloob closed this Apr 30, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Aug 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants