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

Add restrictions for semantic point/property tags on channels #4615

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

andrewfg
Copy link
Contributor

@andrewfg andrewfg commented Feb 22, 2025

Add XSD schema restrictions for semantic tags on channel type definitions

See also #4616 #4619 #4622

Signed-off-by: Andrew Fiddian-Green [email protected]

Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg andrewfg requested a review from a team as a code owner February 22, 2025 14:38
@andrewfg andrewfg marked this pull request as draft February 22, 2025 14:38
Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg andrewfg marked this pull request as ready for review February 22, 2025 14:43
@jimtng
Copy link
Contributor

jimtng commented Feb 22, 2025

If we want to be even more precise, only allow one Point, and one Property. This would avoid having two Points / two properties.

furthermore, perhaps Property can only be added with a Point, not alone. I don't know how tricky this would be though.

@jimtng
Copy link
Contributor

jimtng commented Feb 22, 2025

The list of tags here can be updated by generateTagClasses.groovy

@andrewfg
Copy link
Contributor Author

andrewfg commented Feb 22, 2025

If we want to be even more precise, only allow one Point, and one Property. This would avoid having two Points / two properties.

Fully agreed. I just can't figure out the XSD syntax to specify that. Problem is that one could split the allowed values into two separate enums, but it is not allowed to have two xml elements of the same name 'tag' each being differently linked to those different enums. (If there are any XSD gurus please speak up).


EDIT:

After much checking with online XML validators I can confirm that the current OH schema for semantic tags is not really compliant, and cannot disambiguate single usage of property vs. point tags.

Given the following two enumerations for property-tags and point-tags..

	<xs:simpleType name="property-tags">
		<xs:restriction base="xs:string">
			<!-- Allowed Property Tag Values -->
			<xs:enumeration value="Temperature" />
			<xs:enumeration value="Light" />
			<xs:enumeration value="ColorTemperature" />
			<xs:enumeration value="Humidity" />
			<xs:enumeration value="Presence" />
			<xs:enumeration value="Pressure" />
			<xs:enumeration value="Smoke" />
			<xs:enumeration value="Noise" />
			<xs:enumeration value="Rain" />
			<xs:enumeration value="Wind" />
			<xs:enumeration value="Water" />
			<xs:enumeration value="CO2" />
			<xs:enumeration value="CO" />
			<xs:enumeration value="Energy" />
			<xs:enumeration value="Power" />
			<xs:enumeration value="Voltage" />
			<xs:enumeration value="Current" />
			<xs:enumeration value="Frequency" />
			<xs:enumeration value="Gas" />
			<xs:enumeration value="SoundVolume" />
			<xs:enumeration value="Oil" />
			<xs:enumeration value="Duration" />
			<xs:enumeration value="Level" />
			<xs:enumeration value="Opening" />
			<xs:enumeration value="Timestamp" />
			<xs:enumeration value="Ultraviolet" />
			<xs:enumeration value="Vibration" />
		</xs:restriction>
	</xs:simpleType>

	<xs:simpleType name="point-tags">
		<xs:restriction base="xs:string">
			<!-- Allowed Point Tag Values -->
			<xs:enumeration value="Alarm" />
			<xs:enumeration value="Control" />
			<xs:enumeration value="Switch" />
			<xs:enumeration value="Measurement" />
			<xs:enumeration value="Setpoint" />
			<xs:enumeration value="Status" />
			<xs:enumeration value="LowBattery" />
			<xs:enumeration value="OpenLevel" />
			<xs:enumeration value="OpenState" />
			<xs:enumeration value="Tampered" />
			<xs:enumeration value="Tilt" />
		</xs:restriction>
	</xs:simpleType>

Then this is NOT allowed..

	<xs:complexType name="tags">
		<xs:sequence>
			<xs:element name="tag" type="point-tags" minOccurs="0" maxOccurs="1" />
			<xs:element name="tag" type="property-tags" minOccurs="0" maxOccurs="1" />
		</xs:sequence>
	</xs:complexType>

Whereas this IS allowed..

	<xs:complexType name="tags">
		<xs:sequence>
			<xs:element name="point-tag" type="point-tags" minOccurs="0" maxOccurs="1" />
			<xs:element name="property-tag" type="property-tags" minOccurs="0" maxOccurs="1" />
		</xs:sequence>
	</xs:complexType>

The problem is the inabilility to disabiguate two elements of the same name 'tag' that have different allowed value sets.

So we could either ..

  1. Change the OH schema to allow disambiguation of property and point tags. This would be a mega effort with potential risks of breaking changes, or
  2. Do NOT attempt to disabiguate property and point tags, and allow the XSD schema to validate both possible values. We could eventually add a run-time checker that logs a warning when the wrong combination of mixed non-disabiguated property and point tags is encountered. This is probably less effort and less risk.

Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg
Copy link
Contributor Author

Please see #4617 also.

@andrewfg andrewfg changed the title XSD Add restrictions for tags XSD Add restrictions for semantic tags on channel type definitions Feb 23, 2025
@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/openhab-5-semantic-model-proposal/162526/96

@jimtng
Copy link
Contributor

jimtng commented Feb 23, 2025

@jimtng
Copy link
Contributor

jimtng commented Feb 23, 2025

Also another idea... probably crazy, we can probably write a validation script before build time.

@andrewfg
Copy link
Contributor Author

Maybe this can help?

Uh. No. The accepted answer is as follows; and in our case we can neither give the elements different names, nor put them in different contexts. Unless we want to completely re-engineer the schema (which I do not not..)

You can't declare two elements with the same name with different types in the same context, but I think I understand what you want to do.

If you really had elements with very different contents, it would make sense to create two types (and it would also make sense for them to have different names or to at least occur in another context).

@andrewfg
Copy link
Contributor Author

andrewfg commented Feb 23, 2025

Also another idea... probably crazy, we can probably write a validation script before build time.

Not so crazy. I remember we did something similar with the addon suggestion finder code. AFAIR someone wrote a script that pools all the XML from all of the individual addon.xml files (all addons in the OH main distro) into a single addons.xml file, and I wrote a component that parses that file and logs anything bad. I think it would probably be trivial to extend that mechanism.

EDIT: here..

EDIT2: unfortunately after checking that code further I can see that the addonInfo.xml does not contain the channel type information, so we cannot do our newly desired analysis in that component. However it could show a potential way towards implementing something similar for this case.

Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg andrewfg marked this pull request as draft February 24, 2025 07:56
Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg andrewfg marked this pull request as ready for review February 24, 2025 10:57
@andrewfg
Copy link
Contributor Author

Also another idea... probably crazy, we can probably write a validation script before build time.

@jimtng I don't think my idea for a validator (above) is going to work. So do you have any further thoughts about your pre-build script idea? .. Another idea is to check the tags when the channel class is instantiated; but this would be problematic since by the time of instantiation the user may well have added custom (e.g. location) tags..

@andrewfg
Copy link
Contributor Author

Note: The actual tag values are still under discussion in #4616, #4617, #4619, #4622 .. so the XSD enumeration restriction in the thing-types.xml schema will need to be finally updated to match those PRs.

@jimtng
Copy link
Contributor

jimtng commented Feb 28, 2025

The generator script could also update this xsd file - after this PR has been merged, so it has the correct section to insert into.

@andrewfg
Copy link
Contributor Author

andrewfg commented Feb 28, 2025

The generator script could also update this xsd file

Wow that's cool. But I may need your help to write that script, as it is a language I have not yet studied.

@jimtng
Copy link
Contributor

jimtng commented Feb 28, 2025

The generator script could also update this xsd file

Wow that's cool. But I may need your help to write that script, as ot is a language I have not yet studied.

Sure, I'll be happy to.

@andrewfg andrewfg changed the title XSD Add restrictions for semantic tags on channel type definitions Add restrictions for semantic tags on channel type definitions Feb 28, 2025
@jimtng
Copy link
Contributor

jimtng commented Mar 1, 2025

Does the xsd file not supposed to have one blank line at the end?

@andrewfg
Copy link
Contributor Author

andrewfg commented Mar 1, 2025

I guess it should have a return at the end. Albeit that the current version does not have one.

@jimtng
Copy link
Contributor

jimtng commented Mar 1, 2025

Weird, my build is complaining about it and running spotless:apply added the missing blank line

Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg andrewfg changed the title Add restrictions for semantic tags on channel type definitions Add restrictions for semantic tags on channels Mar 1, 2025
Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg andrewfg changed the title Add restrictions for semantic tags on channels Add restrictions for semantic point/property tags on channels Mar 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants