这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@ArjenKorevaar
Copy link
Member

@ArjenKorevaar ArjenKorevaar commented Aug 1, 2023

Fixes #15346

I would still like to keep the entire set of possible message id's from the OpenTherm specification, even though some message id's or some of their bits are not yet in use. This makes it easier to compare the bindings implementation to the OpenTherm specification documentation and implement future versions.

Signed-off-by: Arjen Korevaar <a.korevaar@mephix.com>
@ArjenKorevaar ArjenKorevaar requested a review from andrewfg as a code owner August 1, 2023 17:13
@jlaur jlaur changed the title Removed invalid characters from channel UIDs Remove invalid characters from channel UIDs Aug 1, 2023
@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/opentherm-gateway-binding/39160/336

@wborn wborn changed the title Remove invalid characters from channel UIDs [openthermgateway] Remove invalid characters from channel UIDs Aug 1, 2023
@wborn wborn added the bug An unexpected problem or unintended behavior of an add-on label Aug 1, 2023
@jlaur
Copy link
Contributor

jlaur commented Aug 1, 2023

@ArjenKorevaar - thanks for the fix! Is it correct that these channels are highly unlikely to be linked to any items because they are only reserved for future use?

@ArjenKorevaar
Copy link
Member Author

ArjenKorevaar commented Aug 1, 2023

@ArjenKorevaar - thanks for the fix! Is it correct that these channels are highly unlikely to be linked to any items because they are only reserved for future use?

That is correct. Allthough, the fact that this user reports this issue indicates that his particular device does in fact sends messages using one of these id's. I can't link them to either 2.1 or 3.0 specifications of OpenTherm, so they are probably some OEM or irrelevant messages. But yes, they are highly unlikely to be linked to any items.

I'm fine with it either way: I'll leave them in for reference and completeness compared to the OpenTherm specification, or remove them as they probably won't be linked to items in the current implementation.

@jlaur
Copy link
Contributor

jlaur commented Aug 1, 2023

That is correct. Allthough, the fact that this user reports this issue indicates that his particular device does in fact sends messages using one of these id's. I can't link them to either 2.1 or 3.0 specifications of OpenTherm, so they are probably some OEM or irrelevant messages. So yes, they are highly unlikely to be linked to any items.

I'm fine with it either way: I'll leave them in for reference and completeness compared to the OpenTherm specification, or remove them as they probably won't be linked to items in the current implementation.

I was mostly asking to assess if update instructions are needed. But since they have invalid id's, probably it would be a problem providing instructions for removing those channels. Since they are probably not used, this doesn't seem worth any more considerations.

I will leave it to you and @andrewfg to decide if they should be kept. Are they marked as advanced, or will they show in the UI always? And is it possible to actually use them to get access to undocumented data, or are they completely useless?

If they are completely useless, I would suggest to remove them. If they can be used by advanced users knowing what they are doing, I would at least mark them as advanced (because they are - and also to be hidden by default).

@ArjenKorevaar
Copy link
Member Author

The question whether or not these message id's are completely useless is a bit difficult to answer, since the OpenTherm specifications (although using the term 'open') are not publicly available. As you stated correctly, these channels are highly unlikely to be used since they are for all the documation that I have been able to aquire reserved for future use. But it's not impossible for particular devices to use them anyways.

My suggestion at this point: remove them from the binding. I expect the chances that a user uses any of these channels is close to zero (even the user that reported this issue doesn't use any of these channels). And if someone turns out to actually use them, I'm sure they would be able to reach out and allow us to properly name and support them.

Signed-off-by: Arjen Korevaar <a.korevaar@mephix.com>
@andrewfg
Copy link
Contributor

andrewfg commented Aug 2, 2023

FWIW the OpenTherm specification is here; which shows some hints about the possible uses of some reserved fields.

Opentherm Protocol v2-2.pdf

Copy link
Contributor

@andrewfg andrewfg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andrewfg andrewfg changed the title [openthermgateway] Remove invalid characters from channel UIDs [openthermgateway] Remove 'reserved' channels (with invalid channelUIDs) Aug 2, 2023
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll cherry-pick this to be included in 4.0.2.

@jlaur jlaur merged commit e057e79 into openhab:main Aug 2, 2023
@jlaur jlaur added this to the 4.1 milestone Aug 2, 2023
jlaur pushed a commit that referenced this pull request Aug 2, 2023
…Ds) (#15355)

* Removed invalid characters from channel UIDs
* Removed unused message ids

Signed-off-by: Arjen Korevaar <a.korevaar@mephix.com>
@jlaur jlaur added the backported A PR that has been cherry-picked to a patch release branch label Aug 2, 2023
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
…Ds) (openhab#15355)

* Removed invalid characters from channel UIDs
* Removed unused message ids

Signed-off-by: Arjen Korevaar <a.korevaar@mephix.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backported A PR that has been cherry-picked to a patch release branch bug An unexpected problem or unintended behavior of an add-on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[openthermgateway] warning that channel UID contains invalid characters

5 participants