Thing upgrade creates a duplicate channel if channel already exists (corner case bug)

[theater]

Prerequisite: You have a "test version" of a binding as a jar which already has the new channel implemented but in the metadata has the old "thingTypeVersion". Your thing has been rediscovered with the test version and contains the new channel. In the upgrade instructions you have "add channel" instruction for the specified channel. This is usually a very corner case when the binding has been tested before the upgrade instructions and the thing versions have been changed.

Result: bundle org.openhab.core.thing:4.1.0.M3 (216)[org.openhab.core.thing.internal.ThingManagerImpl(207)] : The addThingHandlerFactory method has thrown an exception java.lang.IllegalArgumentException: Duplicate channels <THING ID>:<THE_NEW_CHANNEL_ID> at org.openhab.core.thing.util.ThingHelper.ensureUniqueChannels(ThingHelper.java:135) ~[?:?] at org.openhab.core.thing.util.ThingHelper.ensureUniqueChannels(ThingHelper.java:127) ~[?:?] at org.openhab.core.thing.util.ThingHelper.ensureUniqueChannels(ThingHelper.java:123) ~[?:?]

Potential solution. I would suggest the following change in the UpdateChannelInstructionImpl, i.e. remove the channel if it exists even though you're in the update channel case: private void doChannel(Thing thing, ThingBuilder thingBuilder, ChannelUID affectedChannelUid) { Channel oldChannel = thing.getChannel(affectedChannelUid); if (removeOldChannel || oldChannel != null) { thingBuilder.withoutChannel(affectedChannelUid); } .....

[J-N-K]

I don't think this is a bug. It's more an issue with the binding development. How did the channel get into the thing without incrementing the thingTypeVersion?

[theater]

During development... I guess it affects only people who test dev versions before the instructions are implemented. That's why i say it's very corner-casish... :)

On Sun, Nov 12, 2023, 7:58 PM J-N-K @.***> wrote:

I don't think this is a bug. It's more an issue with the binding development. How did the channel get into the thing without incrementing the thingTypeVersion?

— Reply to this email directly, view it on GitHub https://github.com/openhab/openhab-core/issues/3864#issuecomment-1807197132, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB7Q3LWOP6KNXWTRZVND5G3YEEE3XAVCNFSM6AAAAAA66MI7Z6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBXGE4TOMJTGI . You are receiving this because you authored the thread.Message ID: @.***>

[lolodomo]

The developer must provide ugrapde instructions. This is one point we (try to) check during review process.

[theater]

Yes. I don't argue about that. I just say that I usually implement something, give it to people to test (especially if I don't own the box/iron) and then do the details like these instructions. This lead to the requirement to delete/recreate the thing. As said.. it's a corner case issue which affects only the willing to test users during the "before PR" phase.

[lolodomo]

@J-N-K : I would suggest to close this issue.