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

Add support for SOMA Smartshades devices #26226

Merged
merged 24 commits into from Sep 30, 2019

Conversation

@ratsept
Copy link
Contributor

commented Aug 27, 2019

Description:

Add support for SOMA Smartshades devices through a SOMA Connect HTTP API. Connect image for the Raspberry Pi 3 can be downloaded for free from https://we.tl/t-mdCdhUeUk9 (password: GetYourSoma).

Docs PR:

home-assistant/home-assistant.io#10460

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.
  • I have followed the development checklist

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. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.
@ratsept

This comment has been minimized.

Copy link
Contributor Author

commented Aug 27, 2019

I added the other email to my account. How can I rerun the automatic checks?

@homeassistant

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2019

Hi @ratsept,

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!

.coveragerc Outdated Show resolved Hide resolved
homeassistant/components/soma/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/soma/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/soma/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/soma/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/soma/cover.py Outdated Show resolved Hide resolved
homeassistant/components/soma/cover.py Outdated Show resolved Hide resolved
homeassistant/components/soma/cover.py Outdated Show resolved Hide resolved
homeassistant/components/soma/cover.py Outdated Show resolved Hide resolved
homeassistant/components/soma/cover.py Outdated Show resolved Hide resolved
Dev automation moved this from Needs review to Review in progress Aug 31, 2019
@ratsept

This comment has been minimized.

Copy link
Contributor Author

commented Sep 16, 2019

Hi,

I think I made all the changes to the actual code that you wanted me to but for the autotests I'm really at a loss. I can't seem to find any description or anything on what is actually needed and how to implement them. I have tried looking at the other components and their tests but it is extremely difficult to figure out what is actually needed in my case. I don't think I can have the test actually attempt a connection to our external hardware as this would always fail with nothing there. But without the hardware the test can't really do anything in my case. I can maybe make a test that tries a connection to the hub and checks if it fails with nothing there but that's all I can think of. Or should I send our devices to somebody who would run these tests?

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Sep 16, 2019

We just need to test all the steps and paths in the config flow module. The hardware library should be patched to avoid I/O.

Read some blogs about unit testing and read the pytest docs.

Look at the existing tests we have of different integrations config flow.

@ratsept

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2019

I'm trying to write test autotests now but I'm having a hard time testing the tests :)
I'm developing this on a Raspberry Pi for now and it seems the Pi is not good enough to run Tox. I tried many different flags and things but it still takes hours to run and never actually finishes. One time I got something called "py.test" to run on my test file and that seemed a lot faster but for some reason that is no longer found in my venv.
What can I use to actually run the single tests file to see if I made any mistakes? Ideally it would be something that is rather fast as waiting for many hours after every little change seems bad. Sorry -Python is not my main language and this is a huge project that seems to keep evolving faster than I can keep up.

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Sep 19, 2019

Use pytest to run the tests. You can specify what modules or directory you want to be tested. Please read the pytest docs.

Copy link
Member

left a comment

Please test all entry points in the config flow to completion, and all logic paths within.

tests/components/soma/test_config_flow.py Outdated Show resolved Hide resolved
tests/components/soma/test_config_flow.py Outdated Show resolved Hide resolved
@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Sep 19, 2019

Please solve the merge conflicts.

@ratsept ratsept force-pushed the ratsept:soma branch from 4b447f6 to 79594c4 Sep 24, 2019
@frenck frenck added the docs-missing label Sep 25, 2019
@ratsept ratsept referenced this pull request Sep 26, 2019
2 of 2 tasks complete
…closed position values back and I reversed them in this integration as well
Dev automation moved this from Review in progress to Reviewer approved Sep 26, 2019
.coveragerc Show resolved Hide resolved
Dev automation moved this from Reviewer approved to Review in progress Sep 26, 2019
@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

requirements_test_all.txt is not up to date. Please run script/gen_requirements_all.py and commit the changes.

Please run black code formatter:

black --fast homeassistant tests
Copy link
Member

left a comment

We need to add the library to this list:

TEST_REQUIREMENTS = (

Then run script/gen_requirements_all.py again and commit the changes.

@frenck frenck removed the docs-missing label Sep 27, 2019
flow = config_flow.SomaFlowHandler()
flow.hass = hass
with patch.object(
SomaApi, "list_devices", return_value={}, side_effect=RequestException()

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Sep 27, 2019

Member

We can remove the return value.

Dev automation moved this from Review in progress to Reviewer approved Sep 30, 2019
Copy link
Member

left a comment

Great!


async def async_unload_entry(hass: HomeAssistantType, entry: ConfigEntry):
"""Unload a config entry."""
return True

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Sep 30, 2019

Member

For a future PR we can unload the cover platform here by forwarding the entry unloading to the cover component.

@MartinHjelmare MartinHjelmare merged commit 48d0746 into home-assistant:dev Sep 30, 2019
11 checks passed
11 checks passed
CI Build #20190930.19 succeeded
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (FullCheck Pylint) FullCheck Pylint succeeded
Details
CI (Overview CheckFormat) Overview CheckFormat succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
CI (Tests PyTest Python36) Tests PyTest Python36 succeeded
Details
CI (Tests PyTest Python37) Tests PyTest Python37 succeeded
Details
cla-bot Everyone involved has signed the CLA
codecov/patch 100% of diff hit (target 94.11%)
Details
codecov/project 94.31% (target 90%)
Details
Dev automation moved this from Reviewer approved to Done Sep 30, 2019
@ratsept ratsept deleted the ratsept:soma branch Sep 30, 2019
@lock lock bot locked and limited conversation to collaborators Oct 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
4 participants
You can’t perform that action at this time.