Support for Multiple modbus hubs #19726

benvm · 2019-01-02T21:12:42Z

Description:

This PR implements support for multiple modbus hubs. It adds new configuration variables to distinguish between different hubs.

In the modbus section a new variable "name" is introduced. Sensors can then reference this name in a new "hub" variable. Both default to "default" when omitted so existing configuration should continue to work.

This is a continuation of #17901 with some alterations. The original author indicated that he didn't have time to finish it.

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

Example entry for `configuration.yaml` (if applicable):

modbus:
  - name: hub1
    type: tcp
    host: 192.168.0.1
    port: 502

  - name: hub2
    type: rtuovertcp
    host: 192.168.0.2
    port: 8887

sensor:
  - platform: modbus
    scan_interval: 5
    registers:
      - name: first_sensor
        hub: hub1
        register: 20
        data_type: uint
      - name: another_sensor
        hub: hub2
        slave: 1
        register: 2
        register_type: holding
        count: 1

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.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.io

janitza-bjag · 2019-01-16T05:57:03Z

very cool, waiting for this!

MartinHjelmare

Looks good! Some comments.

MartinHjelmare · 2019-01-31T10:03:28Z

homeassistant/components/binary_sensor/modbus.py

@@ -16,11 +16,13 @@
 _LOGGER = logging.getLogger(__name__)
 DEPENDENCIES = ['modbus']

+CONF_HUB = 'hub'


Maybe we should define this in the modbus component instead, and import it where needed.

Yes, that seems better. Done.

MartinHjelmare · 2019-01-31T10:04:10Z

homeassistant/components/binary_sensor/modbus.py

 CONF_COIL = 'coil'
 CONF_COILS = 'coils'

 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
    vol.Required(CONF_COILS): [{
+        vol.Required(CONF_HUB, default='default'): cv.string,


The default value should also be defined centrally.

Done. Defined DEFAULT_HUB.

MartinHjelmare · 2019-01-31T10:05:01Z

homeassistant/components/binary_sensor/modbus.py

@@ -7,7 +7,7 @@
 import logging
 import voluptuous as vol

-from homeassistant.components import modbus
+from homeassistant.components.modbus import DOMAIN


This platform belongs to the binary_sensor domain. Import the modbus domain as another name.

Changed for all platforms.

MartinHjelmare · 2019-01-31T10:06:18Z

homeassistant/components/modbus.py

-
+    DOMAIN:
+        vol.All(cv.ensure_list,
+                vol.Schema([vol.Any(SERIAL_SCHEMA, ETHERNET_SCHEMA)]))


Do we need schema definition here? Try without it.

I'm not entirely sure what you mean here. Do you want me to revert to what was there before?
That gives an 'Invalid config' when you actually define multiple hubs because a list isn't expected then.

No, I just think we don't need to add vol.Schema around the list of schemas. Have you tried without that?

Sorry, got confused. I removed it and that seems to work.

MartinHjelmare · 2019-01-31T10:10:53Z

homeassistant/components/modbus.py

        hass.bus.listen_once(EVENT_HOMEASSISTANT_STOP, stop_modbus)

        # Register services for modbus
        hass.services.register(
-            DOMAIN, SERVICE_WRITE_REGISTER, write_register,
+            DOMAIN,


Please don't change unrelated formatting.

homeassistant/components/switch/modbus.py

homeassistant/components/climate/modbus.py

MartinHjelmare

Great!

MartinHjelmare

Sorry, found some more corrections needed.

MartinHjelmare · 2019-02-01T07:06:50Z

homeassistant/components/sensor/modbus.py

@@ -40,6 +41,7 @@

 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
    vol.Required(CONF_REGISTERS): [{
+        vol.Required(CONF_HUB, DEFAULT_HUB): cv.string,


Please use keyword argument default for default value.

Since we set a default we can make this optional.

Made hub name optional.

MartinHjelmare · 2019-02-01T07:08:06Z

homeassistant/components/modbus.py

 ATTR_UNIT = 'unit'
 ATTR_VALUE = 'value'

 SERVICE_WRITE_REGISTER_SCHEMA = vol.Schema({
+    vol.Optional(ATTR_HUB, DEFAULT_HUB): cv.string,


Use default keyword argument for the default value. The second argument is message, so we have to specify with name what argument we're providing.

Good catch, I must have accidentally deleted those while I was adding DEFAULT_HUB.

MartinHjelmare · 2019-02-01T07:08:27Z

homeassistant/components/modbus.py

    vol.Required(ATTR_UNIT): cv.positive_int,
    vol.Required(ATTR_ADDRESS): cv.positive_int,
    vol.Required(ATTR_VALUE): vol.All(cv.ensure_list, [cv.positive_int])
 })

 SERVICE_WRITE_COIL_SCHEMA = vol.Schema({
+    vol.Optional(ATTR_HUB, DEFAULT_HUB): cv.string,


Same as above.

MartinHjelmare · 2019-02-01T07:10:37Z

homeassistant/components/binary_sensor/modbus.py

@@ -21,6 +22,7 @@

 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
    vol.Required(CONF_COILS): [{
+        vol.Required(CONF_HUB, default=DEFAULT_HUB): cv.string,


Make it optional.

MartinHjelmare · 2019-02-01T07:10:52Z

homeassistant/components/climate/flexit.py

 import homeassistant.helpers.config_validation as cv

 REQUIREMENTS = ['pyflexit==0.3']
 DEPENDENCIES = ['modbus']

 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
+    vol.Required(CONF_HUB, default=DEFAULT_HUB): cv.string,


MartinHjelmare · 2019-02-01T07:11:10Z

homeassistant/components/climate/modbus.py

@@ -35,6 +35,7 @@
 DATA_TYPE_FLOAT = 'float'

 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
+    vol.Required(CONF_HUB, default=DEFAULT_HUB): cv.string,


MartinHjelmare · 2019-02-01T07:11:48Z

homeassistant/components/switch/modbus.py

@@ -31,6 +33,7 @@
 REGISTER_TYPE_INPUT = 'input'

 REGISTERS_SCHEMA = vol.Schema({
+    vol.Required(CONF_HUB, default=DEFAULT_HUB): cv.string,


MartinHjelmare · 2019-02-01T07:12:06Z

homeassistant/components/switch/modbus.py

@@ -46,6 +49,7 @@
 })

 COILS_SCHEMA = vol.Schema({
+    vol.Required(CONF_HUB, default=DEFAULT_HUB): cv.string,


MartinHjelmare · 2019-02-01T20:21:28Z

Can be merged when build passes.

balloob · 2019-02-06T01:11:40Z

Can you merge upstream or rebase so we can merge this? Thanks.

…module level

…t now

benvm · 2019-02-06T16:07:23Z

Rebase done. Also one more small commit added, sorry.

I added a name property to ModbusHub. The platforms retrieve that to include in error messages.

MartinHjelmare · 2019-02-06T16:13:43Z

Can be merged when build passes.

homeassistant added cla-signed integration: modbus platform: binary_sensor.modbus labels Jan 2, 2019

ghost added the in progress label Jan 2, 2019

This was referenced Jan 2, 2019

[WIP]Support for Multi modbus hub #17901

Closed

Add support for multiple modbus hubs. home-assistant/home-assistant.io#8027

Merged

benvm changed the title ~~[WIP] Support for Multiple modbus hubs~~ Support for Multiple modbus hubs Jan 16, 2019

MartinHjelmare requested changes Jan 31, 2019

View reviewed changes

houndci-bot reviewed Jan 31, 2019

View reviewed changes

MartinHjelmare approved these changes Feb 1, 2019

View reviewed changes

MartinHjelmare requested changes Feb 1, 2019

View reviewed changes

MartinHjelmare approved these changes Feb 1, 2019

View reviewed changes

crhan and others added 11 commits February 6, 2019 15:51

modbus: support multiple modbus hub

e2d8165

update data after entities added

ac81d9e

pass hub object to each entity. and save hub to hass.data but not in …

e9016d5

…module level

add hub_client setup log

7b78534

don't update when adding device, because hub_client is not ready righ…

3a60643

…t now

support restore last state

88e559a

remove useless func

154ea7a

compatible with python35

e210578

removed unrelated style changes

e6ab38c

Update flexit for multi-device modbus

9557c5b

change how hubs are referenced in the configuration

6daf920

benvm added 6 commits February 6, 2019 15:55

Also update climate/modbus.py

1ab40a4

Remove unwanted whitescapce

c97eaa0

Defined common constants centrally

1f1f80b

Update DOMAIN in climate and switch components

906433d

Removed unnecessary vol.schema

73170d0

Make hub name optional

9bec1d1

benvm force-pushed the multi_modbus branch from 614448c to 9bec1d1 Compare February 6, 2019 15:12

Add name property to ModbusHub

60175a9

MartinHjelmare approved these changes Feb 6, 2019

View reviewed changes

cgarwood merged commit 861d58f into home-assistant:dev Feb 11, 2019

ghost removed the in progress label Feb 11, 2019

balloob mentioned this pull request Feb 20, 2019

0.88.0 #21238

Merged

ghost added integration: flexit and removed platform: binary_sensor.modbus labels Mar 21, 2019

wellsy57 mentioned this pull request Mar 21, 2019

Modbus binary sensors appear to be problematic #22213

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for Multiple modbus hubs #19726

Support for Multiple modbus hubs #19726

benvm commented Jan 2, 2019 •

edited

janitza-bjag commented Jan 16, 2019

MartinHjelmare left a comment

MartinHjelmare Jan 31, 2019

benvm Jan 31, 2019

MartinHjelmare Jan 31, 2019

benvm Jan 31, 2019

MartinHjelmare Jan 31, 2019

benvm Jan 31, 2019

MartinHjelmare Jan 31, 2019

benvm Jan 31, 2019

MartinHjelmare Jan 31, 2019

benvm Jan 31, 2019

MartinHjelmare Jan 31, 2019

MartinHjelmare left a comment

MartinHjelmare left a comment

MartinHjelmare Feb 1, 2019

MartinHjelmare Feb 1, 2019

benvm Feb 1, 2019

MartinHjelmare Feb 1, 2019 •

edited

benvm Feb 1, 2019

MartinHjelmare Feb 1, 2019

MartinHjelmare Feb 1, 2019

MartinHjelmare Feb 1, 2019

MartinHjelmare Feb 1, 2019

MartinHjelmare Feb 1, 2019

MartinHjelmare Feb 1, 2019

MartinHjelmare commented Feb 1, 2019

balloob commented Feb 6, 2019

benvm commented Feb 6, 2019

MartinHjelmare commented Feb 6, 2019

Support for Multiple modbus hubs #19726

Support for Multiple modbus hubs #19726

Conversation

benvm commented Jan 2, 2019 • edited

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

janitza-bjag commented Jan 16, 2019

MartinHjelmare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare left a comment

Choose a reason for hiding this comment

MartinHjelmare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare Feb 1, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare commented Feb 1, 2019

balloob commented Feb 6, 2019

benvm commented Feb 6, 2019

MartinHjelmare commented Feb 6, 2019

benvm commented Jan 2, 2019 •

edited

Example entry for `configuration.yaml` (if applicable):

MartinHjelmare Feb 1, 2019 •

edited