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

thing upgrades: missing support to add/change/remove channel properties #3610

Closed
alexf2015 opened this issue May 13, 2023 · 10 comments · Fixed by #3612
Closed

thing upgrades: missing support to add/change/remove channel properties #3610

alexf2015 opened this issue May 13, 2023 · 10 comments · Fixed by #3612
Labels
bug An unexpected problem or unintended behavior of the Core

Comments

@alexf2015
Copy link

alexf2015 commented May 13, 2023

I am using channel properties and as part of a change I also need to add some properties as part of a migration. See #14866 for details. Changing channel type worked fine for me but this is only a part of the migration. Please add support for this.

@alexf2015 alexf2015 added the bug An unexpected problem or unintended behavior of the Core label May 13, 2023
@J-N-K
Copy link
Member

J-N-K commented May 13, 2023

Channels don't have properties:

    <xs:complexType name="channelType">
        <xs:sequence>
            <xs:element name="item-type" type="xs:string" minOccurs="0"/>
            <xs:element name="kind" type="xs:string" minOccurs="0"/>
            <xs:element name="label" type="xs:string"/>
            <xs:element name="description" type="xs:string" minOccurs="0"/>
            <xs:element name="category" type="xs:string" minOccurs="0"/>
            <xs:element name="tags" type="thing-description:tags" minOccurs="0"/>
            <xs:element name="state" type="thing-description:state" minOccurs="0"/>
            <xs:element name="command" type="thing-description:command" minOccurs="0"/>
            <xs:element name="event" type="thing-description:event" minOccurs="0"/>
            <xs:element name="autoUpdatePolicy" type="thing-description:auto-update-policy" minOccurs="0"/>
            <xs:choice minOccurs="0">
                <xs:element name="config-description" type="config-description:configDescription"/>
                <xs:element name="config-description-ref" type="config-description:configDescriptionRef"/>
            </xs:choice>
        </xs:sequence>
        <xs:attribute name="id" type="config-description:idRestrictionPattern" use="required"/>
        <xs:attribute name="advanced" type="xs:boolean" default="false" use="optional"/>
        <xs:attribute name="system" type="xs:boolean" default="false" use="optional"/>
    </xs:complexType>

@lolodomo
Copy link
Contributor

Please update the issue title as it does not concern the upgradetool.

@J-N-K J-N-K changed the title upgradetool: missing support to add/change/remove channel properties thing upgrades: missing support to add/change/remove channel properties May 13, 2023
@J-N-K J-N-K removed the bug An unexpected problem or unintended behavior of the Core label May 13, 2023
@J-N-K
Copy link
Member

J-N-K commented May 13, 2023

As I already said: I would vote for won't fix because properties are not part of channel-types.

@lolodomo
Copy link
Contributor

I was also a little surprised to discover properties on channel types.
Properties are for thing types.
On channel type, you can have configuration parameters.
I would be curious how many bindings defined properties on channel types. But not easy to check with a simple grep.

@J-N-K
Copy link
Member

J-N-K commented May 13, 2023

If SAT works ad designed it should trigger a SAT warning that the XML is invalid. The interesting thing is that the Channel class defines properties, that's why it works.

@lolodomo
Copy link
Contributor

Yes, and in fact @alexf2015 defined properties on channels, not channel types:

		<channels>
			<channel id="lockCablePermanently" typeId="rwtype-switch">
				<label>Lock Cable Permanently</label>
				<description>Lock Cable Permanently status of the wallbox.</description>
				<properties>
					<property name="writeCommand">ChangeConfiguration</property>
					<property name="validationExpression">.*</property>
				</properties>
			</channel>

When updating such channel, the properties are apparently lost, that is the object of the issue.

@jlaur
Copy link
Contributor

jlaur commented May 13, 2023

So this is the relevant part of the XSD (declaring properties for a channel):

<xs:complexType name="channel">
    <xs:sequence>
        <xs:element name="label" type="xs:string" minOccurs="0"/>
        <xs:element name="description" type="xs:string" minOccurs="0"/>
        <xs:element name="properties" type="thing-description:properties" minOccurs="0"/>
        <xs:element name="autoUpdatePolicy" type="thing-description:auto-update-policy" minOccurs="0"/>
    </xs:sequence>
    <xs:attribute name="id" type="config-description:idRestrictionPattern" use="required"/>
    <xs:attribute name="typeId" type="thing-description:namespaceIdRestrictionPattern" use="required"/>
</xs:complexType>

@lolodomo
Copy link
Contributor

lolodomo commented May 13, 2023

That means when updating a channel, there are some data to keep (label, description, properties, autoUpdatePolicy) to reuse them when inserting the new channel from the channel type ... if they were present.
I believe this is in fact a very similar issue as the one for label discussed in the other issue.

@J-N-K J-N-K added the bug An unexpected problem or unintended behavior of the Core label May 13, 2023
@alexf2015
Copy link
Author

I do not think that #3612 will fix my issue. I wanted to add two properties as part of the migration. If I understand #3612 correctly this will only nsure that already existing properties will not get lost during the update.

@openhab-bot
Copy link
Collaborator

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

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 the Core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants