Skip to content

Rename domain import in modbus#33906

Merged
springstan merged 2 commits intohome-assistant:devfrom
janiversen:modbus_issues4
Apr 9, 2020
Merged

Rename domain import in modbus#33906
springstan merged 2 commits intohome-assistant:devfrom
janiversen:modbus_issues4

Conversation

@janiversen
Copy link
Copy Markdown
Member

@janiversen janiversen commented Apr 9, 2020 •
edited
Loading

Breaking change

Proposed change

changed MODBUS_DOMAIN to DOMAIN,
the constant need to be named DOMAIN because homeassistant/config.py check for that name

This was identified when using packages and a double hub configuration.

Type of change

  • Dependency upgrade
  • [x ] 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

Additional information

A special thank to polcape, for helping pinning this bug down.

Checklist

  • [x ] The code change is tested and works locally.
  • [ x] Local tests pass. Your PR cannot be merged unless tests pass
  • [x ] There is no commented out code in this PR.
  • [ x] I have followed the development checklist
  • [x ] 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

@dshokouhi dshokouhi added this to the 0.108.2 milestone Apr 9, 2020
@janiversen
fix #33873
7bb5132 
changed MODBUS_DOMAIN to DOMAIN, the constant need to be named
DOMAIN because homeassistant/config.py check for that name.

Remove timeout from serial client.
springstan
springstan approved these changes Apr 9, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@springstan springstan left a comment

Choose a reason for hiding this comment

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

Looks good, just one comment.

Comment thread homeassistant/components/modbus/__init__.py
@springstan springstan changed the title modbus fix issue #33873 Rename domain import in modbus Apr 9, 2020
springstan
springstan reviewed Apr 9, 2020
View reviewed changes
Comment thread tests/components/modbus/conftest.py Outdated
@janiversen
review 1
23c3d2e 
updated MODBUS_DOMAIN - DOMAIN
@springstan springstan merged commit 9535dd8 into home-assistant:dev Apr 9, 2020
balloob pushed a commit that referenced this pull request Apr 10, 2020
@janiversen @balloob
Rename domain import in modbus (#33906)
54bf838
@balloob balloob mentioned this pull request Apr 10, 2020
Merged
@janiversen janiversen deleted the modbus_issues4 branch April 10, 2020 08:02
@lock lock Bot locked and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ludeeus ludeeus ludeeus left review comments

@springstan springstan springstan approved these changes

Assignees

No one assigned

Labels

cherry-picked cla-signed has-tests integration: modbus small-pr PRs with less than 30 lines.

Projects

None yet

Milestone

0.108.2

Development

Successfully merging this pull request may close these issues.

Modbus crashed on 0.108.1 with pakcages

6 participants

@janiversen @ludeeus @springstan @balloob @dshokouhi @homeassistant