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

[enocean] Improve capability listing to align with EEP documentation #17522

Merged
merged 1 commit into from
Oct 13, 2024

Conversation

dilyanpalauzov
Copy link
Contributor

This updates the descriptions for EnOcean devices, based on the table on page 132 of https://www.enocean-alliance.org/wp-content/uploads/2017/05/EnOcean_Equipment_Profiles_EEP_v2.6.7_public.pdf, whether the properties from the subject are present.

This makes the device types more distinguishable from each other, and eases the users.

In particular it replaces “dimming” with “configurable dimming” where appropriate.

I have not checked if the class implementation does support these properties (match the table): in case of D2_01_0A I have removed “energy measurement” from the description.

Related to #17429 and #17450.

@dilyanpalauzov
Copy link
Contributor Author

receivingEEPId and sendingEEPId are kept identical. I do not know if this is correct.

@lovery
Copy link
Contributor

lovery commented Oct 8, 2024

Can you also sign your commit - this will fix the DCO check?

Do you know if power failure detection is implemented?

By the way the pilot wire mode for D2_01_0C is still not merged because it needs testing - #17450 if you have such a device and you have time to test the implementation I will appreciate it.

@dilyanpalauzov
Copy link
Contributor Author

I am not going to sign my commits.

Do you know if power failure detection is implemented?

No, I do not know, neither for Configurable Dimming, but it does not matter here. This changeset updates the device descriptions, so users can select based on what they have bought. I could however update README to state that the receivingEEPId and sendingEEPId descriptions include Configurable dimming and Power failure detection properties, which are supported by the device, but not implemented in OH.

I have no D2_01_0C device.

Do you know if receivingEEPId and sendingEEPId should be identical or distinct?

@dilyanpalauzov
Copy link
Contributor Author

Do you know if receivingEEPId and sendingEEPId should be identical or distinct?

I mean their descriptions, as changed here.

@lovery
Copy link
Contributor

lovery commented Oct 8, 2024

@dilyanpalauzov I think they are identical everywhere.

@dilyanpalauzov dilyanpalauzov changed the title [enocean] Spell when devices support power measurement, power failure detection, configurable dimming, pilot wire. [enocean] Spell when devices support power measurement, power failure detection, configurable dimming, pilot wire mode, without local control Oct 8, 2024
@dilyanpalauzov dilyanpalauzov changed the title [enocean] Spell when devices support power measurement, power failure detection, configurable dimming, pilot wire mode, without local control [enocean] Spell when devices support power measurement, power failure detection, configurable dimming, pilot wire mode, do not have local control Oct 8, 2024
… detection

configurable dimming or pilot wire, do not have local control.
@lsiepel
Copy link
Contributor

lsiepel commented Oct 11, 2024

Besides the last comment that's waiting for @lovery comment, i wonder if we can merge this or if it needs to wait before the pilot wire PR is merged. As this update states pilot wire capability's

@lsiepel lsiepel changed the title [enocean] Spell when devices support power measurement, power failure detection, configurable dimming, pilot wire mode, do not have local control [enocean] Improve capability listing to align with EEP documentation Oct 11, 2024
@lsiepel lsiepel added the bug An unexpected problem or unintended behavior of an add-on label Oct 11, 2024
@dilyanpalauzov
Copy link
Contributor Author

This improves the descriptions of the devices, which otherwise had very identical spelling, and were hard to distinguish. Whether the listed features are supported by OpenHAB is secondary.

@lovery
Copy link
Contributor

lovery commented Oct 11, 2024

@lsiepel - it is only label change, it won't break any functionality and I am not feeling really competent to say how it should be because I don't know if the description in the label connected to EEP in the OH interface should be according to the EnOcean specification or based on the implemented features by the OH. For example:
...option.D2_01_0B = Switching, power failure detection with power and energy measurement (EEP: D2_01_0B) - based on the EnOcean specification or if it has to be based on the supported features of the OH power failure detection shouldn't be added.
...option.D2_01_0C = Switching, pilot wire mode with power and energy measurement (EEP: D2_01_0C) - based on the EnOcean specification or if it has to be based on the supported features of the OH pilot wire mode shouldn't be added.

@dilyanpalauzov there is no need to update the README.txt because there are only details about the supported functionalities, no need to add info about something that is not supported.

Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM.

Let's hope we can merge the pilot wire mode PR aslo before 4.3.0

@lsiepel lsiepel merged commit 39a2754 into openhab:main Oct 13, 2024
4 of 5 checks passed
@lsiepel lsiepel added this to the 4.3 milestone Oct 13, 2024
@dilyanpalauzov dilyanpalauzov deleted the enocean_update_descriptions branch October 13, 2024 19:21
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
… detection (openhab#17522)

configurable dimming or pilot wire, do not have local control.

Co-authored-by: Cody Cutrer <cody@cutrer.us>
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 an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants