org.openhab.binding.zigbee
org.openhab.binding.zigbee copied to clipboard
openhab
Add missing UoMs
Thanks. Have you tested these changes and you confirm it works? I'm not completely sure that humidity is dimensionless either - it's a percentage so probably should be a PercentType don't you think?.
Tested it on my setup, works fine.
As far as I understand, PercentType is not a valid Item type (https://www.openhab.org/docs/concepts/items.html#items), so in this context using Number:Dimensionless and hardcoding the unit to % is the right approach. Maybe @kaikreuzer can confirm.
As far as I understand, PercentType is not a valid Item type
I think you are getting mixed up. Remember, bindings know nothing about items - we only care about States and PercentType is of course valid - as per the reference you provided.
Relative humidity is defined in percentage - I think this should be therefore a PercentType given it is a percentage, but maybe there's a better option?
Remember, bindings know nothing about items - we only care about
StatesandPercentTypeis of course valid - as per the reference you provided.
Yet the xml entry is called item-type.
Relative humidity is defined in percentage - I think this should be therefore a
PercentTypegiven it is a percentage, but maybe there's a better option?
Absolutely willing to go with the best option, but I see similar configuration also in other bindings (https://github.com/openhab/openhab-addons/blob/2.5.x/bundles/org.openhab.binding.openweathermap/src/main/resources/ESH-INF/thing/channel-types.xml#L232).
Let's wait for external help, I'll try PercentType locally in the meantime.
Yes, I was getting at the states that the binding uses - the two are obviously linked 👍
Really this should be a system channel so that everyone does the same thing rather than choosing a random binding to align with.
I'm a bit loath to change just for the sake of it - the best option really would be to have a "full set" of common system channels so that everyone is 100% aligned.
Anyway, I see the other point now - the Converters should be updated too, I'm currently testing a change. Don't merge yet.
Ok, but I don't want to randomly change this. Currently this will work fine and I don't really see the need to just change this.
What is the actual problem? If I search through the Addons repo, there is a split of different channel configrations for humidity and I would suggest that we should make a system channel, and change this once - not twice.
I should say - I'm happy enough with illuminance - that's clearer.
I see - I will remove the changes to RH and wait for a better implementation, then.
Rebased, found and fixed a few channels that were not correctly initialized.
Did these changes require you to redefine your items to the new type?
The channels need to be re-discovered, in order to show the new type. I don't have any manually-defined item.
The Things linked to those channels will keep on working without changes (and without UoM) until their type is properly defined, due to automagic conversion.
Thanks. That's what I thought. I think we should park this for now and possibly add it in the OH3 branch which should be coming soon. Otherwise we break everyones system and require them to rediscover everything.
Fine for me.
On a side note, I've checked multiple bindings dealing with humidity, and all of them are using Number:Dimensionless with a fixed % unit, so I guess until something major changes, that's the way to define relative humidity :)
Thanks. Yes, I also did a search so I concur. I still think it would be good to add this to system channels so I might look at that for OH3.
@giannello I've rebased this as we discussed a while back, and we now have some merge conflicts. Please can you take a look and update accordingly. Thanks.
There is actually a system channel for humidity in percentage: https://www.openhab.org/docs/developer/bindings/thing-xml.html#system-state-channel-types system.atmospheric-humidity
Hi all, what is needed to finish this out? I also have a local commit changing relative humidity to Number:Dimensionless, but figure we should merge this instead. I can chase down conflicts as long as we all agree on the path forward.
Should we maintain our own channel type for humidity and just change the definition (and converter) to use Number:Dimensionless, or adopt the system.atmospheric-humidity type? I anticipate there could be some debate over whether "indoor relative humidity" is the same as "atmospheric humidity".
I don't think it needs any major changes - just rebasing and resolving issues. The big problem is this will break everyones system which is less nice.
If we're going to break everything, we might as well use the system channel for humidity. Humidity is humidity - indoors and outdoors so there shouldn't be different channel types.
Oddly, there is a system type for temperature but it explicitly says "Outdoor Temperature". That's where my concern was rooted. Otherwise I would say wholesale we should adopt those types across the board including for temperature.
IMHO "channel types" should be abstract enough to be reusable - so "temperature" is a channel type. The "channel" can then be of type "temperature", and called "indoor temperature" or "outdoor temperature" - or "water temperature" - who knows. We shouldn't have types for every possible place someone might want to measure a temperature...
Anyway, that's a core problem. I added the ability to customise type labels a long time ago (in ESH) for the Zigbee and ZWave bindings to provide this separation between channel types and the actual channels, but I guess this is not being used.
For Zigbee I prefer not to stick with generic types. I don't care too much about the label being used in the type as that can be customised in the channel definition.
I opened a new PR in https://github.com/openhab/org.openhab.binding.zigbee/pull/765.
For now I pushed up the humidity channel as I have it currently implemented, but we can continue discussion there/change as necessary.