configuration = getConfigAs(); error on integers about openhab-addons HOT 23 CLOSED

Marcel, in MaxCubeBridgeConfiguration.java all members have to of the String type, and then, when you need te value, you do something like
Integer.parseInt(getConfigAs(MaxCubeBridgeConfiguration.class).refreshInterval)
Karel

from openhab-addons.

Sounds more like a workaround, I would agree with Marcel that this should be cleaned up.

from openhab-addons.

Workaround ? ;-) This is how it also done in the Hue binding for example, and I took the same approach in all the bindings that are in the pipeline. Personally I would keep it like this, because the String approach - I guess - comes from the fact that this is read as String from the DSL files. Otherwise you will have a mess of looking up the format of the variable, then converting it appropriately, complicating the Configuration.classes and so on.

On 03 Dec 2014, at 18:58, Kai Kreuzer [email protected] wrote:

Sounds more like a workaround, I would agree with Marcel that this should be cleaned up.

—
Reply to this email directly or view it on GitHub #64 (comment).

from openhab-addons.

This is how it also done in the Hue binding for example

Really? Show me where! Actually, I am not convinced that we need such a dedicated Configuration class at all - using it as a "map" might be good enough for most cases.

from openhab-addons.

Err… It’s in the YahooWeather binding

https://github.com/eclipse/smarthome/blob/master/binding/org.eclipse.smarthome.binding.yahooweather/src/main/java/org/eclipse/smarthome/binding/yahooweather/handler/YahooWeatherHandler.java#L69 https://github.com/eclipse/smarthome/blob/master/binding/org.eclipse.smarthome.binding.yahooweather/src/main/java/org/eclipse/smarthome/binding/yahooweather/handler/YahooWeatherHandler.java#L69

for example

On 04 Dec 2014, at 11:31, Kai Kreuzer [email protected] wrote:

This is how it also done in the Hue binding for example

Really? Show me where! Actually, I am not convinced that we need such a dedicated Configuration class at all - using it as a "map" might be good enough for most cases.

—
Reply to this email directly or view it on GitHub #64 (comment).

from openhab-addons.

Ah, ok. It does not use a XXXConfiguration class, but it indeed also parses the stuff from a string.
As we have config descriptions defined for the things, it should actually be possible to fill the configurations accordingly.

Am 04 Dec 2014 um 23:26 schrieb kgoderis [email protected]:

Err… It’s in the YahooWeather binding

https://github.com/eclipse/smarthome/blob/master/binding/org.eclipse.smarthome.binding.yahooweather/src/main/java/org/eclipse/smarthome/binding/yahooweather/handler/YahooWeatherHandler.java#L69 https://github.com/eclipse/smarthome/blob/master/binding/org.eclipse.smarthome.binding.yahooweather/src/main/java/org/eclipse/smarthome/binding/yahooweather/handler/YahooWeatherHandler.java#L69

for example

On 04 Dec 2014, at 11:31, Kai Kreuzer [email protected] wrote:

This is how it also done in the Hue binding for example

Really? Show me where! Actually, I am not convinced that we need such a dedicated Configuration class at all - using it as a "map" might be good enough for most cases.

—
Reply to this email directly or view it on GitHub #64 (comment).

—
Reply to this email directly or view it on GitHub #64 (comment).

from openhab-addons.

The problem must be how the value is being stored/specified. The runtime supports different primitive data types and automatic conversion to a configuration class. Does the configuration come from a DSL? I think there was a bug in ESH, that did not allow to specify integers and boolean as configuration values in the DSL. But I cannot find it anymore.

from openhab-addons.

I'll give it another try tonight if the issue has been resolved.

Quick question on the default value, is it correct that if no value is
specified the during the configuration the getConfigAs will ensure the
default value from dsl is returned ?
Op 8 dec. 2014 13:44 schreef "Dennis Nobel" [email protected]:

The problem must be how the value is being stored/specified. The runtime
supports different primitive data types and automatic conversion to a
configuration class. Does the configuration come from a DSL? I think there
was a bug in ESH, that did not allow to specify integers and boolean as
configuration values in the DSL. But I cannot find it anymore.

—
Reply to this email directly or view it on GitHub
#64 (comment).

from openhab-addons.

Dennis,

I have been investigating this bit more deeper. Indeed seems to me the issue is not so much with the getConfigAs as that appears to indeed consider the object type. However in the Configuration all properties appear to be stored as strings rather than the object type of the parameter. So in the getconfigAs line 54 Object value = this.get(field.getName()); indeed gets the right value, however in the field.set(configuration, value); it goes wrong, as the value is in string and the field as per the object type in the class.

The config of my item is created via discover, but these specific values actually are not set during the discovery but rather they come from the default value from the dsl

Hence if I am setting e.g. the value directly during discovery to a certain type (e.g. integer) than the getConfigAs actually works fine. So I assume we need to look at the part where the xml file is read to the configuration

from openhab-addons.

Thanks! Now I see where the problem is. We fixed another Bug in ESH, in which configuration values for default parameters are now automatically created. Its in the class ThingFactory. But there the type of the parameter is not honored and it creates strings all the time. So it must be fixed in ESH.

from openhab-addons.

Added the bug in ESH: https://bugs.eclipse.org/bugs/show_bug.cgi?id=454938. I will create a PR with the fix soon.

from openhab-addons.

Dennis, maybe you can also fix/upgrade the Hue/Yahoo bindings to serve as an example of good coding conduct? (or another example, that's fine as well, so at least all the openhab bindings in the pipe can be adjusted accordingly)

from openhab-addons.

Good point, thanks for the hint. I´ll adapt the Yahoo Weather binding too. The Hue binding looks fine for me.

from openhab-addons.

Done: eclipse-archived/smarthome#124

from openhab-addons.

Can be closed now as it is implemented in ESH

from openhab-addons.

Well.... it worked for a day.
Seems it broke again, I think because of commit eclipse-archived/smarthome@f7953f2

Now the value after discovery is of type bigdecimal, but once restarted and it comes from the database it comes with double type

As in the conversion class if (value != null && value instanceof BigDecimal && !typeName.equals("BigDecimal")) it fails this test and does not do conversion

from openhab-addons.

I don´t think that the mentioned PR has broken it. There seems to a general issue with the GSON serialization and deserialization for the database. I´ll try to investigate this.

from openhab-addons.

I have written a test for it and unfortunately it fails:

@Test
    void 'assert store configuration works'() {
        def storageWithoutClassloader = storageService.getStorage("storage")

        def configuration = new Configuration();
        configuration.put("bigDecimal", new BigDecimal(3))

        storageWithoutClassloader.put("configuration", configuration)

        def bigDecimal = storageWithoutClassloader.get("configuration").get("bigDecimal")
        assertThat bigDecimal instanceof BigDecimal, is(true)
    }

That means GSON deserializes doubles for serialized BigDecimal values as default. We need another bug in ESH to fix this issue (I will create one), but it is independent from the configAs feature.

from openhab-addons.

Now this is a truly unexpected behavior. This GSON stuff is really weird...

from openhab-addons.

Bug created: https://bugs.eclipse.org/bugs/show_bug.cgi?id=456979

from openhab-addons.

What's strange to me is that I'm sure it was working yesterday when I'm
testing it.
Did anything change in the configuration between yesterday and today?

2015-01-07 22:53 GMT+01:00 Dennis Nobel [email protected]:

Bug created: https://bugs.eclipse.org/bugs/show_bug.cgi?id=456979

—
Reply to this email directly or view it on GitHub
#64 (comment).

from openhab-addons.

No, it was not changed. The error is potentially there since the initial contribution. But with the introduction of the typed parameters, the bug went visible.

Please note, that it only occurs, after the configuration is deserialized from the Database, so only after you restart your openHAB instance.

from openhab-addons.

As https://bugs.eclipse.org/bugs/show_bug.cgi?id=456979 is fixed and available to openHAB 2, I close this issue.

from openhab-addons.