Converting state into accepted types

[sjsf]

This PR is intended to deal with the fact that users might be so creative and link e.g. a SwitchItem to a color channel which maintains a HSBType state. In order to get these items updated successfully whenever values change, the State object needs to be converted into something accepted (if possible at all...).

When implementing this,

• I needed access to the linked items (and not only their names). Therefore I changed the AbstractLinkRegistry.getLinkedItems() method to AbstractLinkRegistry.getLinkedItemNames() because that is what it actually does and introduced a ItemChannelLinkRegistry.getLinkedItems() method with returns what it promises. The alternative would have been to re-add the ThingManager -> ItemRegistry dependency which I assume we all are happy was dropped.
• I decided to do the conversion in all three places: the ThingManager, ItemUpdater and AutoUpdater. Sending the state update event "as is" and doing the conversion in the ItemUpdater would lead to UIs still getting "wrong" events so they won't benefit from it. And doing it only in the ThingManager is not enough as there might be other origins for events having the "wrong" type. The ItemUtil seems like a good place for it.
• I had to change the order of the acceptedDataTypes in some items so they have the most meaningful type at the top of the list. I'm very well aware of the TypeParser and therefore I'm wondering even more why they were ordered like that before... I'm just not 100% sure if anywhere else something might rely in this order being the over way round of if if worked until now "by chance" because there was no overlap and the result will always be the same.
• I introduced the Convertible interface so that not all types have to implement that method. Alternatively an AbstractState could provide a default implemnentation, if you like that better. I whish we had Java 8...
• I decided to put the conversion logic into the types themselves. The conversion logic doesn't seem to be context specific, so it does not need to be within the items. Alternatively we could have something like a TypeConverter which takes a State and a target type and does the conversion. I thought it's easier to maintain like this, but if you prefer a clearer SoC, I will happily factory it out.

An example to test it with:

sample.things:

Bridge hue:bridge:01234567890 "Philips Hue Bridge" [ ipAddress="192.168.0.42", userName="********" ] {
    0210 1 "Hue Bulb" [ lightId="1" ]
}

sample.items:

Switch hue_switch "Hue Bulb Switch" <light> { channel="hue:0210:01234567890:1:color" }
Dimmer hue_dimmer "Hue Bulb Dimmer" <light> { channel="hue:0210:01234567890:1:color" }
Color hue_color "Hue Bulb Color" <light> { channel="hue:0210:01234567890:1:color" }

sample.sitemap:

sitemap demo label="Sample" {
    Frame label="Sample" {
        Switch item=hue_switch
        Slider item=hue_dimmer
        Colorpicker item=hue_color
    }

}

fixes #2253
Signed-off-by: Simon Kaufmann simon.kfm@googlemail.com

[watou]

Does this PR address any of the conversations at the ESH forum on this subject? The mechanism in openHAB 1 that allows bindings to determine and adapt to the item's accepted data types at state update time is not present in the ESH APIs, and allowing bindings like HTTP, MQTT, TCP, etc., to be implemented against the ESH APIs would be a nice addition. (Currently, the only way to get this capability is via OH1 bindings and the compat1x layer).

[sjsf]

The mechanism in openHAB 1 that allows bindings to determine and adapt to the item's accepted data types at state update time is not present in the ESH APIs

Wouldn't channels be the right layer for bindings to adapt to? I mean, since opposed to OH1 we have the abstraction between the "real world" (Things) and the presentation layer (Items) in ESH, it sounds odd to me if bindings should know anything about items at all. Channels have an accepted item type which bindings can access. I think I would need to further understand the use-cases you are referring to, but it sounds to me like this discussion is a out-of-scope within this PR.

This PR addresses the presentation layer, mainly allowing "more simplistic" items to be bound to channels which are defined with more "sophisticated" datatypes (e.g. SwitchItem against a PercentType). Maybe this implicitly also solves some use-cases you have in mind...?

[watou]

it sounds odd to me if bindings should know anything about items at all

The issue is usability. For example, only the user knows that an MQTT topic publishes decimal numbers, and wants a Number item to be updated when some client publishes on that topic. There is no feature of the ESH APIs that make this simple configuration possible, while the MQTT binding in openHAB 1.x adapts to the accepted data types of the bound item because that model ties bindings directly to items without channels adding another layer to contend with.

This PR addresses the presentation layer

If your PR is on a completely different subject that's fine with me, but my interest is seeing how the simple use cases of OH1 could be implemented with ESH APIs and to not miss an opportunity to improve the ESH APIs to address those use cases. Generally, embedding the concept in the architecture instead of at the presentation layer could potentially yield more benefits.

[kaikreuzer]

I think the discussion @watou is referring to is #583.
So in general, there might be a need to have the possibility for a binding to find out what types are linked to it and how these links are configured (note that we do not yet have configurations on links). Imho we should NOT allow it to find out about the individual items that are linked to it, though. #583 probably goes beyond the scope of this PR, but it is fair of @watou to mention it here as it might have an impact on the chosen implementation (which I didn't yet review, so I don't have any opinion on this right now).

@SJKA Without having had a look at your code yet, only having read your description: It strikes me odd that you also touch ItemUpdater and AutoUpdateBinding - they only deal with Item events, so on this level, everything should be alright already. There is no need to convert a Percent command to an ON state, because if the item is a switch, a Percent command is anyhow invalid and we do not have to care for such cases (actually we should not). Am I missing something?

[sjsf]

There is no need to convert a Percent command to an ON state, because if the item is a switch, a Percent command is anyhow invalid and we do not have to care for such cases (actually we should not).

Okay, I just thought it would be cool as we easily can allow such stuff using the same code, but actually wasn't aware that it should fail by intention. I'll remove it then.

[kaikreuzer]

wasn't aware that it should fail by intention

Well, this was my suggestion - if others think it is worthwhile to also accept invalid commands, I am open to discuss.

[kaikreuzer]

I'll remove it then.

@SJKA Just ftr, I am still waiting for this before merging, right?

[sjsf]

Hmm. I think I was not precise enough with my cites...

I removed the conversions in the AutoUpdateBinding and from the ItemUpdater.

However, IMHO we should keep the conversion from PercentType to OnOffType. I actually didn't invent it, but refactored it from DimmerItem.java. So it's not about sending (wrong) commands at all. Think of a dimmable light, where you have linked a switch item against. In order to make that switch getting updated correctly if somebody changes the brightness, we need this conversion.

[kaikreuzer]

Ok, sorry, I actually didn't notice that you had removed the changes in ItemUpdater and AutoUpdateBinding already - my comment referred to these, so all fine!

I had to change the order of the acceptedDataTypes in some items

I think these changes should be ok - afair, the order was mainly relevant for StringType, since parsing as a String always succeeds (so UNDEF etc. has to come first).

Alternatively an AbstractState could provide a default implemnentation, if you like that better. I whish we had Java 8...

I agree, a default impl would be much nicer - something to add to https://github.com/eclipse/smarthome/wiki/Java-8-CP2-cleanup, I think.
May I suggest a change in the method name? How about calling it simply as(Type type) instead of convertTo?

Alternatively we could have something like a TypeConverter which takes a State

No, I am fine with having the logic in the types. But as you mention "TypeConverter": I wonder if we should make the as() method available on Type and not only on State (once we add it as a Java8 default impl. For now, this would mainly impact the JavaDoc of the Convertible interface)? I don't have a use case at hand right now, but it feels ok to also leave the option to have it implemented on Commands.

[sjsf]

May I suggest a change in the method name? How about calling it simply as(Type type) instead of convertTo?

Yes, sure. I changed it.

I wonder if we should make the as() method available on Type and not only on State (once we add it as a Java8 default impl. For now, this would mainly impact the JavaDoc of the Convertible interface)?

I agree, it feels okay. The main reason why I haven't done that so far is the lovely type erasure in Java: The items actually want a State, not a Type, therefore we'd need to cast it at every usage. This would be solvable if we could change the signature to something like this:

public <T extends Type> T as(Class<T> target);

However, Java does not accept any sub-type of Type as T for a return value (i.e. OnOffType.ON is marked as an error). We would clutter the code completely with casts at this side then.

Again, with Java 8 we would get the chance to implement some magic in the interface that does the casting only in a single place, but so far I don't see a straight-forward way of doing this while keeping both sides clean to read. Maybe I missed something...?

[kaikreuzer]

The items actually want a State, not a Type, therefore we'd need to cast it at every usage.

Ok, that's a very valid argument.

[kaikreuzer]

Thanks!