Modbus wont verify switch status when write_coil/read_coils is used

gribouk commented 3 years ago

The problem

I have a 4 gang switch module which uses 05 - write single coil command to change channel status, but for reading status of that channel it lets me use only 01- read coils command with 8 coils to read, starting address 0 - first channel.

I was proposed to use new feature write_coil/read_coils to verify switch status which just turned available with 2021.8.0 release, which I did.

The problem is, that switch stopped reading it's status. When I try to change it's state, it's status falls back to the initial one. I can assign a status to a channel using ModbusPoll (for test purposes to se if change of coil data leads to change in status of a switch) and switch status does not change either...

What is version of Home Assistant Core has the issue?

2021.8.0

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

MODBUS

Link to integration documentation on our website

https://www.home-assistant.io/integrations/modbus/

Example YAML snippet

- name: solar_relay_1
        slave: 255
        address: 0
        write_type: coil
        verify:
            input_type: coils
 #           address: 0
 #           state_on: 1
 #           state_off: 0


### Anything in the logs that might be useful for us?

```txt
2021-08-04 14:35:56 WARNING (MainThread) [homeassistant.components.modbus.validators] Modbus hub1 timeout(10) is adjusted(9) due to scan_interval
2021-08-04 14:35:56 WARNING (MainThread) [homeassistant.components.modbus.validators] Sensor1 with int is not valid, trying to convert
2021-08-04 14:35:56 WARNING (MainThread) [homeassistant.components.modbus.validators] Sensor2 with int is not valid, trying to convert
2021-08-04 14:35:56 WARNING (MainThread) [homeassistant.components.modbus.validators] Sensor1 with int is not valid, trying to convert
2021-08-04 14:35:56 WARNING (MainThread) [homeassistant.components.modbus.validators] Sensor2 with int is not valid, trying to convert
2021-08-04 14:35:56 DEBUG (SyncWorker_1) [pymodbus.client.sync] Connection to Modbus server established. Socket ('192.168.1.240', 49307)
2021-08-04 14:36:07 DEBUG (SyncWorker_3) [pymodbus.client.sync] New Transaction state 'SENDING'
2021-08-04 14:36:07 DEBUG (SyncWorker_1) [pymodbus.client.sync] New Transaction state 'SENDING'
2021-08-04 14:36:17 ERROR (MainThread) [homeassistant] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
File "/usr/src/homeassistant/homeassistant/components/modbus/base_platform.py", line 246, in async_update
result = await self._hub.async_pymodbus_call(
File "/usr/src/homeassistant/homeassistant/components/modbus/modbus.py", line 322, in async_pymodbus_call
result = await self.hass.async_add_executor_job(
File "/usr/local/lib/python3.9/concurrent/futures/thread.py", line 52, in run
result = self.fn(*self.args, **self.kwargs)
File "/usr/src/homeassistant/homeassistant/components/modbus/modbus.py", line 305, in _pymodbus_call
result = self._pb_call[use_call][ENTRY_FUNC](address, value, **kwargs)
KeyError: 'coils'
2021-08-04 14:36:17 DEBUG (SyncWorker_3) [pymodbus.client.sync] New Transaction state 'SENDING'

Additional information

No response

ReneHezser commented 3 years ago

Same here. Getting data from my SMA Power Inverter is not working anymore. `

type: tcp host: 192.168.x.x port: 502 name: sma sensors:
- name: Betriebszustand scan_interval: 60 slave: 3 address: 40029 count: 2 data_type: uint `

gribouk commented 3 years ago

Same here. Getting data from my SMA Power Inverter is not working anymore. `

type: tcp host: 192.168.x.x port: 502 name: sma sensors:

name: Betriebszustand scan_interval: 60 slave: 3 address: 40029 count: 2 data_type: uint `

Are you sure it worked before? The address is somewhat big, it fiels like, since you are reading holding registry by default, you should try address: 28 for the reason holding registers are dedicated addresses from 40001 to 49999... (address 40001 has number 0 in holding registry universe). May be this is the reason?

ReneHezser commented 3 years ago

Yes, the configuration has been working a while with multiple HA versions. Edit: With the fix release, the integration is working again.

probot-home-assistant[bot] commented 3 years ago

modbus documentation modbus source _{^{(message by IssueLinks)}}

probot-home-assistant[bot] commented 3 years ago

Hey there @adamchengtkc, @vzahradnik, mind taking a look at this issue as it has been labeled with an integration (modbus) you are listed as a code owner for? Thanks! _{^{(message by CodeOwnersMention)}}

janiversen commented 3 years ago

Please check with newest 2021.8.x is is solved and are released.

gribouk commented 3 years ago

No, with 2021.8.6 no success

2021-08-14 21:46:08 ERROR (MainThread) [homeassistant] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
File "/usr/src/homeassistant/homeassistant/components/modbus/base_platform.py", line 250, in async_update
result = await self._hub.async_pymodbus_call(
File "/usr/src/homeassistant/homeassistant/components/modbus/modbus.py", line 329, in async_pymodbus_call
result = await self.hass.async_add_executor_job(
File "/usr/local/lib/python3.9/concurrent/futures/thread.py", line 52, in run
result = self.fn(*self.args, **self.kwargs)
File "/usr/src/homeassistant/homeassistant/components/modbus/modbus.py", line 312, in _pymodbus_call
result = self._pb_call[use_call][ENTRY_FUNC](address, value, **kwargs)
KeyError: 'coils'
2021-08-14 21:46:09 DEBUG (SyncWorker_4) [pymodbus.client.sync] New Transaction state 'SENDING'

Frankly I do not understand what option 'coils' does since I cannot map it on MODBUS commands (how many coils? how to interpret the result?)

As I wrote, the solution clear to me is right at your path. For sensor platform you have already done the reading of several holding registers and input registers - commands 03 and 02 respectivelly. For those you've implemented reading of multiple addresses and their interpretation using python struc. Why not extend same thing to command 01 for reading coils? Thus It would be possible to make a templete switch with modbus sensor for state reading and modbus switch for state setting... Am I missing something?

janiversen commented 3 years ago

If you do not understand “coils” then you have not read the modbus specification: https://modbus.org/docs/Modbus_Application_Protocol_V1_1b3.pdf

coil is command code 01 used with count 1 (because a switch/binary_sensor/light/fan only are 1 sensor with the value ON/OFF). For write you can write a single bit (coil) or multiple (coils), and if you specify "coils" it is used with count=1.

I frankly do not see what this have to do with the issue reported, if you want a new feature gather support for it in the community first.

The error you see, mean you use "coils" in your configuration, but you have not supplied your updated configuration which must have changed from "sensor" to "switch/fan/light" since "coils" can only be used as "write_type: coils" in those.

gribouk commented 3 years ago

if you do not understand “coils” then you have not read the modbus specification:

"6 Function codes descriptions 6.1 01 (0x01) Read Coils This function code is used to read from 1 to 2000 contiguous status of coils in a remote device. The Request PDU specifies the starting address, i.e. the address of the first coil specified, and the number of coils"

Src: Page.11 https://modbus.org/docs/Modbus_Application_Protocol_V1_1b3.pdf

Not to mention I've referenced that source in one of my previous issues, I do not see any difference or contradictions to my previous statements.

I understand what MODBUS "Read Coils" command 01 does, and what coil is in MODBUS “universe” - I did not understand what option "coils" does in a switch configuration verify section.

For write you can write a single bit (coil) or multiple (coils)...

Writing is a different story because these implemented with functions 05 and 15 for coil/coils, and 06 and 16 for holding register/registers. But I do not remember I've ever raised questions about write commands, haven’t I? The issue is about command 01 - READ. And yes - for READ 1 coil and MANY coils - it is same command - 01 - Read Coils... (see ref. you’ve provided)

coils is command code 01 used with count 1

Thus I am left guessing what “coil” option would mean in your notation for a switch configuration verify section, though you’ve edited it already and now it looks like this “coil is command code 01 used with count 1…” which is even more confusing…

I frankly do not see what this have to do with the issue reported, if you want a new feature gather support for it in the community first.

Quote: "I have a 4 gang switch module ... but for reading status of that channel it lets me use only 01- read coils command with 8 coils to read, starting address 0 - first channel." And that is how these are related… I can read only 8 coils from device, not 1. If I read one – device returns error… So, it is command 01, but for 8 coils… If you say there is a tool to read 8 coils and extract the value for N-th byte from a result with a single option “coils” in verify section – great it is! I would like to use it!

P.S. General Note. Ever in my theoretical physics background I’ve applied general rule to everything I used to do – never use anything you do not understand the origin of and verified your self. I’ve re-derived nearly everything I was using in my projects, and there was a lot of heavy stuff to “break some teeth” with. Hence, it is not a crime to ask questions if you do no see how something is supposed to solve your problem – maybe it is not a solution to your problem after all ( Like SUSY models were buried with LHC data, though it was obvious to many, not me only, this thing has no room to exist in reality… )

janiversen commented 3 years ago

Whatever….you forget 2 important things:

your original problem, with your original config, have been solved for a while
in the following messages you seem to question a number of different issues.

Your original config is wrong because you as for “coils” as input type.

May I politely remind you this channel is for development issues, not general support. If you want to understand why “coil” and count: 1 makes sense, please take a look at the code, instead if making theoretical assumptions what it mean. If you have general issues as to “why” in order for you to understand the integration, please redirect those to the community forum.

My goal here is to solve real problems with the integration, not participating in discussions on whether “coil” or “coils” are correct.

If you still have a problem with the integration, please start from a fresh and describe it again.

janiversen commented 3 years ago

As for the quote “ but for reading status of that channel it lets me use only 01- read coils command with 8 coils to read, starting address 0 - first channel.” That works out of the box without specifying input_type or doing “input_type: 1”, your device might deliver 8 bits (and typically will) but you cannot set “count: 8” since it does not make sense. We use the first bit and only the first bit

janiversen commented 3 years ago

Pr #54645 correct the use of “input_type: coils” by silently converting it.

home-assistant / core