search

openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.85k stars 3.56k forks source link

[modbus] openHAB 4.0 refactoring: turn things into channels similar to mqtt.generic #14058

Open ssalonen opened 1 year ago

ssalonen commented 1 year ago

Prototyping in

https://github.com/ssalonen/openhab-core/tree/modbus-14058 https://github.com/ssalonen/openhab2-addons/tree/modbus-14058 README

openHAB ~4.0~ x.0 (30.5.2023: this is not going to make openHAB 4.0, we would need much more testing and user feedback) is in the works and this is the right time to consider breaking changes. I create this issue to bring visibility and open up discussion. cc @Rossko57 as FYI

Background

Modbus binding was one of the first bindings with complex thing hierarchy...and that shows. At the time, I decided to go for the following thing hierarchy

Read/write data things:

endpoint (tcp or serial)
-> poller (reading block of data, every x milliseconds)
---> data (referring to one piece of data from poller, optionally how to write data)

Write-only data things:

endpoint (tcp or serial)
--> data (how to write data)

This setup introduces lot of things which is cumbersome to edit in MainUI. For example, MainUI YAML view is optimized to edit single thing, not multiple things. Often with Modbus, many of the data things are very similar, just the "address" is different. It would be much more convenient to edit the channels of a single thing in UI.

There's other negative downsides with several things, for example a lot of thing status spamming in logs when poller goes OFFLINE.

Proposal (WIP)

We can take example of mqtt.generic binding where one can configure channels.

The exact same approach is applicable to modbus. Instead of data things, we could have configurable channels for poller (read/write behaviour) and tcp/serial (write-only behaviour). Channels could be specialized for the item type, allowing ease of use, for example

Switch channel could have on_value and off_value to encode/decode raw bit pattern from/to modbus (similar to mqtt). update: this is implemented in proto, as readIntoOnOff channel

At the same time, we probably should look into initiliazation logic, starting with UNKNOWN tcp/serial/poller until we have a successful read/write (update: this is implemented in the proto). Currently endpoint thing starts with ONLINE, even when no single TCP packet has been communicated with the device

read channels

https://github.com/ssalonen/openhab2-addons/tree/modbus-14058/bundles/org.openhab.binding.modbus#read-channels

Write channels:

[Note: writing "json" would not be supported anymore. One would have thing actions for endpoint thing instead.

https://github.com/ssalonen/openhab2-addons/tree/modbus-14058/bundles/org.openhab.binding.modbus#write-channels

Notes from prototyping

Example of read and write channel linked to same item: https://github.com/ssalonen/openhab2-addons/tree/modbus-14058/bundles/org.openhab.binding.modbus#valve-example-writing-to-different-address-and-type-than-read

Example of one read channel and two write channels linked to Dimmer item: https://github.com/ssalonen/openhab2-addons/tree/modbus-14058/bundles/org.openhab.binding.modbus#dimmer-example

Downside: This approach might be hard to understand for users as this is not a model used by other bindings. I have also noted that due to https://github.com/openhab/openhab-webui/issues/1478 linking via UI is hard and clunky. Upside: However, with this separation-of-concerns, separating e.g. writing of ON/OFF from writing of PercentType with different channels, neatly divides the configuration task into "orthogonal" sub-problems.

Some considerations

  1. Many modbus users might have tens if not hundreds of data things. We probably should consider if it is possible to create a tool to assist with migration? This might get complicated though. Can we use core migration capability?
  2. We need to ensure no breakages with modbux.xxx device bindings. I think this is not an issue, they do not rely on poller and data things.
  3. Data things had channels for erroring/successfull read/write. Emulate same by propagating channel write errors to poller (if parent for channel) and endpoint (always) thing status? Badly configured read channels should make poller go offline
  4. Can we provide suitable ready-to-use transformations with the binding distribution? Eg number-to-switch and vice versa? Similar how persistence addons bring their config https://github.com/openhab/openhab-addons/blob/4d6d6443ef38a5799e8913e9f04edbbf245c7f06/bundles/org.openhab.persistence.dynamodb/src/main/feature/feature.xml#L9
lsiepel commented 1 year ago

As this refactoring mainly focuses on thing structure if i understand it well, i'm wondering if these two enahncements are also considered? https://github.com/openhab/openhab-addons/issues/9576 https://github.com/openhab/openhab-addons/issues/13247

ssalonen commented 1 year ago

Yes, returning raw data as hex string or RawType is alreeady in the plans

ssalonen commented 1 year ago

UI Prototype

image image

openhab-bot commented 1 year ago

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/is-there-need-for-different-channel-types-number-dimmer-switch-nowadays/142992/1

openhab-bot commented 1 year ago

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/modubus-binding-fails-by-writevaluetype-uint32-swap/143873/5

ssalonen commented 1 year ago

This is not going to make OH 4.0, for sure. I have had not have time to work on this during the last months, really.

Changes like this require more community feedback, and perhaps even guidance on the thing/channel design.

I have cleaned up the the initial description, documenting the current state of the prototype.

J-N-K commented 1 year ago

Please also have a look at https://github.com/openhab/openhab-core/pull/3355

ssalonen commented 7 months ago

I am thinking of working around https://github.com/openhab/openhab-webui/issues/1478 by introducing multiple channel types, one for each Item type (yikes!)

Example: image

I probably need to introduce even more channel types, for example "Interpret read data as percent (0-100%)" for all item types. One could for example, first scale the raw modbus data to % and then transform it to Switch (ON/OFF) using channel transform.

@ghys @Rossko57 this is exploding pretty rapidly... Would it be ok to simply remove the item type filtering when linking channels? https://github.com/openhab/openhab-webui/issues/1478#issuecomment-1437361133

ssalonen commented 7 months ago

I guess one alternative to making the linking more "flexible" in UI would be to change the channel structure more to the original binding, channel names mapping to the item names

The functionality of interpreting the binary data to openHAB data types (e.g. converting to ON/OFF, converting to PercentType) from modbus would be facilitated by transformations provided by the binding. Transformations could be combined with ∩.

This is almost analogous to the original binding channels (see https://www.openhab.org/addons/bindings/modbus/#data-thing), but with read and writes separated, as well as coil writes and register writes separated. Also, I would like to have the configuration at the channel level, no need to introduce additional things (the data thing in the original binding). It should be possible to instantiate multiple channels of same channel type, but just with different configuration.

This separation allows a bit cleaner UI, not showing irrelevant options that resulted in channel configuration error in the original binding (see the complexity of the validation logic here: https://github.com/openhab/openhab-addons/blob/a31b1578be628f0de10171167cf66d91cb4953ff/bundles/org.openhab.binding.modbus/src/main/java/org/openhab/binding/modbus/internal/handler/ModbusDataThingHandler.java#L500-L784). For example,