Thing migration: channels order is lost

[lolodomo]

When migrating a thing for example by changing a channel type, the updated channel is added at the bottom of channels.
The initial order of channels (thought by the developer) is then lost.

[J-N-K]

This can't be solved because we don't have access to the channel ordering of the XML. It's also not an issue because it works fine the way it is now.

[lolodomo]

It is an issue because it introduces something unexpected.
But I can agree that it is not a critical issue.

Issue #3442 was created the last time it happened, that means it is something important for a part of users/developers.

[J-N-K]

There's an easy fix: delete the thing and re-create it.

[andrewfg]

can't be solved because we don't have access to the channel ordering of the XML

I am not 100% sure about your statement. With #3448 we (once more) have the channels in the order in which they were created via the XML. So before migrating the thing, one could cache the prior channel order, and when the channel is recreated, instead of adding at the end of the list, it can be re-injected into the new channel list at the correct position.

[J-N-K]

That was different, we just used a collection that preserves order.

Again: there is no reason why order is important.

[andrewfg]

there is no reason why order is important

IMHO there IS a reason: it is more user friendly in Main UI because the channels are shown in a logical order defined by the developer (probably more important channels appearing on top, and grouping similar channels together (e.g. battery-level and battery-low..)

[J-N-K]

Then let's revert the thing updates. If we force users to delete and re-add things, the very important order is ensured.

[lolodomo]

We are not going to revert but if the issue can't be fixed, this "limit" should be at least documented/explained.

In practice, the channels order is important mainly when a thing has many channels.

[J-N-K]

In general, since most changes still require user-interaction (e.g. change of linked item, link an item, remove a broken link), changes to existing thing-types, especially updating, not so much adding channels) should still be made with some thought if the change is really worth it.

[andrewfg]

^
I will write some demo code to demonstrate how I would do this inside a specific binding. I am not sure if it would make sense for such code to be integrated into core or not. But have a look at the demo, and then you can decide.

[andrewfg]

Here you go..

    /**
     * Take a random list of new channels as input, and create a new list of channels sorted by the order of their
     * position in the thing's original list of channels. If the input list contains new channels which are not in the
     * original list then they will be at the end of the list.
     *
     * @param randomChannelList a list of channels in random order.
     * @return a channel list sorted according to position in the thing's existing channel list.
     */
    private List<Channel> sortChannelList(List<Channel> randomChannelList) {
        // create a lookup with the thing's existing channels
        List<String> lookup = thing.getChannels().stream().map(c -> c.getUID().getId()).collect(Collectors.toList());
        // create a copy of the randomChannelList, and sort it
        List<Channel> sortedChannelList = new ArrayList<>(randomChannelList);
        sortedChannelList.sort(Comparator.comparing(c -> {
            int index = lookup.indexOf(c.getUID().getId());
            return index < 0 ? Integer.MAX_VALUE : index;
        }));
        return sortedChannelList;
    }

[J-N-K]

I don't understand what the benefit of that is. Usually if a thing is changed, channels are removed (doesn't affect channel order) or added (will be added at the end). Updating the channel-type should be avoided id possible, they are in nearly all cases breaking (because except in cases where bindig-specific channel-types are replaced by system-channel-types they usually require changes to the linked thing).

Also it doesn't work that way because the channel list is internal to the ThingBuilder and can't be accessed while processing the update instructions.

[andrewfg]

it doesn't work that way because the channel list is internal to the ThingBuilder and can't be accessed while processing the update instructions.

This is indeed correct. If you read my message more carefully, I said that I would provide demo code for how I would implement it in a specific binding.

[lolodomo]

In general, since most changes still require user-interaction (e.g. change of linked item, link an item, remove a broken link), changes to existing thing-types, especially updating, not so much adding channels) should still be made with some thought if the change is really worth it.

Looking at the content of DB file org.openhab.core.thing.link.ItemChannelLink.json and considering the results of my test with the meteoalerte binding, it looks like when a channel type is updated for one channel but the channel UID is unchanged, the link to the item is still valid after the upgrade. The channel type UID is not stored in the DB file, only the channel UID.
Of course, there are cases for which it will not work, in particular if the updated channel type has a different item type. In that case, the user will have to link the channel to another item after the upgrade. But this is certainly something to avoid by developers. In that case, a new channel should be created.

[openhab-bot]

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/come-back-and-learn-how-to-use-file-based-configuration/147067/15