Fix Modbus review comments

[janiversen]

Breaking change

Proposed change

Updates for review comments from @MartinHjelmare, which was done after the code was merged.

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)
• [x ] Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml

modbus:
  - name: plc
    type: tcp
    host: 127.0.0.1
    port: 5020

binary_sensor:
  - platform: modbus
    scan_interval: 1
    inputs:
      - name: plc_aaa
        hub: plc
        slave: 1
        address: 0
        input_type: discrete_input

Remark depend a modbus tcp server.

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request:

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:

• Documentation added/updated for www.home-assistant.io

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

[probot-home-assistant]

Hey there @adamchengtkc, mind taking a look at this pull request as its been labeled with a integration (modbus) you are listed as a codeowner for? Thanks!

[janiversen]

@MartinHjelmare would you please review the code, and ensure it solves your review comments, thanks in advance.

[MartinHjelmare]

Looks good! I still think we can drop the global:
#33447 (comment)

Is there something I'm missing about that?

[janiversen]

No you are not missing something, I was.

I did the change, but then ran into some a pre-commit problem, and got side tracked by helping Frenck.

It is quite interesting that there are a scope difference between variables in a class and a function, I was not aware of that.

[MartinHjelmare]

Nice! Just a small convention comment.

[janiversen]

Changed, and I also rebased on latest dev, so it is hopefully ready to get merged.

[MartinHjelmare]

Great!

[janiversen]

this:
2020-04-07T07:57:49.7295319Z /__w/_temp/c2caee9c-40af-42c8-b623-1fd58fc3d1b3.sh: line 5: codecovToken: command not found
2020-04-07T07:57:49.7296032Z usage: codecov [-h] [--version] [--token TOKEN] [--file [FILE [FILE ...]]]
2020-04-07T07:57:49.7296885Z [--flags [FLAGS [FLAGS ...]]] [--env [ENV [ENV ...]]]
2020-04-07T07:57:49.7297522Z [--required] [--name NAME] [--gcov-root GCOV_ROOT]
2020-04-07T07:57:49.7298372Z [--gcov-glob [GCOV_GLOB [GCOV_GLOB ...]]]
2020-04-07T07:57:49.7299006Z [--gcov-exec GCOV_EXEC] [--gcov-args GCOV_ARGS]
2020-04-07T07:57:49.7299879Z [-X [DISABLE [DISABLE ...]]] [--root ROOT] [--commit COMMIT]
2020-04-07T07:57:49.7300599Z [--prefix PREFIX] [--branch BRANCH] [--build BUILD] [--pr PR]
2020-04-07T07:57:49.7302004Z [--tag TAG] [--slug SLUG] [--url URL] [--cacert CACERT]
2020-04-07T07:57:49.7302960Z [--dump] [-v] [--no-color]
2020-04-07T07:57:49.7303937Z codecov: error: argument --token/-t: expected one argument

can hardly by something I introduced, trying to force push again.

[MartinHjelmare]

Yeah, we've tried to fix this this morning. Please rebase on latest dev branch and see if it works.

[janiversen]

Done, rebase with dev as of this minute. And force push, lets how if it works

[janiversen]

same problem! can I help to pinpoint it?

[codecov]

Codecov Report

Merging #33755 into dev will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev   #33755   +/-   ##
=======================================
  Coverage   94.75%   94.75%           
=======================================
  Files         858      858           
  Lines       60797    60797           
=======================================
  Hits        57609    57609           
  Misses       3188     3188           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb98d62...702f38c. Read the comment docs.

[janiversen]

All green....yeah !!!!! Now just waiting for somebody to push the merge bottom :-)