Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[homematic] HM-ES-TX-WM Gasmeter not funktioning due to config error #12183

Closed
TBail opened this issue Feb 1, 2022 · 17 comments · Fixed by #12192
Closed

[homematic] HM-ES-TX-WM Gasmeter not funktioning due to config error #12183

TBail opened this issue Feb 1, 2022 · 17 comments · Fixed by #12192
Labels
bug An unexpected problem or unintended behavior of an add-on

Comments

@TBail
Copy link

TBail commented Feb 1, 2022

I am using the latest nightly and have a probelm that my gas meter stay in the state of handler_configuration_pendig.
image
if i try to save the thing i got the message that some configuration values are not right.

image

image

image

image

The meter constant is in the required Range and has worked in earlier versions of openHAB. The threshold could not be changed and saved, because the meter constand should not bei changed.
@TBail TBail added the bug An unexpected problem or unintended behavior of an add-on label Feb 1, 2022
@J-N-K
Copy link
Member

J-N-K commented Feb 1, 2022

Please show the config-description for this thing. You an get it from the REST API. First retrieve the thingType, take the config-description-uri from there and the request the config-description with that uri. Thanks.

@TBail
Copy link
Author

TBail commented Feb 1, 2022

I just copied out the relevant parts.

    {
        "default": "0.01",
        "label": "Meter Constant Gas",
        "name": "HMP_1_METER_CONSTANT_GAS",
        "required": false,
        "type": "DECIMAL",
        "min": 0.001,
        "max": 65.536,
        "stepsize": 0.1,
        "readOnly": false,
        "multiple": false,
        "groupName": "HMG_1",
        "advanced": false,
        "verify": false,
        "limitToOptions": true,
        "unitLabel": "m3/Imp.",
        "options": [],
        "filterCriteria": []
    }

From my Point of view the step size does not match the default and the min values in some way

    {
        "default": "100.0",
        "description": "TX Differenz Leistung",
        "label": "Tx Threshold Power",
        "name": "HMP_1_TX_THRESHOLD_POWER",
        "required": false,
        "type": "DECIMAL",
        "min": 0.01,
        "max": 160000,
        "stepsize": 0.1,
        "readOnly": false,
        "multiple": false,
        "groupName": "HMG_1",
        "advanced": false,
        "verify": false,
        "limitToOptions": true,
        "unitLabel": "W",
        "options": [],
        "filterCriteria": []
    },

The attached file has the full definition
Untitled-1.txt

@J-N-K
Copy link
Member

J-N-K commented Feb 1, 2022

The step size is not validated ATM and needs to be fixed by the binding developer. The problem is that it is not possible just to change a file, the config description are auto-generated.

I'll check why the validation for saved values fails.

@J-N-K
Copy link
Member

J-N-K commented Feb 1, 2022

It doesn't fail in the test for me. Can you please set org.openhab.core.thing.internal.ThingManagerImpl to TRACE logging and disable/enable the thing once? You should find a log-message Failed to validate config for '{}' with URI '{}': {} then which contains the detected violations. Thanks. You can revert to normal logging with a log-level of DEFAULT.

@TBail
Copy link
Author

TBail commented Feb 1, 2022

These are the log entries.

16:02:01.816 [DEBUG] [.core.thing.internal.ThingManagerImpl] - Thing with UID homematic:HG-HM-ES-TX-WM:8FBEF76F-0C30-3305-BE99-1D32A72A7FAF:MEQ0381486 will be persisted as enabled..
16:02:01.819 [DEBUG] [.core.thing.internal.ThingManagerImpl] - Thing homematic:HG-HM-ES-TX-WM:8FBEF76F-0C30-3305-BE99-1D32A72A7FAF:MEQ0381486 will be enabled.
16:02:01.821 [DEBUG] [.core.thing.internal.ThingManagerImpl] - Calling 'HomematicThingHandlerFactory.registerHandler()' for thing 'homematic:HG-HM-ES-TX-WM:8FBEF76F-0C30-3305-BE99-1D32A72A7FAF:MEQ0381486'.
16:02:01.827 [TRACE] [.core.thing.internal.ThingManagerImpl] - Failed to validate config for 'homematic:HG-HM-ES-TX-WM:8FBEF76F-0C30-3305-BE99-1D32A72A7FAF:MEQ0381486' with URI 'thing-type:homematic:HG-HM-ES-TX-WM': {HMP_2_TX_THRESHOLD_POWER=The value must not be less than 0.01., HMP_1_TX_THRESHOLD_POWER=The value must not be less than 0.01.}
16:02:01.829 [DEBUG] [.core.thing.internal.ThingManagerImpl] - Thing 'homematic:HG-HM-ES-TX-WM:8FBEF76F-0C30-3305-BE99-1D32A72A7FAF:MEQ0381486' not initializable, check if required configuration parameters are present and set values are valid.

I tried to change the Value to the default of 100 and get a message
image

With a value of 100,01 the message is gone

@J-N-K
Copy link
Member

J-N-K commented Feb 1, 2022

Ok, so the issue is not in the validation code but UI/binding. The presented "step" value is invalid and therefore it is not possible to save valid values (defined by min/max). This is the case for "meter constant". The validation code complains about the TX Threshold power and which indeed has an invalid value of 0.

I'm not entirely sure what the best way to proceed is. IMO the step values should probably be removed from the binding as they don't make sense (invalid for the "meter constant") or require "strange" values (like the TX power).

@MHerbst and @maniac103 I believe you did most of the work on this binding. WDYT?

@cweitkamp
Copy link
Contributor

Thanks for the analysis. Afair there have been done similar corrections for other devices like #9434.

@maniac103
Copy link
Contributor

maniac103 commented Feb 1, 2022

The problem is right below the code from #9434: here
It looks like the CCU doesn't provide any information about the step size, only min and max (this method doesn't even know about step size). The binding thus tries (not very hard) to second guess it.
I wonder what a meaningful approach should look like, though:

  • Omit step completely? But the results may be unexpected as well, e.g. if one enters a value like '1.234567' in the field above and it gets truncated when sending it out.
  • Or rather try to guess it from the number of decimal digits of the min or max value and only fall back to '1 decimal digit' in case the min/max values have no decimal digits?

@J-N-K
Copy link
Member

J-N-K commented Feb 1, 2022

IMO it should be removed. It's only used for helping the UI to display sliders.

@TBail
Copy link
Author

TBail commented Feb 1, 2022

At the moment my device is not usabel, because i could not set the gas meter constant to the right value.

I am with @J-N-K . Just remove it. Since openHAB 1.xxx i think it worked without this check and now these checks make more trouble.

@maniac103
Copy link
Contributor

Since openHAB 1.xxx i think it worked without this check

The step generation is in there since (at least) the initial OH2 Homematic binding, committed in April 2016. If anything is new, it's the UI side of things.

@MHerbst
Copy link
Contributor

MHerbst commented Feb 1, 2022

I agree, a step size of 0.1 does not make sense for large ranges like in this example.

It is OK for me to remove the stepsize if we are sure that it will not lead to further problems.

@J-N-K
Copy link
Member

J-N-K commented Feb 1, 2022

The step-size only restricts what can be set in the UI. In textual configuration only min/max are validated, so I would be very surprised if it would lead to any issues.

@maniac103
Copy link
Contributor

In textual configuration only min/max are validated

This is probably why it went unnoticed for so long... +1 for removing it in that case.

@maniac103
Copy link
Contributor

maniac103 commented Feb 1, 2022

At the moment my device is not usabel, because i could not set the gas meter constant to the right value.

BTW: wouldn't going back to textual configuration just for this one device be a viable workaround for now? Probably not I guess, because core now?) seems to validate the setting against step, as otherwise the thing handler would come up. It's not just a UI thing.

@MHerbst
Copy link
Contributor

MHerbst commented Feb 2, 2022

If no one is faster, I can take a look at it this weekend and remove the stepsize.

maniac103 added a commit to maniac103/openhab2-addons that referenced this issue Feb 3, 2022
The RPC protocol doesn't provide this value, so it always was made up
more or less arbitrarily. Since the UI now uses this value for
validation purposes, there are cases where valid values can not be
entered due to this step size (particularly for datapoints with more
than 1 decimal digit).

Fixes openhab#12183

Signed-off-by: Danny Baumann <dannybaumann@web.de>
@maniac103
Copy link
Contributor

Done -> #12192

lolodomo pushed a commit that referenced this issue Feb 6, 2022
The RPC protocol doesn't provide this value, so it always was made up
more or less arbitrarily. Since the UI now uses this value for
validation purposes, there are cases where valid values can not be
entered due to this step size (particularly for datapoints with more
than 1 decimal digit).

Fixes #12183

Signed-off-by: Danny Baumann <dannybaumann@web.de>
volkmarnissen pushed a commit to volkmarnissen/openhab-addons that referenced this issue Mar 3, 2022
The RPC protocol doesn't provide this value, so it always was made up
more or less arbitrarily. Since the UI now uses this value for
validation purposes, there are cases where valid values can not be
entered due to this step size (particularly for datapoints with more
than 1 decimal digit).

Fixes openhab#12183

Signed-off-by: Danny Baumann <dannybaumann@web.de>
NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this issue Apr 27, 2022
The RPC protocol doesn't provide this value, so it always was made up
more or less arbitrarily. Since the UI now uses this value for
validation purposes, there are cases where valid values can not be
entered due to this step size (particularly for datapoints with more
than 1 decimal digit).

Fixes openhab#12183

Signed-off-by: Danny Baumann <dannybaumann@web.de>
Signed-off-by: Nick Waterton <n.waterton@outlook.com>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this issue Jun 29, 2022
The RPC protocol doesn't provide this value, so it always was made up
more or less arbitrarily. Since the UI now uses this value for
validation purposes, there are cases where valid values can not be
entered due to this step size (particularly for datapoints with more
than 1 decimal digit).

Fixes openhab#12183

Signed-off-by: Danny Baumann <dannybaumann@web.de>
andan67 pushed a commit to andan67/openhab-addons that referenced this issue Nov 6, 2022
The RPC protocol doesn't provide this value, so it always was made up
more or less arbitrarily. Since the UI now uses this value for
validation purposes, there are cases where valid values can not be
entered due to this step size (particularly for datapoints with more
than 1 decimal digit).

Fixes openhab#12183

Signed-off-by: Danny Baumann <dannybaumann@web.de>
andrasU pushed a commit to andrasU/openhab-addons that referenced this issue Nov 12, 2022
The RPC protocol doesn't provide this value, so it always was made up
more or less arbitrarily. Since the UI now uses this value for
validation purposes, there are cases where valid values can not be
entered due to this step size (particularly for datapoints with more
than 1 decimal digit).

Fixes openhab#12183

Signed-off-by: Danny Baumann <dannybaumann@web.de>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
psmedley pushed a commit to psmedley/openhab-addons that referenced this issue Feb 23, 2023
The RPC protocol doesn't provide this value, so it always was made up
more or less arbitrarily. Since the UI now uses this value for
validation purposes, there are cases where valid values can not be
entered due to this step size (particularly for datapoints with more
than 1 decimal digit).

Fixes openhab#12183

Signed-off-by: Danny Baumann <dannybaumann@web.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants