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] Remove state description step size handling #12192

Merged
merged 1 commit into from
Feb 6, 2022

Conversation

maniac103
Copy link
Contributor

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).

See #12183 for a concrete breakage example.

Fixes #12183

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>
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lolodomo
Copy link
Contributor

lolodomo commented Feb 3, 2022

@FStolte @gerrieg @mdicke2s : is it ok for you?

@lolodomo lolodomo added the bug An unexpected problem or unintended behavior of an add-on label Feb 3, 2022
@@ -361,8 +354,6 @@ public ParameterOption createOption(String value, String description) {
}
builder.withMinimum(MetadataUtils.createBigDecimal(dp.getMinValue()));
builder.withMaximum(MetadataUtils.createBigDecimal(maxValue));
builder.withStepSize(MetadataUtils
.createBigDecimal(dp.isFloatType() ? Float.valueOf(0.1f) : Long.valueOf(1L)));
builder.withUnitLabel(MetadataUtils.getUnit(dp));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing came to my mind: maybe we should keep step=1 for integer types.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the UI knows integers have no fraction, so it should be fine to omit it.

@lolodomo
Copy link
Contributor

lolodomo commented Feb 6, 2022

Will be merged this evening if no veto.

@lolodomo lolodomo merged commit f29d86c into openhab:main Feb 6, 2022
@lolodomo lolodomo added this to the 3.3 milestone Feb 6, 2022
@TBail
Copy link

TBail commented Feb 7, 2022

I am using now Build 2744 and i think that the change made it even worse, because i did not get any configuration
image

@maniac103
Copy link
Contributor Author

because i did not get any configuration

Anything relevant in the logs?

@maniac103
Copy link
Contributor Author

maniac103 commented Feb 7, 2022

FWIW, I just tried locally with my push button devices which have a similar data point, whose config description is now this one:

    {
      "default": "0.4",
      "description": "Mindestdauer für langen Tastendruck",
      "label": "Long Press Time",
      "name": "HMP_1_LONG_PRESS_TIME",
      "required": false,
      "type": "DECIMAL",
      "min": 0.3,
      "max": 1.8,
      "readOnly": false,
      "multiple": false,
      "groupName": "HMG_1",
      "advanced": false,
      "verify": false,
      "limitToOptions": true,
      "unitLabel": "s",
      "options": [],
      "filterCriteria": []
    },

... so step is gone as expected. It's still present in UI though, but I created this in my dev environment from scratch, so maybe deleting and re-adding the thing is worth a try?

In any case, this PR was only half of the solution it seems:

grafik

... because MainUI employs a similar arbitrary default step size.
@ghys Any comments about this? Please see #12183 for the actual discussion triggering this PR, which shows there's (HM) devices that need step sizes of 0.001, and the binding can not (meaningfully) provide the step size.

@TBail
Copy link

TBail commented Feb 7, 2022

Deleteting and Readding didnt solve the problem. Without deleting the thing i got the old messages, after deleting and readding the thing the configuration is gone.

until now i had no time to look into the logs

@ghys
Copy link
Member

ghys commented Feb 7, 2022

similar arbitrary default step size.

You're right it's arbitrary but based on what seemed to be (admittedly, empirically) the most common step size for a decimal when it's not provided.
It's actually a pain point in the UI as well as it was changed over the last 1.5 years in openhab/openhab-webui#434, openhab/openhab-webui#658, openhab/openhab-webui#873
If it's not possible to figure out the most appropriate step size I don't really know what to advise here apart from providing the most granular step size that can be expected.

@maniac103
Copy link
Contributor Author

If it's not possible to figure out the most appropriate step size I don't really know what to advise here apart from providing the most granular step size that can be expected.

Wouldn't the most logical choice be 'do not enforce any step size at all in that case'?

@ghys
Copy link
Member

ghys commented Feb 7, 2022

Sure, in this case it's probably the best choice.

https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/step

The step attribute is a number that specifies the granularity that the value must adhere to or the keyword any. It is valid for the numeric input types, including the date, month, week, time, datetime-local, number and range types.

That would be setting the HTML input element's step attribute to "any" if the config description doesn't specify a stepsize.

I remember considering that and then backtracking... but I don't remember the reason for the backtrack right now 😓

@TBail
Copy link

TBail commented Feb 19, 2022

@maniac103 @ghys So do i understand it right that this is an issue in the UI, so should we open an issues?

@maniac103
Copy link
Contributor Author

@TBail There is an issue left in the UI, but that's aside from your 'option isn't shown at all' issue (which I can not reproduce).

@TBail
Copy link

TBail commented Feb 19, 2022

Now i updated homegear deleted the thing an redicovered them, but still the following error

image

@TBail
Copy link

TBail commented Feb 25, 2022

As far as i can see from the log the problem ist not within the Homematic binding. a Trace shows that the thing receives values.

2022-02-25 08:44:03.367 [DEBUG] [ommunicator.AbstractHomematicGateway] - Received new (Integer) value '-49' for 'MEQ0381486:0#RSSI_DEVICE' from gateway with id '8FBEF76F-0C30-3305-BE99-1D32A72A7FAF'
2022-02-25 08:44:03.372 [TRACE] [nicator.server.BinRpcResponseHandler] - Event BinRpcMessage: system.multicall()
[
	{
		methodName=event
		params=
		[
			0ee4c62e-a3d3-4380-9ee1-c0b59db0747f
			MEQ0381486:1
			BOOT
			false
		]
	}
	{
		methodName=event
		params=
		[
			0ee4c62e-a3d3-4380-9ee1-c0b59db0747f
			MEQ0381486:1
			GAS_ENERGY_COUNTER
			0.24
		]
	}
	{
		methodName=event
		params=
		[
			0ee4c62e-a3d3-4380-9ee1-c0b59db0747f
			MEQ0381486:1
			GAS_POWER
			0.74
		]
	}
]

2022-02-25 08:44:03.373 [DEBUG] [ommunicator.AbstractHomematicGateway] - Received new (Boolean) value 'false' for 'MEQ0381486:1#BOOT' from gateway with id '8FBEF76F-0C30-3305-BE99-1D32A72A7FAF'
2022-02-25 08:44:03.375 [DEBUG] [ommunicator.AbstractHomematicGateway] - Received new (Double) value '0.24' for 'MEQ0381486:1#GAS_ENERGY_COUNTER' from gateway with id '8FBEF76F-0C30-3305-BE99-1D32A72A7FAF'
2022-02-25 08:44:03.377 [DEBUG] [ommunicator.AbstractHomematicGateway] - Received new (Double) value '0.74' for 'MEQ0381486:1#GAS_POWER' from gateway with id '8FBEF76F-0C30-3305-BE99-1D32A72A7FAF'

So it seems that this is still an ui problem. I will try to find some hints on that.

@TBail
Copy link

TBail commented Feb 25, 2022

Now i got the trace from the validation process.

2022-02-25 09:02:43.414 [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_-1_BAUDRATE=The value 5 does not match allowed parameter options. Allowed options are: [ParameterOption [value="300 Bd", label="300 Bd"], ParameterOption [value="600 Bd", label="600 Bd"], ParameterOption [value="1200 Bd", label="1200 Bd"], ParameterOption [value="2400 Bd", label="2400 Bd"], ParameterOption [value="4800 Bd", label="4800 Bd"], ParameterOption [value="9600 Bd", label="9600 Bd"], ParameterOption [value="19200 Bd", label="19200 Bd"]], HMP_-1_METER_PROTOCOLMODE=The value 3 does not match allowed parameter options. Allowed options are: [ParameterOption [value="PROTOKOLL_MODE_A", label="PROTOKOLL_MODE_A"], ParameterOption [value="PROTOKOLL_MODE_B", label="PROTOKOLL_MODE_B"], ParameterOption [value="PROTOKOLL_MODE_C", label="PROTOKOLL_MODE_C"], ParameterOption [value="PROTOKOLL_MODE_D", label="PROTOKOLL_MODE_D"]], HMP_0_METER_POWERMODE=The value 0 does not match allowed parameter options. Allowed options are: [ParameterOption [value="MAINS_POWERED", label="Netzbetrieb"], ParameterOption [value="BATTERY_POWERED", label="Batteriebetrieb"]], HMP_0_METER_PROTOCOLMODE=The value 3 does not match allowed parameter options. Allowed options are: [ParameterOption [value="PROTOKOLL_MODE_A", label="PROTOKOLL_MODE_A"], ParameterOption [value="PROTOKOLL_MODE_B", label="PROTOKOLL_MODE_B"], ParameterOption [value="PROTOKOLL_MODE_C", label="PROTOKOLL_MODE_C"], ParameterOption [value="PROTOKOLL_MODE_D", label="PROTOKOLL_MODE_D"]], HMP_0_BAUDRATE=The value 5 does not match allowed parameter options. Allowed options are: [ParameterOption [value="300 Bd", label="300 Bd"], ParameterOption [value="600 Bd", label="600 Bd"], ParameterOption [value="1200 Bd", label="1200 Bd"], ParameterOption [value="2400 Bd", label="2400 Bd"], ParameterOption [value="4800 Bd", label="4800 Bd"], ParameterOption [value="9600 Bd", label="9600 Bd"], ParameterOption [value="19200 Bd", label="19200 Bd"]], HMP_-1_METER_POWERMODE=The value 0 does not match allowed parameter options. Allowed options are: [ParameterOption [value="MAINS_POWERED", label="Netzbetrieb"], ParameterOption [value="BATTERY_POWERED", label="Batteriebetrieb"]], HMP_-1_SERIAL_FORMAT=The value 0 does not match allowed parameter options. Allowed options are: [ParameterOption [value="1_7D_1P_E_1S", label="1_7D_1P_E_1S"], ParameterOption [value="1_7D_1P_E_2S", label="1_7D_1P_E_2S"], ParameterOption [value="1_8D_0P_N_1S", label="1_8D_0P_N_1S"], ParameterOption [value="1_8D_1P_E_1S", label="1_8D_1P_E_1S"]], HMP_2_METER_TYPE=The value 4 does not match allowed parameter options. Allowed options are: [ParameterOption [value="GAS-SENSOR", label="GAS-SENSOR"], ParameterOption [value="IR-SENSOR", label="IR-SENSOR"], ParameterOption [value="LED-SENSOR", label="LED-SENSOR"], ParameterOption [value="IEC-SENSOR", label="IEC-SENSOR"], ParameterOption [value="UNKOWN", label="UNKOWN"]], HMP_1_METER_TYPE=The value 4 does not match allowed parameter options. Allowed options are: [ParameterOption [value="GAS-SENSOR", label="GAS-SENSOR"], ParameterOption [value="IR-SENSOR", label="IR-SENSOR"], ParameterOption [value="LED-SENSOR", label="LED-SENSOR"], ParameterOption [value="IEC-SENSOR", label="IEC-SENSOR"], ParameterOption [value="UNKOWN", label="UNKOWN"]], HMP_0_SERIAL_FORMAT=The value 0 does not match allowed parameter options. Allowed options are: [ParameterOption [value="1_7D_1P_E_1S", label="1_7D_1P_E_1S"], ParameterOption [value="1_7D_1P_E_2S", label="1_7D_1P_E_2S"], ParameterOption [value="1_8D_0P_N_1S", label="1_8D_0P_N_1S"], ParameterOption [value="1_8D_1P_E_1S", label="1_8D_1P_E_1S"]]}
2022-02-25 09:02:43.416 [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.

It seems to me that the validation does not match the entry indexes.

@TBail
Copy link

TBail commented Feb 25, 2022

And now i tried to set vaild parameters and i got the standard eroor

image

@maniac103
Copy link
Contributor Author

And now i tried to set vaild parameters and i got the standard eroor

That one is the one I mentioned here. For the other one I agree this is clearly a UI, not binding, issue, because indices were saved as option value instead of the actual value.

@TBail
Copy link

TBail commented Feb 25, 2022

So at the end the problem is that we need a step size that covers most of the option. So i would suggest to set a value of 1*10E-6.

With this value the ui sould be satisfied and it is up to the user to decide if the value is usefull for the channel.

@ghys Isn't that an option?

@maniac103
Copy link
Contributor Author

I think he already shared what should be done here ;-)

@Elle4u
Copy link

Elle4u commented Mar 22, 2022

Since the update from 3.2.0 stable to 3.3.0 M1/M2 I have problems with 6 of my actors (things).
I am getting the error, mentioned above:
Pending

Now with M2 I get the following additinal information:
2022-03-22 10:04:50.208 [INFO ] [ab.event.ThingStatusInfoChangedEvent] - Thing 'homematic:HM-ES-PMSw1-Pl-DN-R1:CCU3:OEQ0572502' changed from UNINITIALIZED (BRIDGE_UNINITIALIZED) to UNINITIALIZED (HANDLER_CONFIGURATION_PENDING): {HMP_1_POWERUP_ACTION=Der Wert 0 ist nicht in den erlaubten Optionen enthalten. Erlaubte Optionen sind: [ParameterOption [value="POWERUP_OFF", label="none"], ParameterOption [value="POWERUP_ON", label="simulate short button press"]]}
(more in Text-File)

Is this regarding to this issue/change?

@maniac103
Copy link
Contributor Author

You're not the only one (see here), but it's a different issue, likely in the UI.

@Elle4u
Copy link

Elle4u commented Mar 23, 2022

OK. If I can test something, please don't hesitate to contact me ;)

Until this I will have to stay at the final version...

P.S. Interesting: I have 3 "HmIPW-DRBL4". One is still working and 2 have this problem. All the same FW-Versions.

@maniac103
Copy link
Contributor Author

maniac103 commented Mar 23, 2022

If you have some working and some broken ones, it may be worth a try to check whether

  • saving the config in the UI for a broken one 'revives' it
  • saving the config in the UI for a working one breaks it

That way, one could probably tell if the problem is caused by some condition in the past (first case) or in present code (second case).

Edit: This code block should update configuration settings whenever the thing handler is initialized (that is, when starting OH or when disabling/re-enabling a thing).

It would probably be nice to see a startup log of OH with enabled TRACE logging for HM binding, to check whether anything changed in the output format of the CCU. What also would be interesting is whether deleting and recreating (one of, for now) the affected things helps.

@Elle4u
Copy link

Elle4u commented Mar 26, 2022

Also ich habe auf TRACE gestellt und neu gestartet. Dann war vieles nicht INITILIZED (weil zu langsam, denke ich).
Habe die Bridge dann DISABLED und ENABELD -> nach einiger Zeit hatte openhab gar keine AddOns mehr geladen. Konnte kein MAP mehr usw... Das scheint also für meine HW (rasperry 3) ggf. zu viel zu sein...

Ich mache jetzt mal folgendes:
Ich bringe alles wieder an Laufen und mache dann das TRACE mal nur zum Löschen/Hinzufügen bzw. DISABLE/ENABLE von einem betroffenen Gerät. Einmal mit 3.2.0 und einmal mit dem akt. Milestone...

@maniac103
Copy link
Contributor Author

zum Löschen/Hinzufügen bzw. DISABLE/ENABLE von einem betroffenen Gerät

Disable/Enable ist nicht das gleiche wie Löschen/Hinzufügen. Die von der CCU geladenen Datenpunkte werden nach Disable/Enable nicht neu geladen.

@Elle4u
Copy link

Elle4u commented Mar 28, 2022

OK. Ich habe also 3 der betroffenen Devices entfernt,

  • HmIPW-DRBL4 Links
  • HmIPW-DRBL4 Links unten
  • HmIPW-DRBL4 Links Mitte

TRACE eingeschaltet und DISCOVER gestartet.
Es wurden 2 gefunden und diese habe ich beide hinzugefügt.
Dann TRACE habe ich anschließend wieder deaktiviert.
Bei einem erneutem Scan wurde dann auch das 3. Device gefunden.

Ich habe das mehrfach wiederholt. Während TRACE an ist, werden nicht alle Devices sofort gefunden (TIMEOUT?). Aber das ist ja jetzt nicht das Problem ;)

Das Log von dem einen Vorgang anbei: 3_2_0_Release.zip

Jetzt habe ich auf das akt. Milestone aktualisiert und wieder sind von den 3 Devices 2 nicht in Ordnung:
2022-03-28 11_32_46-openHAB

Habe die obige Prozedur wiederholt und das Problem bleibt bestehen.
Schlimmer - jetzt funktioniert auch das 3. Device nicht mehr:
2022-03-28 11_45_47-openHAB

Hier das Log dazu: 3_3_0_M2.zip

Ich hoffe, ihr könnt mit den gesammelten Informationen etwas anfangen ;)

Elle

EDIT: Sorry for switching to german. That was not my intention....

NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request 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 pull request 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 pull request 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 pull request 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 pull request 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 this pull request may close these issues.

[homematic] HM-ES-TX-WM Gasmeter not funktioning due to config error
6 participants