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

[MQTT] Fix state description #16866

Merged
merged 2 commits into from
Jun 14, 2024
Merged

[MQTT] Fix state description #16866

merged 2 commits into from
Jun 14, 2024

Conversation

mherwege
Copy link
Contributor

@mherwege mherwege commented Jun 13, 2024

The dynamic stateDescription created by the MQTT binding always uses %s for all of its types.

The consequence of this is that for a Number QuantityType, the full state including the unit will be returned. When retrieving the state description from the Number:Dimension item linked to that channel, another unit will be added, effectively showing the unit label twice.
See https://community.openhab.org/t/doubled-unit-output/156591 for more details about the problem.

This PR makes sure proper state descriptions, complying to the default, are returned.

At the same time, the MQTT binding currently always returns a QuantityType<Dimensionless> with Unit.ONE for channels where the unit is not set. This has been changed here to return DecimalType instead. Without it, the state description patterns would need special handling, different from handling elsewhere, and the conversion between Unit.ONE and Unit.PERCENT could not be left to the framework. This is a change in behaviour and may impact handling with % values. But the handling in the MQTT binding at this time is inconsistent with the overall handling in OH, so I believe this change improves consistency overall.
The impact of this change can be seen in the modified and extra test in ValueTests.numberPercentageUpdate().

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege mherwege marked this pull request as draft June 13, 2024 15:29
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@mherwege mherwege marked this pull request as ready for review June 13, 2024 15:46
@mherwege
Copy link
Contributor Author

To my surprise, my Eclipse IDE has automatically refactored some explicit type casting in ValueTests. It looks like it does not have an impact on compilation and execution of the tests.
The only intended changes where in ValueTests.numberPercentageUpdate(), but the other changes look like being a good simplification as well.

@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/basicui-4-2-0-m3-issue-with-uom/156700/3

@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/doubled-unit-output/156591/16

@ccutrer
Copy link
Contributor

ccutrer commented Jun 13, 2024

Yes, I noticed them. A while back I changed those methods to return the specific Type for each Value, but didn't go change any of the tests. So I agree it's a good simplification.

@jlaur jlaur added the bug An unexpected problem or unintended behavior of an add-on label Jun 14, 2024
@jlaur jlaur changed the title [MQTT] fix state description [MQTT] Fix state description Jun 14, 2024
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

@lsiepel lsiepel merged commit f64cb0b into openhab:main Jun 14, 2024
5 checks passed
@mherwege mherwege deleted the mqtt branch June 14, 2024 17:33
@jlaur jlaur added this to the 4.2 milestone Jun 24, 2024
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Jun 29, 2024
* fix state description

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
@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/change-the-default-state-pattern-in-4-2-x/158322/10

pgfeller pushed a commit to pgfeller/openhab-addons that referenced this pull request Sep 29, 2024
* fix state description

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
* fix state description

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Oct 18, 2024
* fix state description

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
cipianpascu pushed a commit to cipianpascu/openhab-addons that referenced this pull request Jan 2, 2025
* fix state description

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
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.

5 participants