Improve thing updates #3576

J-N-K · 2023-04-24T19:02:03Z

lolodomo · 2023-04-29T10:14:23Z

I am going to run again my scenario with the meteoalerte binding.

lolodomo · 2023-04-29T10:41:42Z

@J-N-K : it looks clearly better ;)

First subject: the warning in log and the updated thing configuration. Is it expected ?
The logs:

2023-04-29 12:27:11.190 [WARN ] [core.thing.internal.ThingManagerImpl] - Channel types or config descriptions for thing 'meteoalerte:department:test' are missing in the respective registry for more than 120s. This should be fixed in the binding.
2023-04-29 12:27:11.227 [INFO ] [core.thing.internal.ThingManagerImpl] - Updating 'meteoalerte:department:test' from version 0 to 1
2023-04-29 12:27:11.248 [WARN ] [core.thing.internal.ThingManagerImpl] - Failed to normalize configuration for thing 'meteoalerte:department:test': {thing/channel=Type description for {0} not found although we checked the presence before.}

Please note the warning about normalizing configuration and {0} which is not replaced in the message.
Here is my initial thing config:

      "configuration": {
        "department": "VAL-D\u0027OISE",
        "refresh": 1440
      },
      "properties": {},

and the thing config after migration:


      "configuration": {
        "department": "VAL-D\u0027OISE",
        "refresh": 1440.0
      },
      "properties": {
        "thingTypeVersion": "1"
      },

As you can see the "refresh" config parameter value was changed from 1440 to 1440.0. Is it normal and expected ?
Note that the initial thing was created just today with a recent snapshot (3427) using the binding from the distribution.

The NOT_YET_READY status and the delay to initialize this thing is probably not related to the migration ?

lolodomo · 2023-04-29T10:45:42Z

Second point that needs your feedback: I see that on all my unchanged channels, something was added:
"autoUpdatePolicy": "DEFAULT". Is it normal ?
But it was not added on all migrated channels. Is it normal ?

lolodomo · 2023-04-29T10:53:19Z

But what is important to say is that you have apparently fixed the issue I declared. I mean label, description and itemType are back after migration, and label/description are the new ones from the updated channel type.

Example of channel before:

        {
          "uid": "meteoalerte:department:test:pluie-inondation",
          "id": "pluie-inondation",
          "channelTypeUID": "meteoalerte:alert-level",
          "itemType": "Number",
          "kind": "STATE",
          "label": "Etat Pluie Inondation",
          "defaultTags": [],
          "properties": {},
          "configuration": {}
        },

and after migration:

        {
          "uid": "meteoalerte:department:test:pluie-inondation",
          "id": "pluie-inondation",
          "channelTypeUID": "meteoalerte:pluie-inondation",
          "itemType": "Number",
          "kind": "STATE",
          "label": "Pluie Inondation",
          "description": "Niveau d\u0027alerte aux inondations causées par les précipitations",
          "defaultTags": [],
          "properties": {},
          "configuration": {}
        },

J-N-K · 2023-04-29T11:28:24Z

Second point that needs your feedback: I see that on all my unchanged channels, something was added:
"autoUpdatePolicy": "DEFAULT". Is it normal ?
But it was not added on all migrated channels. Is it normal ?

Looking at #3575 I think that DEFAULT should not be added, it is applied anyway if it is not set.

J-N-K · 2023-04-29T11:29:20Z

The {0} not being replaced really puzzles me, because I can't find out what is wrong. It's clearly a bug, but I have no idea how to fix it.

lolodomo · 2023-04-29T11:43:44Z

Second point that needs your feedback: I see that on all my unchanged channels, something was added:
"autoUpdatePolicy": "DEFAULT". Is it normal ?
But it was not added on all migrated channels. Is it normal ?

Looking at #3575 I think that DEFAULT should not be added, it is applied anyway if it is not set.

Does your branch includes Kai's change ?
Maybe I should first move to a more recent snapshot containing Kai's change and create again the initial thing with MainUI.
Anyways, there is some kind of inconsistency to have this field set for all channels but not for the migrated ones ?

lolodomo · 2023-04-29T11:44:37Z

The {0} not being replaced really puzzles me, because I can't find out what is wrong. It's clearly a bug, but I have no idea how to fix it.

This is almost secondary. The real question is why I have this warning and why the parameter value is updated ?

As a reminder, the thing was created using MainUI and I did not set/update this parameter.

J-N-K · 2023-04-29T11:48:18Z

The value is probably also changed when you set it to 1439. It's a serialization/deserialiation issue (that's why we have the normalization code).

Regarding the warning: do you see the channel-type in the registry?

lolodomo · 2023-04-29T12:12:37Z

The value is probably also changed when you set it to 1439. It's a serialization/deserialiation issue (that's why we have the normalization code).

That would mean there is no config normalization when MainUI creates or updates the thing ?
Strange, that would be a severe bug and I thought you fixed all of them in OH3.

I can try again to restart OH keeping the original binding, just to confirm if I see the same warning and the same change of value, or if the warning and the change of value is only triggered by the thing migration.

Regarding the warning: do you see the channel-type in the registry?

You mean in org.openhab.core.thing.Thing.json file ?

lolodomo · 2023-04-29T12:15:00Z

What is strange is that MainUI still shows 1440, not 1440.0

J-N-K · 2023-04-29T12:27:06Z

Yes, that's the way it works. The issue is that the value is written as-is to the database, so if MainUI sets 1440, 1440 is written to the database. However, when the entry is de-serialized, GSON makes 1440.0 of it (because all numbers are converted to doubles). The normalization code then uses the config description to convert it to 1440.

During the thing-update the config is read (so it can be applied later) and written to the database. It doesn't matter if the database has 1440 or 1440.0 in it, the normalization code converts it to 1440 before the configuration is applied.

The goal is not to mimic what the UI does, the goal is to make it functionally equivalent. This is obviously not the case when the itemType is omitted, but 1440/1440.0 makes no difference. We would need to normalize during the update, which would require checking if all config descriptions are available before. This is very complex and - besides optics in the file - has no effect.

J-N-K · 2023-04-29T12:29:18Z

No, I mean if you use the API Explorer under channel-types and config-descriptions.

lolodomo · 2023-04-29T12:33:24Z

The goal is not to mimic what the UI does, the goal is to make it functionally equivalent. This is obviously not the case when the itemType is omitted, but 1440/1440.0 makes no difference. We would need to normalize during the update, which would require checking if all config descriptions are available before. This is very complex and - besides optics in the file - has no effect.

Ok but why is a warning logged if this is not important ?

J-N-K · 2023-04-29T12:44:34Z

The warning is independent from the update, it is written by the ThingManager when it checks if a thing can be initialized and fails to find the thing/channel-type or a referenced config-description.

I didn't check if this is the case here, but if e.g. the "old thing" has a channel-type foo and this is removed from the binding, then the ThingManager would read the (still not updated) thing from the database. Before trying to initialize the thing, a check is performed if all necessary channel/thing-types and config-descriptions are present. If that is not the case, the ThingManager waits 120s before it prints the warning and continues to initialize the thing. The next step is checking if there is any update necessary. That's the update message you are seeing. After the update, the channel with type foo has been replaces with a channel with type bar. What exactly is done with the channel during the update?

lolodomo · 2023-04-29T12:56:18Z

I didn't check if this is the case here, but if e.g. the "old thing" has a channel-type foo and this is removed from the binding, then the ThingManager would read the (still not updated) thing from the database. Before trying to initialize the thing, a check is performed if all necessary channel/thing-types and config-descriptions are present. If that is not the case, the ThingManager waits 120s before it prints the warning and continues to initialize the thing. The next step is checking if there is any update necessary. That's the update message you are seeing. After the update, the channel with type foo has been replaces with a channel with type bar. What exactly is done with the channel during the update?

Yes, exactly, that is this scenario, a channel type was removed (meteoalerte:alert-level) and new channel types were created.
The content of the migration file:

<?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
<update:update-descriptions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
	xmlns:update="https://openhab.org/schemas/update-description/v1.0.0"
	xsi:schemaLocation="https://openhab.org/schemas/update-description/v1.0.0 https://openhab.org/schemas/update-description-1.0.0.xsd">

	<thing-type uid="meteoalerte:department">

		<instruction-set targetVersion="1">
			<update-channel id="vague-submersion">
				<type>meteoalerte:vague-submersion</type>
			</update-channel>
			<update-channel id="vent">
				<type>meteoalerte:vent</type>
			</update-channel>
			<update-channel id="pluie-inondation">
				<type>meteoalerte:pluie-inondation</type>
			</update-channel>
			<update-channel id="orage">
				<type>meteoalerte:orage</type>
			</update-channel>
			<update-channel id="inondation">
				<type>meteoalerte:inondation</type>
			</update-channel>
			<update-channel id="neige">
				<type>meteoalerte:neige</type>
			</update-channel>
			<update-channel id="canicule">
				<type>meteoalerte:canicule</type>
			</update-channel>
			<update-channel id="grand-froid">
				<type>meteoalerte:grand-froid</type>
			</update-channel>
			<update-channel id="avalanches">
				<type>meteoalerte:avalanches</type>
			</update-channel>
		</instruction-set>

	</thing-type>

</update:update-descriptions>

So the NOT_YET_READY status and the delay of 120s is now explained. Remains the warning.

lolodomo · 2023-04-29T13:07:42Z

Second point that needs your feedback: I see that on all my unchanged channels, something was added:
"autoUpdatePolicy": "DEFAULT". Is it normal ?
But it was not added on all migrated channels. Is it normal ?

Looking at #3575 I think that DEFAULT should not be added, it is applied anyway if it is not set.

Does your branch includes Kai's change ? Maybe I should first move to a more recent snapshot containing Kai's change and create again the initial thing with MainUI. Anyways, there is some kind of inconsistency to have this field set for all channels but not for the migrated ones ?

I have now installed the last snapshot (3434).
When I create a thing for the official meteoalerte binding, there is no field "autoUpdatePolicy" in my org.openhab.core.thing.Thing.json file. Even after a restart of OH.
So my conclusion is that it is added by the migration stuff. I will trigger a migration again to confirm it is still the case with snapshot 3434 and your version of o.o.core.thing bundle.

lolodomo · 2023-04-29T13:27:48Z

Same result with the snapshot 3434 patched with this PR. Migration code adds "autoUpdatePolicy": "DEFAULT" on not migrated channels and do not on migrated channels. I have no idea if it is consistent and if it is without impact.

lolodomo · 2023-04-29T13:40:31Z

No, I mean if you use the API Explorer under channel-types and config-descriptions.

Result of GET /channel-types/meteoalerte:pluie-inondation:

{
  "parameters": [],
  "parameterGroups": [],
  "description": "Niveau d'alerte aux inondations causées par les précipitations",
  "label": "Pluie Inondation",
  "category": "oh:meteoalerte:pluie-inondation",
  "itemType": "Number",
  "kind": "STATE",
  "stateDescription": {
    "readOnly": true,
    "options": [
      {
        "value": "0",
        "label": "Vert"
      },
      {
        "value": "1",
        "label": "Jaune"
      },
      {
        "value": "2",
        "label": "Orange"
      },
      {
        "value": "3",
        "label": "Rouge"
      }
    ]
  },
  "tags": [],
  "UID": "meteoalerte:pluie-inondation",
  "advanced": false
}

No channels have a configuration.
The configuration of the thing is embedded in the thing type:

<?xml version="1.0" encoding="UTF-8"?>
<thing:thing-descriptions bindingId="meteoalerte"
	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
	xmlns:thing="https://openhab.org/schemas/thing-description/v1.0.0"
	xsi:schemaLocation="https://openhab.org/schemas/thing-description/v1.0.0 https://openhab.org/schemas/thing-description-1.0.0.xsd">

	<thing-type id="department">
		<label>Alertes Département</label>
		<description>Fournit les niveaux d'alerte météo pour un département.</description>

		<channels>
			<channel id="vague-submersion" typeId="vague-submersion"/>
			<channel id="vent" typeId="vent"/>
			<channel id="pluie-inondation" typeId="pluie-inondation"/>
			<channel id="orage" typeId="orage"/>
			<channel id="inondation" typeId="inondation"/>
			<channel id="neige" typeId="neige"/>
			<channel id="canicule" typeId="canicule"/>
			<channel id="grand-froid" typeId="grand-froid"/>
			<channel id="avalanches" typeId="avalanches"/>
			<channel id="vague-submersion-icon" typeId="condition-icon">
				<label>Icône Vague Submersion</label>
			</channel>
			<channel id="vent-icon" typeId="condition-icon">
				<label>Icône Vent</label>
			</channel>
			<channel id="pluie-inondation-icon" typeId="condition-icon">
				<label>Icône Pluie Inondation</label>
			</channel>
			<channel id="orage-icon" typeId="condition-icon">
				<label>Icône Orage</label>
			</channel>
			<channel id="inondation-icon" typeId="condition-icon">
				<label>Icône Inondation</label>
			</channel>
			<channel id="neige-icon" typeId="condition-icon">
				<label>Icône Neige</label>
			</channel>
			<channel id="canicule-icon" typeId="condition-icon">
				<label>Icône Canicule</label>
			</channel>
			<channel id="grand-froid-icon" typeId="condition-icon">
				<label>Icône Grand Froid</label>
			</channel>
			<channel id="avalanches-icon" typeId="condition-icon">
				<label>Icône Avalanches</label>
			</channel>
			<channel id="comment" typeId="comment"/>
			<channel id="observation-time" typeId="timestamp">
				<label>Début de validité</label>
			</channel>
			<channel id="end-time" typeId="timestamp">
				<label>Fin de validité</label>
			</channel>
		</channels>

		<properties>
			<property name="thingTypeVersion">1</property>
		</properties>

		<config-description>
			<parameter name="department" type="text" required="true">
				<label>Département</label>
				<description>Nom du département</description>
				<options>
					<option value="AIN">Ain</option>
					<option value="AISNE">Aisne</option>
					<option value="ALLIER">Allier</option>
					<option value="ALPES-DE-HAUTE-PROVENCE">Alpes de Haute Provence</option>
					<option value="ALPES-MARITIMES">Alpes Maritimes</option>
					<option value="ARDECHE">Ardèche</option>
					<option value="ARDENNES">Ardennes</option>
					<option value="ARIEGE">Ariège</option>
					<option value="AUBE">Aube</option>
					<option value="AUDE">Aude</option>
					<option value="AVEYRON">Aveyron</option>
					<option value="BAS-RHIN">Bas-Rhin</option>
					<option value="BOUCHES-DU-RHONE">Bouches du Rhône</option>
					<option value="CALVADOS">Calvados</option>
					<option value="CANTAL">Cantal</option>
					<option value="CHARENTE">Charente</option>
					<option value="CHARENTE-MARITIME">Charente Maritime</option>
					<option value="CHER">Cher</option>
					<option value="CORREZE">Corrèze</option>
					<option value="CORSE-DU-SUD">Corse du Sud</option>
					<option value="COTE-D'OR">Côte D'Or</option>
					<option value="COTES-D'ARMOR">Côtes D'Armor</option>
					<option value="CREUSE">Creuse</option>
					<option value="DEUX-SEVRES">Deux Sèvres</option>
					<option value="DORDOGNE">Dordogne</option>
					<option value="DOUBS">Doubs</option>
					<option value="DROME">Drôme</option>
					<option value="ESSONNE">Essonne</option>
					<option value="EURE">Eure</option>
					<option value="EURE-ET-LOIR">Eure et Loir</option>
					<option value="FINISTERE">Finistère</option>
					<option value="GARD">Gard</option>
					<option value="GERS">Gers</option>
					<option value="GIRONDE">Gironde</option>
					<option value="HAUT-RHIN">Haut-Rhin</option>
					<option value="HAUTE-CORSE">Haute Corse</option>
					<option value="HAUTE-GARONNE">Haute Garonne</option>
					<option value="HAUTE-LOIRE">Haute Loire</option>
					<option value="HAUTE-MARNE">Haute Marne</option>
					<option value="HAUTE-SAONE">Haute Saône</option>
					<option value="HAUTE-SAVOIE">Haute Savoie</option>
					<option value="HAUTE-VIENNE">Haute Vienne</option>
					<option value="HAUTES-ALPES">Hautes Alpes</option>
					<option value="HAUTES-PYRENEES">Hautes Pyrénées</option>
					<option value="HAUTS-DE-SEINE">Hauts de Seine</option>
					<option value="HERAULT">Hérault</option>
					<option value="ILLE-ET-VILAINE">Ille et Vilaine</option>
					<option value="INDRE">Indre</option>
					<option value="INDRE-ET-LOIRE">Indre et Loire</option>
					<option value="ISERE">Isère</option>
					<option value="JURA">Jura</option>
					<option value="LANDES">Landes</option>
					<option value="LOIR-ET-CHER">Loir et Cher</option>
					<option value="LOIRE">Loire</option>
					<option value="LOIRE-ATLANTIQUE">Loire Atlantique</option>
					<option value="LOIRET">Loiret</option>
					<option value="LOT">Lot</option>
					<option value="LOT-ET-GARONNE">Lot et Garonne</option>
					<option value="LOZERE">Lozère</option>
					<option value="MAINE-ET-LOIRE">Maine et Loire</option>
					<option value="MANCHE">Manche</option>
					<option value="MARNE">Marne</option>
					<option value="MAYENNE">Mayenne</option>
					<option value="MEURTHE-ET-MOSELLE">Meurthe et Moselle</option>
					<option value="MEUSE">Meuse</option>
					<option value="MORBIHAN">Morbihan</option>
					<option value="MOSELLE">Moselle</option>
					<option value="NIEVRE">Nièvre</option>
					<option value="NORD">Nord</option>
					<option value="OISE">Oise</option>
					<option value="ORNE">Orne</option>
					<option value="PARIS">Paris</option>
					<option value="PAS-DE-CALAIS">Pas de Calais</option>
					<option value="PUY-DE-DOME">Puy de Dôme</option>
					<option value="PYRENEES-ATLANTIQUES">Pyrénées Atlantiques</option>
					<option value="PYRENEES-ORIENTALES">Pyrénées Orientales</option>
					<option value="RHONE">Rhône</option>
					<option value="SAONE-ET-LOIRE">Saône et Loire</option>
					<option value="SARTHE">Sarthe</option>
					<option value="SAVOIE">Savoie</option>
					<option value="SEINE-ET-MARNE">Seine et Marne</option>
					<option value="SEINE-MARITIME">Seine Maritime</option>
					<option value="SEINE-SAINT-DENIS">Seine Saint Denis</option>
					<option value="SOMME">Somme</option>
					<option value="TARN">Tarn</option>
					<option value="TARN-ET-GARONNE">Tarn et Garonne</option>
					<option value="TERRITOIRE DE BELFORT">Territoire de Belfort</option>
					<option value="VAL-D'OISE">Val D'Oise</option>
					<option value="VAL-DE-MARNE">Val de Marne</option>
					<option value="VAR">Var</option>
					<option value="VAUCLUSE">Vaucluse</option>
					<option value="VENDEE">Vendée</option>
					<option value="VIENNE">Vienne</option>
					<option value="VOSGES">Vosges</option>
					<option value="YONNE">Yonne</option>
					<option value="YVELINES">Yvelines</option>
				</options>
				<limitToOptions>true</limitToOptions>
			</parameter>
			<parameter name="refresh" type="integer" min="1" required="true" unit="min">
				<label>Fréquence de rafraichissement</label>
				<description>Période d'actualisation des données en minutes.</description>
				<default>1440</default>
			</parameter>
		</config-description>
	</thing-type>

J-N-K · 2023-04-29T13:54:03Z

I have fixed the validation message. Let's see what the ThingManager thinks is missing.

lolodomo · 2023-04-29T16:47:46Z

Here is the result:

18:34:46.410 [WARN ] [.core.thing.internal.ThingManagerImpl] - Failed to normalize configuration for thing 'meteoalerte:department:test': {thing/channel=Type description for meteoalerte:department:test:vague-submersion not found although we checked the presence before.}

This is the channel type description that is missing ?
That is wrong, meteoalerte:vague-submersion channel type has a description:

{
  "parameters": [],
  "parameterGroups": [],
  "description": "Niveau d'alerte aux vagues submersion",
  "label": "Vague Submersion",
  "category": "oh:meteoalerte:vague-submersion",
  "itemType": "Number",
  "kind": "STATE",
  "stateDescription": {
    "readOnly": true,
    "options": [
      {
        "value": "0",
        "label": "Vert"
      },
      {
        "value": "1",
        "label": "Jaune"
      },
      {
        "value": "2",
        "label": "Orange"
      },
      {
        "value": "3",
        "label": "Rouge"
      }
    ]
  },
  "tags": [],
  "UID": "meteoalerte:vague-submersion",
  "advanced": false
}

lolodomo · 2023-04-29T16:52:47Z

Maybe I got it: the original channel in org.openhab.core.thing.Thing.json has no description:

        {
          "uid": "meteoalerte:department:test:vague-submersion",
          "id": "vague-submersion",
          "channelTypeUID": "meteoalerte:alert-level",
          "itemType": "Number",
          "kind": "STATE",
          "label": "Etat Vague Submersion",
          "defaultTags": [],
          "properties": {},
          "configuration": {}
        },

Just because the original channel type (meteoalerte:alert-level) had no description.

I believe it should not trigger a warning when migrating to a new channel type having a description.

lolodomo · 2023-04-29T16:56:16Z

And the log is about normalizing configuration while it seems to have nothing to do with configuration, neither thing configuration nor channel configuration.

lolodomo · 2023-04-30T07:20:17Z

We can also notice that the migration changes the order of channels for the thing. I am going to create an issue but I already imagine it is not easy to fix.

I also compared my migrated thing with a new thing created with MainUI. The only differences I can find (in my specific case) were already discussed:

autoUpdatePolicy field only present in migrated things
Different format for configuration parameter value (1440 vs 1440.0)
Channels order

J-N-K · 2023-04-30T07:36:17Z

Channel ordering is just cosmetics. There is no functional difference.
I just checked that if you add a thing from the UI and save it, then the auto-update policy is not set. If you change the channel configuration (e.g. change the label), then it is set to DEFAULT after saving the channel. The reason is that the de-serialized thing has the auto-update policy set (to DEFAULT) and on serialization this is written to the database.
1440/1440.0 in the database makes no difference. It is automatically normalized during thing initialization.

The only issue I see is the warning for the missing channel type. But that is not related to the thing updates.

J-N-K · 2023-04-30T08:46:17Z

Is that the message AFTER or BEFORE the migration? If it is AFTER the migration then something is wrong, because there should not be an "old" channel in the checking. If it is before, it's difficult to suppress, because we would need to know before the upgrade if the channel will be removed/replaced. Suppressing it all the time would hide "real issues".

lolodomo · 2023-04-30T09:08:19Z

Where is stored the new storage ? I don't find it in userdata/jsondb. What is its name ?
Maybe this file is not created in my environment ?

J-N-K · 2023-04-30T10:00:42Z

The updated thing is stored in the usual JSON database, there is no additional storage for that. The update process should remove the old and put the updated thing to storage. That's why I am wondering why the check is performed for the "old" channel-type after the upgrade.

lolodomo · 2023-04-30T10:07:49Z

The updated thing is stored in the usual JSON database, there is no additional storage for that.

Ah ok.

The update process should remove the old and put the updated thing to storage. That's why I am wondering why the check is performed for the "old" channel-type after the upgrade.

As you previously said, we don't know if it is a log after or before the upgrade. Unfortunately, the log mentions the channel UID but not the channel type UID.
Do you want I enable some DEBUG logs to help determining that ? What bundles ?

Signed-off-by: Jan N. Klug <[email protected]>

J-N-K · 2023-04-30T11:28:53Z

I have added the missing description UID to the validation message. You could set a breakpoint to where the message is created (l. 680) and check what actually is in the channel type registry at that point.

lolodomo · 2023-05-01T08:28:46Z

Ok, here is with the new log message:

10:24:23.819 [INFO ] [.core.thing.internal.ThingManagerImpl] - Updating 'meteoalerte:department:test' from version 0 to 1
10:24:23.853 [WARN ] [.core.thing.internal.ThingManagerImpl] - Failed to normalize configuration for thing 'meteoalerte:department:test': {thing/channel=Type description meteoalerte:alert-level for meteoalerte:department:test:vague-submersion not found although we checked the presence before.}

So the warning is about channel type "meteoalerte:alert-level" which is in fact the old and removed channel type.
If the message is logged after the upgrade, there is a problem.
Edit: and if logged before the upgrade, this is a log that should normally not be logged in the context of an upgrade.
WDYT ?

J-N-K · 2023-05-01T08:55:56Z

Yes, that's an issue. If you look at the database: What is in there for this channel?

lolodomo · 2023-05-01T09:06:19Z

Before upgrade:

        {
          "uid": "meteoalerte:department:test:vague-submersion",
          "id": "vague-submersion",
          "channelTypeUID": "meteoalerte:alert-level",
          "itemType": "Number",
          "kind": "STATE",
          "label": "Etat Vague Submersion",
          "defaultTags": [],
          "properties": {},
          "configuration": {}
        },

and after upgrade:

        {
          "uid": "meteoalerte:department:test:vague-submersion",
          "id": "vague-submersion",
          "channelTypeUID": "meteoalerte:vague-submersion",
          "itemType": "Number",
          "kind": "STATE",
          "label": "Vague Submersion",
          "description": "Niveau d\u0027alerte aux vagues submersion",
          "defaultTags": [],
          "properties": {},
          "configuration": {}
        },

J-N-K · 2023-05-01T09:12:36Z

That looks good.

Can you try adding unique identifiers (like prefixing the message with 1, 2, 3 or the line-number) to the log messages in l. 409, 415 and 923? I would like to see where this really comes from.

lolodomo · 2023-05-01T09:53:18Z

The log is coming from line 409, so when normalizing the old thing.
Not a surprise because the old channel type is now missing.
The log is just inappropriate in this case but maybe appropriate for other update cases ?
Maybe the level should be reduced to DEBUG ?

J-N-K · 2023-05-01T10:23:50Z

Yes, we can probably reduce that to DEBUG. The reason it was introduced at all in #3157 was to provide consistent, normalized data in the ThingUpdatedEvent, it has no effect on thing itself.

Signed-off-by: Jan N. Klug <[email protected]>

lolodomo · 2023-05-01T10:58:57Z

Fine, thank you @J-N-K , the warning disappeared.

Regarding the remaining warning:

12:51:02.685 [WARN ] [.core.thing.internal.ThingManagerImpl] - Channel types or config descriptions for thing 'meteoalerte:department:test' are missing in the respective registry for more than 120s. This should be fixed in the binding.

the last part of the message (This should be fixed in the binding) annoys me as in my current case, there is nothing to fix in the binding.
WDYT ?

J-N-K · 2023-05-01T11:54:44Z

Can you suggest a better wording? It should be clear that if this happens not immediately after an upgrade that is a bug. Unfortunately we can't detect whether this is the first startup after an upgrade or just a restart.

lolodomo · 2023-05-01T14:34:05Z

Something like that ? "In case this is not triggered during a thing upgrade, it should be fixed in the binding."

lolodomo · 2023-05-02T22:32:58Z

@J-N-K : is my proposal to update the log message ok for you?

@openhab/core-maintainers : it would be cool to review & merge this PR quickly before we merge more and more add-ons PR including thing upgrade instructions.

Signed-off-by: Jan N. Klug <[email protected]>

lolodomo · 2023-05-03T11:18:35Z

Thank you @J-N-K

kaikreuzer

lgtm, thanks.

* Improve thing updates Signed-off-by: Jan N. Klug <[email protected]> GitOrigin-RevId: 960cf0c

J-N-K added the bug An unexpected problem or unintended behavior of the Core label Apr 24, 2023

lolodomo mentioned this pull request Apr 29, 2023

[Meteoalerte] Add an IconProvider openhab/openhab-addons#14811

Merged

J-N-K marked this pull request as ready for review April 29, 2023 11:28

J-N-K requested a review from a team as a code owner April 29, 2023 11:28

lolodomo mentioned this pull request Apr 30, 2023

Thing upgrade gives different result than recreating thing #3569

Open

J-N-K added 2 commits April 30, 2023 13:13

Improve thing updates

7c128c7

Signed-off-by: Jan N. Klug <[email protected]>

Fix validation message

2845378

Signed-off-by: Jan N. Klug <[email protected]>

J-N-K force-pushed the feature-thingupdate branch from 9de9efd to 2845378 Compare April 30, 2023 11:13

Improve validation message

21da47e

Signed-off-by: Jan N. Klug <[email protected]>

lolodomo mentioned this pull request May 1, 2023

[easee] Enable write support for smartCharging channel openhab/openhab-addons#14866

Merged

Improve logging

4c3b6bf

Signed-off-by: Jan N. Klug <[email protected]>

improve log mesage

877c46e

Signed-off-by: Jan N. Klug <[email protected]>

This was referenced May 3, 2023

[openwebnet] Add a time stamp when an alarm zone event occurs openhab/openhab-addons#14819

Merged

[netatmo] Add Siren capability to Presence Outdoor Camera openhab/openhab-addons#14485

Merged

kaikreuzer approved these changes May 7, 2023

View reviewed changes

kaikreuzer merged commit 960cf0c into openhab:main May 7, 2023

kaikreuzer added this to the 4.0 milestone May 7, 2023

J-N-K deleted the feature-thingupdate branch May 7, 2023 18:51

splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023

Improve thing updates (openhab#3576)

adc6f49

* Improve thing updates Signed-off-by: Jan N. Klug <[email protected]> GitOrigin-RevId: 960cf0c

Improve thing updates #3576

Improve thing updates #3576

Conversation

J-N-K commented Apr 24, 2023

lolodomo commented Apr 29, 2023

lolodomo commented Apr 29, 2023 • edited Loading

lolodomo commented Apr 29, 2023

lolodomo commented Apr 29, 2023

J-N-K commented Apr 29, 2023

J-N-K commented Apr 29, 2023

lolodomo commented Apr 29, 2023

lolodomo commented Apr 29, 2023 • edited Loading

J-N-K commented Apr 29, 2023

lolodomo commented Apr 29, 2023

lolodomo commented Apr 29, 2023

J-N-K commented Apr 29, 2023

J-N-K commented Apr 29, 2023

lolodomo commented Apr 29, 2023

J-N-K commented Apr 29, 2023

lolodomo commented Apr 29, 2023 • edited Loading

lolodomo commented Apr 29, 2023

lolodomo commented Apr 29, 2023

lolodomo commented Apr 29, 2023 • edited Loading

J-N-K commented Apr 29, 2023

lolodomo commented Apr 29, 2023 • edited Loading

lolodomo commented Apr 29, 2023 • edited Loading

lolodomo commented Apr 29, 2023

lolodomo commented Apr 30, 2023 • edited Loading

J-N-K commented Apr 30, 2023

J-N-K commented Apr 30, 2023

lolodomo commented Apr 30, 2023

J-N-K commented Apr 30, 2023

lolodomo commented Apr 30, 2023

J-N-K commented Apr 30, 2023

lolodomo commented May 1, 2023 • edited Loading

J-N-K commented May 1, 2023

lolodomo commented May 1, 2023

J-N-K commented May 1, 2023

lolodomo commented May 1, 2023 • edited Loading

J-N-K commented May 1, 2023

lolodomo commented May 1, 2023

J-N-K commented May 1, 2023

lolodomo commented May 1, 2023

lolodomo commented May 2, 2023 • edited Loading

lolodomo commented May 3, 2023

kaikreuzer left a comment

Choose a reason for hiding this comment

lolodomo commented Apr 29, 2023 •

edited

Loading

lolodomo commented Apr 29, 2023 •

edited

Loading

lolodomo commented Apr 29, 2023 •

edited

Loading

lolodomo commented Apr 29, 2023 •

edited

Loading

lolodomo commented Apr 29, 2023 •

edited

Loading

lolodomo commented Apr 29, 2023 •

edited

Loading

lolodomo commented Apr 30, 2023 •

edited

Loading

lolodomo commented May 1, 2023 •

edited

Loading

lolodomo commented May 1, 2023 •

edited

Loading

lolodomo commented May 2, 2023 •

edited

Loading