Saving thing channel configuration results in Thing reinitialisation

[cdjackson]

The problem

Bottom line: Thing channel configuration doesn't work and can't be used to get config channel updated to bindings.

If a Thing channel has configuration, when the config is saved, it does a PUT to save the complete thing rather than just updating the channel config. The channel config is updated as part of this, but now the thing is reinitialised rather than just updating the channel configuration.

It also seems that the PUT request to save the Thing is sent even when no configuration update to the channel is made - ie go to the Channel Details for a channel with config, then clock Done and we get the same PUT request that causes the thing to reinitialise. In this case (ie with no actual config change) when moving away from the Thing page, we don't get the error that the thing has changed.

One issue with this is because the thing is reinitialised, this can make it difficult to manage battery devices in zigbee since they need to be woken up when the channel config is updated. If the thing gets reinitialised, then firstly this configuration may not be transferred to the device, or if the binding does try and transfer it, the device may have gone to sleep since the user has no real way to know when this is happening as it's delayed from the point at which they save the UI data.

Expected behavior

The config for the channel should be updated separately, and at the time the user clicks Save/Done, and should only be updated if the channel configuration is changed. The thing shouldn't reinitialise.

Steps to reproduce

1. Edit a thing
2. Select a channel that has configuration
3. Select Channel Details
4. Change channel configuration
5. Click Done - at this point the thing will be reinitialised as there is a PUT to save the thing configuration. It should instead just save the channel config.

Your environment

runtimeInfo:
  version: 4.3.0
  buildString: "Build #4363"
locale: en-NZ
systemInfo:
  configFolder: /etc/openhab
  userdataFolder: /var/lib/openhab
  logFolder: /var/log/openhab
  javaVersion: 17.0.13
  javaVendor: Azul Systems, Inc.
  javaVendorVersion: Zulu17.54+21-CA
  osName: Linux
  osVersion: 6.1.0-25-amd64
  osArchitecture: amd64
  availableProcessors: 4
  freeMemory: 164329368
  totalMemory: 1269825536
  uptime: 229982
  startLevel: 70
addons:
  - automation-jsscripting
  - binding-astro
  - binding-boschshc
  - binding-ipcamera
  - binding-modbus
  - binding-speedtest
  - binding-surepetcare
  - binding-systeminfo
  - binding-wled
  - misc-openhabcloud
  - persistence-jdbc-h2
  - transformation-basicprofiles
  - ui-basic
clientInfo:
  device:
    ios: false
    android: false
    androidChrome: false
    desktop: true
    iphone: false
    ipod: false
    ipad: false
    edge: false
    ie: false
    firefox: false
    macos: true
    windows: false
    cordova: false
    phonegap: false
    electron: false
    nwjs: false
    webView: false
    webview: false
    standalone: false
    os: macos
    pixelRatio: 2
    prefersColorScheme: light
  isSecureContext: false
  locationbarVisible: true
  menubarVisible: true
  navigator:
    cookieEnabled: true
    deviceMemory: N/A
    hardwareConcurrency: 8
    language: en-US
    languages:
      - en-US
      - en
    onLine: true
    platform: MacIntel
  screen:
    width: 1680
    height: 1050
    colorDepth: 24
  support:
    touch: false
    pointerEvents: true
    observer: true
    passiveListener: true
    gestures: false
    intersectionObserver: true
  themeOptions:
    dark: dark
    filled: true
    pageTransitionAnimation: default
    bars: light
    homeNavbar: default
    homeBackground: default
    expandableCardAnimation: default
    blocklyRenderer: null
  userAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36
    (KHTML, like Gecko) Chrome/129.0.0.0 Safari/537.36
timestamp: 2024-11-20T00:42:23.862Z

Browser console

Browser network traffic

Additional information

[cdjackson]

This seems related to #2704 - at least it seems similar so maybe the same issue exists for the channel configuration.

[cdjackson]

@ghys I'm wondering if you might be able to provide a couple of pointers here...

I'm trying to work through this issue as it completely prevents channel config being saved. I can probably change the logic in the thing save code to detect if the channel is updated, and just update that (similar to the change that was done recently for #2704). However, I'm not completely sure why the PUT is being sent when clicking the DONE button when editing the channel - as I understand the code (and my understanding is not great here so maybe I'm wrong!) the concept is that the user can edit the channel configuration, they click DONE to go back to the Thing config page, and nothing is actually saved until you click the SAVE button on the Thing Edit page?

If that's the concept, then I'm trying to work out why there is a PUT sent to save the whole Thing when clicking DONE on the channel edit page. If I've followed the code correctly, the channel edit is in channel-edit.vue, and clicking DONE calls save() -:

    save () {
      let finalChannel = Object.assign({}, this.channel, {
        configuration: this.config
      })
      this.$f7route.route.context.finalChannel = finalChannel
      this.$f7router.back()
      // this.$emit('channelAddComplete', finalChannel)
      // this.$f7.view.main.emit('complete', finalChannel)
    }

This doesn't seem to explicitly send the PUT, so I guess some other code elsewhere is picking this up - do you have any pointers as to where to look?

If we can prevent the PUT being called here, then I'll take a look at the SAVE logic which I think should be (relatively!) straight forward.

Thanks

[ghys]

@cdjackson it's been a while so I had to wrap my head around this again, but I do remember it was indeed quite the can of worms...

I had to search finalChannel to understand it better:
https://github.com/search?q=repo%3Aopenhab%2Fopenhab-webui%20finalChannel&type=code

As you see it's pretty similar in add-channel.vue, edit-channel.vue and copy-channel.vue among others.
The challenge was mainly around those "editable" thing types (like MQTT Generic) where you can define your own list of channels. I believe the intent was even to be able to edit the channel's label & description in channel-edit.vue (not sure about the ID) - but the idea was eventually abandoned.

With that mind, when you change the channel list itself and not merely their configuration, then it was indeed better/simpler to just save the Thing when you're done making changes in order to:

1. refresh the list with fresh channel data by reloading the entire Thing from the API instead of dealing with e.g. inserting the newly created channel in the channel list directly
2. avoid further bugs in situations where you want to immediately link items, which takes you to another screen also fetching fresh channel data and therefore assuming the channel is already there and up-to-date...

When you click on the "Configure Channel" link in the list it runs this code:

openhab-webui/bundles/org.openhab.ui/web/src/components/thing/channel-link.vue

Lines 145 to 170 in ed54bf6

	configureChannel () {
	const self = this
	const path = 'channels/' + this.channelId + '/edit'
	this.$f7router.navigate({
	url: path,
	route: {
	component: ConfigureChannelPage,
	path,
	context: {
	operation: 'edit-channel'
	},
	on: {
	pageAfterOut (event, page) {
	const context = page.route.route.context
	const finalChannel = context.finalChannel
	if (finalChannel) {
	// replace the channel in-place
	const idx = self.thing.channels.findIndex((c) => c.uid === finalChannel.uid)
	self.$set(self.thing.channels, idx, finalChannel)
	self.$emit('channel-updated', true)
	} else {
	self.$emit('channel-updated', false)
	}
	}
	}
	}

As you see it registers a callback on "pageAfterOut" which checks whether there is a "finalChannel", and then performs both the in-place editing of the channels (not immediately sure why though because we're saving & reloading the Thing anyways) but also emits an event "channel-updated" which should be caught here:

openhab-webui/bundles/org.openhab.ui/web/src/pages/settings/things/thing-details.vue

Lines 729 to 732 in ed54bf6

	onChannelsUpdated (save) {
	if (save) this.save(true)
	if (!this.eventSource) this.startEventSource()
	},

So that boolean parameter is whether or not we'll save the Thing (the whole Thing, with this.save(saveThing: true) which does the PUT /rest/thing even when unnecessary).

Now to answer this issue, as said above the edit-channel.vue page cannot edit the channel's label & description anymore, only its configuration, so I suppose it would make sense not to save anything (and triggering some handlers server-side) if only the configuration has changed - just perform the in-place channel update in the callback code below and it should be enough.

Hope this helps and makes sense!

[cdjackson]

Thanks for the info @ghys - I'll take a further look at this over the coming days and see if I can come up with some improved logic that allows the config to be saved independently.

[ghys]

avoid further bugs in situations where you want to immediately link items, which takes you to another screen also fetching fresh channel data and therefore assuming the channel is already there and up-to-date...

I'll add to this - maybe one solution for this scenario is to simply mark the thing "dirty"and make sure it's saved there, just before navigating away. Some of this code is from 1999 (added here with the first commit in January 2000) and this "dirty" flag didn't exist then.

[cdjackson]

@ghys I wanted to clarify a point or two before I create a PR...

Now to answer this issue, as said above the edit-channel.vue page cannot edit the channel's label & description anymore, only its configuration

Currently, the UI appears to allow the label and description to be edited - are you saying that either the core, or elsewhere in the UI this data is ultimately not saved? If so, I guess the UI should make these read-only?

I'll add to this - maybe one solution for this scenario is to simply mark the thing "dirty"and make sure it's saved there, just before navigating away

I'm not sure I understand this point... Why mark the thing "dirty"? We don't want to save the thing - only the channel config. If we mark the thing dirty, then I guess we save the whole thing later.

I've spent a bit of time tonight making sure I understand how this works. What I think is an easy solution is in channel-edit, to do an isEqual check on the config in the save handler. If it's different, then we do a 'PUTon the channel configuration. I think we don't need to return thefinalChannel` in the context, and then the thing doesn't get marked dirty (and doesn't get saved).

I think this achieves is correct - the channel config is saved independently of the thing and I think it shouldn't mess with saving the thing itself. The only concern I think I have is around the saving of the channel label and description which as above is currently editable. If these don't need to be saved, then I think this works and I'm happy to provide a PR tomorrow.

[ghys]

Currently, the UI appears to allow the label and description to be edited

🤦 you're right. I won't lose any sleep if we decide we can't edit those after the channel is created, if it helps. You can always remove the channel and create it again.

I'll add to this - maybe one solution for this scenario is to simply mark the thing "dirty"and make sure it's saved there, just before navigating away

What I meant is something like:

Create a thing > Create a channel > Configure the channel & add it > Thing is actually saved > Link item(s)

would become:

Create a thing > Create a channel > Configure the channel & add it > Thing is NOT actually saved but is marked "dirty" > Try to link item(s) > You are offered the option to save the Thing to continue or cancel and stay on the Thing page

[cdjackson]

Ok, I think we're talking about slightly different use cases...

Create a thing > Create a channel > Configure the channel & add it > Thing is NOT actually saved but is marked "dirty" > Try to link item(s) > You are offered the option to save the Thing to continue or cancel and stay on the Thing page

In this case, we start with Create a thing and this all seems to be part of the creation process... In my use case, the configuration on the channel may be done many times after the thing/channel/link have been created - weeks after creation. As an example, there is reporting configuration that can be adjusted on a (zigbee) channel... The user may find that they only want to get reports when the temperature changes 0.1 degree - then a month or so later they might work out that the battery is being consumed too quickly ;) so they want to change this to 0.5 degrees.

This configuration is on the channel, so the user goes to the existing channel and clicks on the config channel link, then makes the change, and clicks Done. With my proposal, at this point the channel configuration would be PUT to the server and the thing is not marked dirty as the channel config is saved already.

For me this makes sense in my use case - I guess the question is what happens in your use case. I think it still works as I think the use case you mention where everything is done at once.

One other point - it's also possible (and useful) to configure channels that are not linked to an item. In your cases above you talk about "Create a channel" and "Configure the channel and add it" - in my understanding, the user can't add a channel can they? They can only link an item to a channel? Or is there another mechanism used by bindings for dynamic channel creation that I'm not familiar with? I can't see any way to add a channel in the UI so maybe you mean "create a link to an item"? As per the start of this paragraph, we can configure channels that are not linked to items, since the channel is what is being configured, and that is not dependant on the item... (I hope that all makes sense!).

[ghys]

you talk about "Create a channel" and "Configure the channel and add it" - in my understanding, the user can't add a channel can they?

Yes they can, for these "editable" thing types - the ones I know of are from the MQTT and HTTP bindings, maybe there's more.
(to be fair I didn't know about these until late)
Here's an example of users adding channels themselves: https://www.openhab.org/docs/tutorial/things_advanced.html#create-the-generic-mqtt-thing

The issue is that when you add, edit, or remove channels from those things, you modify the Thing itself - hence the HTTP request to save it - not only the channel configuration. And edit-channel.vue was meant to handle both the configuration of the channel and changing the channel itself.

What I proposed was:

• to defer the saving of the Thing when you add/edit/remove channels (not the configuration, the channels themselves) as currently the Thing is saved when you return from those screens
• to decide that we can't "edit" the channel itself - even for "editable" thing types - but only add or remove them - again for those "editable" thing types, and the edit-channel.vue page would just be a screen to change the configuration.

[cdjackson]

What I proposed was:

to defer the saving of the Thing when you add/edit/remove channels (not the configuration, the channels themselves) as currently the Thing is saved when you return from those screens

We could (presumably) save the channel and it's configuration, mark it dirty, and then in the "Save Thing" logic, detect that only the channel configuration had changed, and just make the call to save this. I think this is what you're suggesting?

I did have a look at that, and I'm sure it's possible, but it's more complicated. As you've already pointed out elsewhere I think, this is a bit messy as there are so many different calls for saving different parts of the thing - the big problem with saving the whole thing is that it causes the thing handler to be reinitialised which actually means that the config callbacks never get called when the config is updated.

IIRC when I looked at that option, the problem was that the code that saves the thing doesn't have access to the original channel config - that's only available in the channel edit code which is why I proposed to save the channel config there. Otherwise, in the "Save Thing" handler, we somehow need to differentiate between saving the channel (and hence the thing) or saving the channel config and at the moment we are returning the finalChannel and I think from that we can't make this distinction.

to decide that we can't "edit" the channel itself - even for "editable" thing types - but only add or remove them - again for those "editable" thing types, and the edit-channel.vue page would just be a screen to change the configuration.

I think this is in line with what I suggested - ie to make the name/description fields non-editable, and to save the configuration in edit-channel.vue when the user presses Done. I think this is cleaner - well - it's certainly simpler as that page only becomes an "edit channel configuration" page and we can simply save the channel config when it's Done. There's no complex logic required to work out what parts are dirty and therefore what calls to make to the core.