Skip to content

Initial import for HassIO#6935

Merged
balloob merged 10 commits into
home-assistant:devfrom
pvizeli:hassio_v1
Apr 7, 2017
Merged

Initial import for HassIO#6935
balloob merged 10 commits into
home-assistant:devfrom
pvizeli:hassio_v1

Conversation

@pvizeli
Copy link
Copy Markdown
Member

@pvizeli pvizeli commented Apr 4, 2017

Description:

The next generation of IoT device is comming, HassIO. It provide image for most of avilable IoT hardware and make this device maintenance free and easy to use. It is the first private cloud home solution for HomeAssistant and abstract IoT to a new Level with virtualisation. On this place a big thanks to Resin Team.

HassIO become a full UI Integration into hass and make Hardware and HomeAssistant to one peace. At the moment it is not for production use!

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.

Comment thread homeassistant/components/hassio.py Outdated
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 wonder if services is the right way to expose management of the system. The alternative is an API that we call from a specialized frontend panel. The nice thing about API is that we can have return values and know if a service was a success etc.

Copy link
Copy Markdown
Member Author

@pvizeli pvizeli Apr 5, 2017

Choose a reason for hiding this comment

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

We have no access from outside to supervisor. I think that is the best thing for security. So we need passtrough this stuff over hass.

I design the thing in the way that we call /xy/info with config pannel to load data and send it to frontend. So we need only call this getter (like the code from hassbian config) to rebuild infos after service call.

@pvizeli pvizeli changed the title [WIP] Initial import for HassIO Initial import for HassIO Apr 5, 2017
Comment thread homeassistant/components/hassio.py Outdated
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 suggest you create a dict that maps services to commands, payloads and schemas and lookup that here. See cover component for example. It will use a lot fewer lines of code.

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.

done

Comment thread homeassistant/components/hassio.py Outdated
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.

If you make SERVICE_MAP nested you could include a key-pair for the command string and another key-pair for the payload. Then have two cases at lookup-time, ie here, one where addon is None which just passes the command string to the send_command method and another case where addon is not None which uses string formatting on the command string before passing it along.

You would not need this long if - else trail.

Copy link
Copy Markdown
Member Author

@pvizeli pvizeli Apr 6, 2017

Choose a reason for hiding this comment

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

I think we can do that later. for the moment it help for more readable of code and maby we change some things in next version. That is also the reason of v1 on name :)

@pvizeli
Copy link
Copy Markdown
Member Author

pvizeli commented Apr 6, 2017

Ready to merge 💃

@balloob balloob added this to the 0.42 milestone Apr 6, 2017
try:
with async_timeout.timeout(TIMEOUT, loop=self.loop):
request = yield from self.websession.get(
"http://{}{}".format(self._ip, cmd),
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.

What if self._ip is None, this will return an error?

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.

Resolving IP should also happen inside setup, as it's a Home Assistant concern where it comes from.

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.

We can do that. But without IP it will not load the component

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.

done

@asyncio.coroutine
def post(self, request, addon):
"""Set options on host."""
data = yield from request.json()
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.

We should have voluptuous schemas for data. That way any vulnerability in hassio won't be exposed.

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.

I think we make only a passthrought. HassIO will give a error back to frontend if a option will not support it.

@asyncio.coroutine
def get(self, request):
"""Get base data."""
data = yield from self.hassio.send_command(self._url_info)
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.

Should we wrap these in async_timeout ?

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.

We do that on HassIO object

headers=None, allow_redirects=None, timeout=None,
json=None):
"""Match a request against pre-registered requests."""
data = data or json
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.

We should make sure the json is converted to a string and stored in data. That way we can match the string data = "{}" against json = {} (as the requests are the same)

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.

That make it bad for tests... I need revert it on every test case. That will be nonsense

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.

At the end we use that only inside tests, If possible that we have that test friendly?

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.

oh good point, I thought it was used for matching. This is ok.

Comment thread homeassistant/components/hassio.py Outdated

_LOGGER = logging.getLogger(__name__)

TIMEOUT = 900
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.

Why is this timeout so high?

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.

A docker image can have a long time to pull

Comment thread homeassistant/components/hassio.py Outdated
@property
def connected(self):
"""Return True if it connected to HassIO supervisor."""
return self._ip is not 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.

Should there be a no-op API command so that we can connect to HASSIO and verify that we can make a connection? We do something similar for our normal API https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/remote.py#L81-L86

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.

Ok

@balloob balloob merged commit 3e66df5 into home-assistant:dev Apr 7, 2017
balloob pushed a commit that referenced this pull request Apr 8, 2017
* Initial import for HassIO

* Cleanup api code for views

* First unittest for view

* Add test for edit view

* Finish unittest

* fix addons test

* cleanup service.yaml

* Address first round with ping command

* handle timeout dynamic

* fix lint
@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 8, 2017

Cherry-picked into the 0.42 branch

@fabaff fabaff mentioned this pull request Apr 8, 2017
@balloob balloob mentioned this pull request Apr 21, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Jul 17, 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.

5 participants