Skip to content

Add cover platform to Dynalite#32594

Merged
MartinHjelmare merged 21 commits intohome-assistant:devfrom
ziv1234:dynalite_cover2
Apr 2, 2020
Merged

Add cover platform to Dynalite#32594
MartinHjelmare merged 21 commits intohome-assistant:devfrom
ziv1234:dynalite_cover2

Conversation

@ziv1234
Copy link
Copy Markdown
Contributor

@ziv1234 ziv1234 commented Mar 8, 2020

Breaking change

Not a breaking change

Proposed change

Adding the cover platform to Dynalite. This is reimplemented from #32511 because I did something wrong with the rebase

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml
bridges:
    - host: 10.0.0.200
      port: 95
      autodiscover: false
      polltimer: 1
      active: off
      # log_level: debug
      area:
        '9':
          name: Office
          channel:
            '1': 
              name: Center
            '2': 
              name: East
              fade: 2
            '3': 
              name: West
        '10':
          name: Cinema
          template: room
          channel:
            '1':
              name: Spot
            '2':
              name: Wall
            '3':
              name: Wall Plug
        '108':
          name: Office Blind
          template: timecover
          tilt: 3
          duration: 75

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@springstan springstan changed the title added the cover. reimplemented from the branch I messed up Add cover platform to Dynalite Mar 9, 2020
@ziv1234 ziv1234 force-pushed the dynalite_cover2 branch 2 times, most recently from 163f415 to 2d4e522 Compare March 16, 2020 17:04
@ziv1234
Copy link
Copy Markdown
Contributor Author

ziv1234 commented Mar 25, 2020

just checking in now that 107 is out. Is there something I should do to get a review or should I just wait? This is a very minor fix in code (adding cover.py) but also has some refactoring of the tests to remove redundant code.
In any case, no rush. Just wanted to know if it is waiting on anything on my end

Comment thread homeassistant/components/dynalite/cover.py Outdated
Comment thread tests/components/dynalite/test_const.py Outdated
Comment thread tests/components/dynalite/test_init.py Outdated
Comment thread homeassistant/components/dynalite/__init__.py
ziv1234 and others added 2 commits April 1, 2020 21:30
Co-Authored-By: Martin Hjelmare <marhje52@gmail.com>
Comment thread homeassistant/components/dynalite/__init__.py Outdated
Comment thread homeassistant/components/dynalite/const.py Outdated
Comment thread homeassistant/components/dynalite/const.py
Comment thread homeassistant/components/dynalite/const.py Outdated
Comment thread homeassistant/components/dynalite/convert_config.py Outdated
Comment thread homeassistant/components/dynalite/const.py Outdated
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Good!

@ziv1234
Copy link
Copy Markdown
Contributor Author

ziv1234 commented Apr 2, 2020

Thanks! For some reason there are tests failing in the CI that don't seem related (dyson component, the next one in alphabetical order...). Could this be related to the PR? They all pass in my venv

@MartinHjelmare
Copy link
Copy Markdown
Member

I don't see any dyson tests failing, just tts and they are expected currently.

@MartinHjelmare MartinHjelmare merged commit 39408ab into home-assistant:dev Apr 2, 2020
@ziv1234
Copy link
Copy Markdown
Contributor Author

ziv1234 commented Apr 2, 2020

thanks! Maybe i got mixed up with something. appreciate everything
wanted to ask you about something else that i wanted to add to dynalite, but not sure it follows the home assistant way (per your comments earlier) so thought i would check with you before submitting a PR
the structure of the dynalite system that has to be in configuration.yaml is almost always tightly aligned with the setup of the house. For example, every room will be an area, with presets for on/off and channels for the different lights. This means that the dynalite integration can also easily assign HA areas according to this and it will make the setup much easier since it was already done once. This can be done via the UI, but it involves a lot of manual work.
Is this something that makes sense to add? the code is very simple, but given your questions about the templates, here this is purely a convenience issue. I must say that when i did it home (custom component so far, until the covers are part of the normal release), it is very convenient for a large setup like mine.
Let me know. if interesting, it is a few lines of code that i can submit quickly

@MartinHjelmare
Copy link
Copy Markdown
Member

MartinHjelmare commented Apr 2, 2020

I'm not sure. Ask in the discord dev chat or architecture issue.

@ziv1234
Copy link
Copy Markdown
Contributor Author

ziv1234 commented Apr 2, 2020

thanks. I will make the PR at some point in any case, so it can be reviewed. already have the code so now just copy/paste...

@lock lock Bot locked and limited conversation to collaborators Apr 4, 2020
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