Skip to content

Owlet Care Baby Monitor Component#20258

Closed
oblogic7 wants to merge 20 commits into
home-assistant:devfrom
oblogic7:owlet
Closed

Owlet Care Baby Monitor Component#20258
oblogic7 wants to merge 20 commits into
home-assistant:devfrom
oblogic7:owlet

Conversation

@oblogic7
Copy link
Copy Markdown
Contributor

@oblogic7 oblogic7 commented Jan 20, 2019

Description:

Add support for monitoring stats from Owlet Care baby monitoring sock.

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

Example entry for configuration.yaml (if applicable):

owlet:
  username: USER
  password: PASSWORD
  name: BABY_NAME

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

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

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

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

Comment thread homeassistant/components/owlet.py Outdated
Comment thread homeassistant/components/binary_sensor/owlet.py Outdated
@rohankapoorcom
Copy link
Copy Markdown
Member

I would recommend moving all of these files to:
homeassistant/components/owlet/__init__.py (component) and then binary_sensor.py and sensor.py in the same package. More details in (#19948). This makes it easier to understand what platforms a component supports.

Comment thread homeassistant/components/owlet.py Outdated
Comment thread homeassistant/components/sensor/owlet.py Outdated
Copy link
Copy Markdown
Member

@rohankapoorcom rohankapoorcom left a comment

Choose a reason for hiding this comment

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

A few more thoughts (inline) and here:

I would recommend moving all of these files to:
homeassistant/components/owlet/__init__.py (component) and then binary_sensor.py and sensor.py in the same package. More details in (#19948). This makes it easier to understand what platforms a component supports. For a good example, take a look at the hue component.

Comment thread homeassistant/components/owlet.py Outdated
Comment thread homeassistant/components/sensor/owlet.py
Comment thread homeassistant/components/sensor/owlet.py Outdated
Comment thread homeassistant/components/binary_sensor/owlet.py Outdated
Comment thread homeassistant/components/sensor/owlet.py Outdated
@oblogic7
Copy link
Copy Markdown
Contributor Author

oblogic7 commented Feb 6, 2019

@rohankapoorcom I've made the changes as suggested and the component is working. However, I'm getting the following error and I cannot figure out how to resolve it.

2019-02-05 23:04:44 ERROR (MainThread) [homeassistant.loader] Unable to find component binary_sensor.owlet
2019-02-05 23:04:44 ERROR (MainThread) [homeassistant.components.websocket_api.http.connection.4618004016] Error handling message: {'type': 'frontend/get_translations', 'language': 'en', 'id': 13}
Traceback (most recent call last):
  File "/Users/matt/PycharmProjects/home-assistant/homeassistant/components/websocket_api/decorators.py", line 17, in _handle_async_response
    await func(hass, connection, msg)
  File "/Users/matt/PycharmProjects/home-assistant/homeassistant/components/frontend/__init__.py", line 509, in websocket_get_translations
    resources = await async_get_translations(hass, msg['language'])
  File "/Users/matt/PycharmProjects/home-assistant/homeassistant/helpers/translation.py", line 130, in async_get_translations
    resources = await async_get_component_resources(hass, language)
  File "/Users/matt/PycharmProjects/home-assistant/homeassistant/helpers/translation.py", line 107, in async_get_component_resources
    hass, component, language)
  File "/Users/matt/PycharmProjects/home-assistant/homeassistant/helpers/translation.py", line 42, in component_translation_file
    assert module is not None
AssertionError

Any guidance on how to resolve this would be appreciated.

EDIT: Configuration has been updated in PR description. Error happens with updated config as well as with original entries for sensor and binary_sensor platforms.

@rohankapoorcom
Copy link
Copy Markdown
Member

I'm not familiar with the translation code at all. I believe there was a fix that @balloob had done to fix them with embedded components a while back. Can you rebase off of the latest dev branch and see if the same error occurs?

@oblogic7
Copy link
Copy Markdown
Contributor Author

oblogic7 commented Feb 6, 2019

I tried that right after my last comment, but it did not help. Creating the strings file and generating translations did not resolve it either. Maybe somebody else knows what is wrong? Otherwise, this is working and is ready to merge.

@rohankapoorcom
Copy link
Copy Markdown
Member

@home-assistant/core any suggestions regarding the translations?

@oblogic7
Copy link
Copy Markdown
Contributor Author

@rohankapoorcom Can you approve the changes you requested so this PR will appear ready? Maybe somebody will chime in with a solution to the error before merging.

Comment thread .coveragerc Outdated
Comment thread homeassistant/components/owlet/__init__.py
Comment thread homeassistant/components/owlet/__init__.py Outdated
Comment thread homeassistant/components/owlet/__init__.py Outdated
Comment thread homeassistant/components/owlet/__init__.py Outdated
Comment thread homeassistant/components/owlet/__init__.py Outdated
Comment thread homeassistant/components/owlet/sensor.py Outdated
Comment thread homeassistant/components/owlet/sensor.py Outdated
Comment thread homeassistant/components/owlet/sensor.py Outdated
Comment thread homeassistant/components/owlet/sensor.py Outdated
Comment thread homeassistant/components/owlet/sensor.py Outdated
Comment thread homeassistant/components/owlet/const.py Outdated
Comment thread homeassistant/components/owlet/__init__.py Outdated
Comment thread homeassistant/components/owlet/binary_sensor.py
Comment thread homeassistant/components/owlet/sensor.py
@MartinHjelmare
Copy link
Copy Markdown
Member

Can be merged when lint errors are fixed and the branch is rebased to fix coveralls.

@oblogic7
Copy link
Copy Markdown
Contributor Author

@MartinHjelmare Something blew up when I tried to rebase. I'm going to close this PR since I don't trust the state of the working branch. I've moved all of the changes to a clean working branch and have opened #21108 to replace this one.

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.

6 participants