Add ability to select register type to Modbus Binary Sensor

[keton]

Breaking Change:

None.

Description:

Modbus protocol defines 2 types of binary inputs:

• coil (modbus function code 1)
• discrete inputs (modbus function code 2)

This commit adds ability to read data from discrete inputs using Modbus Binary Sensor.
It adds register_type configuration parameter with possible values of coil and discrete_input.
Default is coil so compatibility with previous behavior is kept.

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#10689

Example entry for configuration.yaml (if applicable):

binary_sensor:
  - platform: modbus
    coils:
      - name: BinSensor1
        hub: hub1
        slave: 1
        coil: 100
        register_type: discrete_input

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:

• Documentation added/updated in home-assistant.io

[homeassistant]

Hi @keton,

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!

[victoryNap]

Any movement on this PR?

@balloob can you take a look at it? Its pretty simple

[keton]

@lorddoskias Is there any chance this gets integrated? I have to deal with devices which need this.

[adamchengtkc]

Looks good to me

[vzahradnik]

Hi @keton, I made very similar PR to yours recently, #30004. I wasn't aware of it.
However, my change is different because it breaks the compatibility for the sake of clarity.

In the Modbus terminology, discrete inputs are not coils. Also, both of them are not registers. In my opinion, keeping a discrete input as a coil type is confusing. And the only benefit is to keep the backwards compatibility.

I renamed coils array to inputs and I also changed coil variable to address, which is more generic and matches with the Modbus terminology (it's used also in pymodbus API).

Now, we should discuss what's best. Breaking the compatibility, but having defined attributes in a clear, non-confusing way or keeping the compatibility.

Please let me know what you think would be better. Thanks!

[keton]

@vzahradnik In my book keeping backwards compatibility always wins.
I bet that there's somewhere already HA installation that uses coils parameter. Imagine what will happen if it auto updates and coils is not valid anymore. This was my exact reasoning when I was making the change.
I had a version of this PR that had all naming corrected and then started thinking about bigger picture. In the end I guess project maintainers should decide as they know the big picture.

If correcting naming is to win I'd keep backwards compatibility at least for a couple releases with clear deprecation warnings in the logs and UI. At least give people the chance to fix their config.

[vzahradnik]

@keton thanks for reply. I agree, the maintainers should decide. Either way, the support for discrete inputs will be there and that's what matters most.

[MartinHjelmare]

The change here and in #30004 look pretty simple. The only question is if we should make a breaking change to correct the naming of options or not.

[balloob]

I suggest we rename it for clarity, but keep supporting the old config but with a deprecation warning, telling people to upgrade.

[vzahradnik]

Hi @keton, do you plan to update your PR as suggested by @balloob? I'm asking to avoid duplicite work. If you have no objections, I could modify my PR #30004 to be backwards compatible.

Thanks!

[keton]

@vzahradnik fell free to go ahead with your PR #30004.

[vzahradnik]

@keton thanks! I will keep you updated.