openhab / openhab1-addons

Add-ons for openHAB 1.x
Eclipse Public License 2.0
3.43k stars 1.71k forks source link

[expire] expired binding commands to group item #5751

Closed Rossko57 closed 3 years ago

Rossko57 commented 5 years ago

OH 2.4.0 stable

Expire binding does not configure for groups, rejects command. It is legitimate to send commands to Group Items (although not updates) A use case here would be to timeout a group of light switches, for example.

Sample Item Group gTestingX "test group [%s]" {expire="30s,command=ON"} Item with type specified Group:Switch gTestingS "test group [%s]" {expire="30s,command=ON"} openhab.log 2019-01-02 23:25:58.911 [INFO ] [el.core.internal.ModelRepositoryImpl] - Refreshing model 'doors.items' 2019-01-02 23:25:58.957 [ERROR] [el.item.internal.GenericItemProvider] - Binding configuration of type 'expire' of item 'gTestingX' could not be parsed correctly. org.eclipse.smarthome.model.item.BindingConfigParseException: The string 'ON' does not represent a valid command for item gTestingX at org.openhab.core.binding.internal.BindingConfigReaderDelegate.processBindingConfiguration(BindingConfigReaderDelegate.java:51) ~[?:?] at org.eclipse.smarthome.model.item.internal.GenericItemProvider.internalDispatchBindings(GenericItemProvider.java:397) ~[?:?] at org.eclipse.smarthome.model.item.internal.GenericItemProvider.internalDispatchBindings(GenericItemProvider.java:366) ~[?:?] at org.eclipse.smarthome.model.item.internal.GenericItemProvider.processBindingConfigsFromModel(GenericItemProvider.java:229) ~[?:?] at org.eclipse.smarthome.model.item.internal.GenericItemProvider.modelChanged(GenericItemProvider.java:432) ~[?:?] at org.eclipse.smarthome.model.core.internal.ModelRepositoryImpl.notifyListeners(ModelRepositoryImpl.java:301) ~[?:?] at org.eclipse.smarthome.model.core.internal.ModelRepositoryImpl.addOrRefreshModel(ModelRepositoryImpl.java:139) ~[?:?] at org.eclipse.smarthome.model.core.internal.folder.FolderObserver.checkFile(FolderObserver.java:227) ~[?:?] at org.eclipse.smarthome.model.core.internal.folder.FolderObserver.processWatchEvent(FolderObserver.java:291) ~[?:?] at org.eclipse.smarthome.core.service.WatchQueueReader.lambda$3(WatchQueueReader.java:323) ~[?:?] at java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source) [?:?] at java.util.concurrent.FutureTask.run(Unknown Source) [?:?] at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(Unknown Source) [?:?] at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(Unknown Source) [?:?] at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) [?:?] at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) [?:?] at java.lang.Thread.run(Unknown Source) [?:?] Caused by: org.openhab.model.item.binding.BindingConfigParseException: The string 'ON' does not represent a valid command for item gTestingX at org.openhab.binding.expire.internal.ExpireGenericBindingProvider$ExpireBindingConfig.(ExpireGenericBindingProvider.java:172) ~[?:?] at org.openhab.binding.expire.internal.ExpireGenericBindingProvider.processBindingConfiguration(ExpireGenericBindingProvider.java:60) ~[?:?] at org.openhab.core.binding.internal.BindingConfigReaderDelegate.processBindingConfiguration(BindingConfigReaderDelegate.java:49) ~[?:?] ... 16 more

2019-01-02 23:25:58.985 [ERROR] [el.item.internal.GenericItemProvider] - Binding configuration of type 'expire' of item 'gTestingS' could not be parsed correctly. org.eclipse.smarthome.model.item.BindingConfigParseException: The string 'ON' does not represent a valid command for item gTestingS ... same error pattern snipped ...

9037568 commented 5 years ago

It appears this may be outside of the binding's control.

Do you have a working Group object that accepts ON commands? Please show the definition and the definitions of the member items.

Please also show the definition of the member items for the failing group.

Rossko57 commented 5 years ago

I have no idea if this is about the way expire binding validates parameters or the way some deeper part of OH/ESH handles them.

Sending commands to a Group is documented since OH1. https://www.openhab.org/docs/configuration/items.html#groups The expected behaviour is that commands sent to a Group item will be passed along to Group members, the state of the group itself unaffected (except later, if/when members states may affect the derived aggregated state).

Demonstration of expected command handling, xxx.items

Group testGroup "testing"
Switch testSwitch  "testing" (testGroup)

events.log when executing testGroup.sendCommand(ON)

2019-01-05 00:47:51.057 [ome.event.ItemCommandEvent] - Item 'testGroup' received command ON
2019-01-05 00:47:51.058 [ome.event.ItemCommandEvent] - Item 'testSwitch' received command ON
2019-01-05 00:47:51.067 [vent.ItemStateChangedEvent] - testSwitch changed from NULL to ON

So Groups can be sent commands.

The expectation is that it would be legitimate to use the expire binding with a Group item, albeit for commands only. (state updates might reasonably be rejected or generate an error)

The current position is that attempts to configure the expire binding on a Group item fail, seemingly not because it is impermissable, but because something thinks the selected command is unsuitable.

Group testGroup "testing" {expire="30s,command=OFF"}

Result upon loading xxx.items file

Binding configuration of type 'expire' of item 'testGroup' could not be parsed correctly.
org.eclipse.smarthome.model.item.BindingConfigParseException: The string 'ON' does not represent a valid command for item testGroup

The Group can be provided with a type, which might make validating permitted commands work better (even though the system does not require this, as already demonstrated)

Group:Switch testGroup "testing" {expire="30s,command=OFF"}

but the same validation error occurs.

The correct formation of the example binding configuration can be demonstrated by Switch testSwitch "testing" (testGroup) {expire="30s,command=OFF"} which successfully loads, and works as expected.

Rossko57 commented 5 years ago

Just as an afterthought, as the expire binding also involves the state of the Item it is bound to - we should also test assigning an aggregate function to the Group, which will ensure that is has a state. (for example, in a rule you can examine testGroup.state and find ON or NULL etc., like any other Item)

Group:Switch:OR(ON.OFF) testGroup "testing" {expire="30s,command=OFF"}

fails at load time in the same way as the simpler cases.

9037568 commented 5 years ago

Do you plan to provide the information I requested?

Rossko57 commented 5 years ago

I'm so sorry, I don't know what else I should provide?

I provided an example of a Group that accepts ON commands, and provided the definition of that group and the definition of it's single member, and the log of that Group accepting commands. This is just demonstrating ordinary, published openHAB capability.

I provided more than one example of a Group with expire binding configured, and the log of the failure of that configuring.

9037568 commented 5 years ago

I was looking for the full set of item definitions. But if you're saying the items file only contains two items, then ok, I get it. Thanks for the clarification.

Group testGroup "testing"
Switch testSwitch  "testing" (testGroup)
Rossko57 commented 5 years ago

Okeydoke, just tried to provide the simplest example. The {expire} problem occurs on a Group with no members, but that's a pretty dumb example :)

Stefan300381 commented 3 years ago

hi guys,

EDIT: Ignore my BS below ;-) As written in the last comment from Rossk57, this seems to be an issue only with groups without members.

this issue hit me in the latest test build OH 2.5.9 Here I see the ticket is set to OPEN since nearly two years, any chance this will get fixed?

Stefan

Rossko57 commented 3 years ago

I doubt there will be any movement on this now, with OH2 core essentially locked. Issue left open as a design prompt for whatever the OH3 "expire" replacement looks like. Working (or not!) with Groups is an edge case, but intended behaviour should be defined.

cweitkamp commented 3 years ago

No, there will be no ore action over here. If you are interested in the new "expire" feature and want to add a comment or an idea have a look in the OHC3 related issue: https://github.com/openhab/openhab-core/issues/1620

tl;dr current idea is to introduce a replacement - a new core feature - for the functionally based on metadata configuration similar to "autoupdate".

9037568 commented 3 years ago

Closing this, since we have another issue for tracking purposes.