Skip to content

Added Xeoma camera platform#11619

Merged
fabaff merged 3 commits into
home-assistant:devfrom
jeradM:platform-xeoma
Jan 25, 2018
Merged

Added Xeoma camera platform#11619
fabaff merged 3 commits into
home-assistant:devfrom
jeradM:platform-xeoma

Conversation

@jeradM
Copy link
Copy Markdown
Member

@jeradM jeradM commented Jan 13, 2018

Description:

Added Xeoma camera platform to display Xeoma server camera streams in Home Assistant

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4405

Example entry for configuration.yaml (if applicable):

camera:
  - platform: xeoma
    host: http://localhost:10090

Checklist:

  • The code change is tested and works locally.

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.

@jeradM jeradM requested a review from andrey-git as a code owner January 13, 2018 07:23
@homeassistant
Copy link
Copy Markdown
Contributor

Hi @jeradM,

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!

def async_setup_platform(hass, config, async_add_devices, discovery_info=None):
"""Discover and setup Xeoma Cameras."""
host = config[CONF_HOST]
login = config[CONF_USERNAME] if CONF_USERNAME in config else None
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.

Use login = config.get(CONF_USERNAME). This way it will be None if no username was set in the configuration.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks. Done

new_version = config[CONF_NEW_VERSION]

from pyxeoma.xeoma import Xeoma, XeomaError
xeoma = Xeoma(host, new_version, login, password)
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.

I assume that there will be a trackback if the credentials are wrong or the camera is not reachable. Please catch that, log an error that the users knows what's going on and return. Otherwise the users end up with a non-functional platform.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Xeoma is an NVR similar to BlueIris or Zoneminder so the connection being made is not to individual cameras, but to a web interface that can be enabled within Xeoma which provides direct access to jpgs from all connected cameras. The constructor for the Xeoma object doesn't make any attempt to connect to the Xeoma web server. All of the calls to the library that can fail are inside the following try block. The first function call inside the try block, xeoma.async_test_connection(), will test the connection to the server. If this fails, the library raises a XeomaError with an appropriate message and the platform logs that.

As far as authentication, there are 2 possible failure scenarios that I can see:

  1. Incorrect credentials are provided: The library will attempt to obtain a session key, but fail and it will raise a XeomaError.
  2. No credential are provided, but credentials are required: The library will attempt to discover cameras, but none will be found and it will raise a XeomaError.

In both cases the platform will catch and log the error and since it will only add entities to home assistant for cameras it is able to discover, no non-working camera entities will be added.

The main issue is that the web server that Xeoma provides doesn't handle HTTP status codes correctly. In scenario 2 above, you still get a 200 response which makes it difficult to determine exactly why camera discovery failed since incorrect configuration of the web server module within Xeoma can also create a situation in which no cameras will be discovered (though this should be obvious since the user won't be able to access their cameras in the Xeoma web ui either). The error message provided in this scenario simply informs the user that no cameras were discovered.

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.

pyxeoma is handling all connection issues. So it makes sense to just push then through.

Copy link
Copy Markdown
Member

@fabaff fabaff left a comment

Choose a reason for hiding this comment

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

Thanks 🐦

@fabaff fabaff merged commit 94e270f into home-assistant:dev Jan 25, 2018
@balloob balloob mentioned this pull request Jan 26, 2018
matemaciek pushed a commit to matemaciek/home-assistant that referenced this pull request Jan 27, 2018
* upstream/master: (465 commits)
  Update pyhomematic to 0.1.38 (home-assistant#11936)
  Implement Alexa temperature sensors (home-assistant#11930)
  Fixed rfxtrx binary_sensor KeyError on missing optional device_class (home-assistant#11925)
  Allow setting climate devices to AUTO mode via Google Assistant (home-assistant#11923)
  fixes home-assistant#11848 (home-assistant#11915)
  Add "write" service to system_log (home-assistant#11901)
  Update frontend to 20180126.0
  Version 0.62
  Allow separate command and state OIDs and payloads in SNMP switch (home-assistant#11075)
  Add ERC20 tokens to etherscan.io sensor (home-assistant#11916)
  Report scripts and groups as scenes to Alexa (home-assistant#11900)
  Minor fix to configuration validation and related log line. (home-assistant#11898)
  Multi-Room Support for Greenwave Reality (home-assistant#11364)
  Added Xeoma camera platform (home-assistant#11619)
  Improve foscam library exception support (home-assistant#11701)
  Iota wallet (home-assistant#11398)
  New venstar climate component (home-assistant#11639)
  Update python-wink version and multiple wink fixes/updates. (home-assistant#11833)
  Use API to discover Hue if no bridges specified (home-assistant#11909)
  Clarify emulated hue warning (home-assistant#11910)
  ...
@jeradM jeradM deleted the platform-xeoma branch February 7, 2018 03:02
@home-assistant home-assistant locked and limited conversation to collaborators May 29, 2018
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.

3 participants