Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add modbus sensor string data_type #35269

Merged
merged 10 commits into from May 8, 2020
Merged

Add modbus sensor string data_type #35269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e5acafe
modbus sensor: string data_type
bradkeifer May 5, 2020
c20abd9
Merge pull request #1 from bradkeifer/modbus-sensor-string
bradkeifer May 5, 2020
7cc28de
Add modbus sensor string data_type #35269
bradkeifer May 6, 2020
14b969c
Delete const.py
bradkeifer May 6, 2020
cf924f5
Delete sensor.py
bradkeifer May 6, 2020
1f69147
Add files via upload
bradkeifer May 7, 2020
60d5169
modbus sensor - add string support. Upload from local dev environment
bradkeifer May 7, 2020
01a648b
Merge branch 'dev' of https://github.com/bradkeifer/core into dev
bradkeifer May 7, 2020
be9fa72
fix executable stauts
bradkeifer May 7, 2020
4a38b09
fix flake8 failure
bradkeifer May 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions homeassistant/components/modbus/const.py
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@
DATA_TYPE_FLOAT = "float"
DATA_TYPE_INT = "int"
DATA_TYPE_UINT = "uint"
DATA_TYPE_STRING = "string"

# call types
CALL_TYPE_COIL = "coil"
Expand Down
33 changes: 24 additions & 9 deletions homeassistant/components/modbus/sensor.py
View file
Open in desktop
Expand Up @@ -34,6 +34,7 @@
DATA_TYPE_CUSTOM,
DATA_TYPE_FLOAT,
DATA_TYPE_INT,
DATA_TYPE_STRING,
DATA_TYPE_UINT,
DEFAULT_HUB,
MODBUS_DOMAIN,
Expand Down Expand Up @@ -69,7 +70,13 @@ def number(value: Any) -> Union[int, float]:
vol.Required(CONF_REGISTER): cv.positive_int,
vol.Optional(CONF_COUNT, default=1): cv.positive_int,
vol.Optional(CONF_DATA_TYPE, default=DATA_TYPE_INT): vol.In(
[DATA_TYPE_INT, DATA_TYPE_UINT, DATA_TYPE_FLOAT, DATA_TYPE_CUSTOM]
[
DATA_TYPE_INT,
DATA_TYPE_UINT,
DATA_TYPE_FLOAT,
DATA_TYPE_STRING,
DATA_TYPE_CUSTOM,
]
),
vol.Optional(CONF_DEVICE_CLASS): DEVICE_CLASSES_SCHEMA,
vol.Optional(CONF_HUB, default=DEFAULT_HUB): cv.string,
Expand Down Expand Up @@ -98,7 +105,9 @@ def setup_platform(hass, config, add_entities, discovery_info=None):

for register in config[CONF_REGISTERS]:
structure = ">i"
if register[CONF_DATA_TYPE] != DATA_TYPE_CUSTOM:
if register[CONF_DATA_TYPE] == DATA_TYPE_STRING:
structure = str(register[CONF_COUNT] * 2) + "s"
elif register[CONF_DATA_TYPE] != DATA_TYPE_CUSTOM:
try:
structure = (
f">{data_types[register[CONF_DATA_TYPE]][register[CONF_COUNT]]}"
Expand Down Expand Up @@ -142,6 +151,7 @@ def setup_platform(hass, config, add_entities, discovery_info=None):
register[CONF_OFFSET],
structure,
register[CONF_PRECISION],
register[CONF_DATA_TYPE],
register.get(CONF_DEVICE_CLASS),
)
)
Expand All @@ -168,6 +178,7 @@ def __init__(
offset,
structure,
precision,
data_type,
device_class,
):
"""Initialize the modbus register sensor."""
Expand All @@ -183,6 +194,7 @@ def __init__(
self._offset = offset
self._precision = precision
self._structure = structure
self._data_type = data_type
self._device_class = device_class
self._value = None
self._available = True
Expand Down Expand Up @@ -243,13 +255,16 @@ def update(self):
registers.reverse()

byte_string = b"".join([x.to_bytes(2, byteorder="big") for x in registers])
val = struct.unpack(self._structure, byte_string)[0]
val = self._scale * val + self._offset
if isinstance(val, int):
self._value = str(val)
if self._precision > 0:
self._value += "." + "0" * self._precision
if self._data_type != DATA_TYPE_STRING:
Copy link
Member

@janiversen janiversen May 6, 2020 •
edited

Choose a reason for hiding this comment

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

This seems complicated especially since “int” is the most common form.

Now that we have data_type it should be used instead of isinstance() please.

Sorry for the comment edits, I managed to confuse myself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @janiversen - Thanks for your prompt review. Is there anything you would like me to action at the moment?

Copy link
Member

Choose a reason for hiding this comment

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

We can do the change of isinstance() but it would be real nice to have test cases for the string type. See core/tests/component/modbus

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds great to me. I am new to python, HA and GitHub, so I might make some mistakes along the way. I'll read up about test cases and give it a go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a commit for a test of the new string data_type

val = struct.unpack(self._structure, byte_string)[0]
val = self._scale * val + self._offset
if isinstance(val, int):
self._value = str(val)
if self._precision > 0:
self._value += "." + "0" * self._precision
else:
self._value = f"{val:.{self._precision}f}"
else:
self._value = f"{val:.{self._precision}f}"
self._value = byte_string.decode()

self._available = True
21 changes: 21 additions & 0 deletions tests/components/modbus/test_modbus_sensor.py
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@
CONF_SCALE,
DATA_TYPE_FLOAT,
DATA_TYPE_INT,
DATA_TYPE_STRING,
DATA_TYPE_UINT,
)
from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN
Expand Down Expand Up @@ -357,3 +358,23 @@ async def test_float_data_type(hass, mock_hub):
register_words=[16286, 1617],
expected="1.23457",
)


async def test_string_data_type(hass, mock_hub):
"""Test byte string register data type."""
register_config = {
CONF_COUNT: 8,
CONF_REGISTER_TYPE: CALL_TYPE_REGISTER_HOLDING,
CONF_DATA_TYPE: DATA_TYPE_STRING,
CONF_SCALE: 1,
CONF_OFFSET: 0,
CONF_PRECISION: 0,
}
await run_test(
hass,
mock_hub,
register_config,
SENSOR_DOMAIN,
register_words=[0x3037, 0x2D30, 0x352D, 0x3230, 0x3230, 0x2031, 0x343A, 0x3335],
expected="07-05-2020 14:35",
)