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.

Sign up for GitHub

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

Jump to bottom

[mqtt] Dynamically change accepted item-type for Number channels #15114

Merged
merged 2 commits into from Jul 21, 2023
Merged

[mqtt] Dynamically change accepted item-type for Number channels #15114

merged 2 commits into from Jul 21, 2023

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Jun 18, 2023

See #15072

@J-N-K J-N-K added the enhancement An enhancement or new feature for an existing add-on label Jun 18, 2023
@J-N-K J-N-K requested a review from davidgraeff as a code owner June 18, 2023 19:26
@NorbertHD
Copy link

Does this PR fix the 120 second timeout with "UNINITIALIZED (NOT_YET_READY)" at startup?

@J-N-K
Copy link
Member Author

J-N-K commented Jul 14, 2023

Does this PR fix the 120 second timeout with "UNINITIALIZED (NOT_YET_READY)" at startup?

No. Is this happening with MQTT? Can you create an easy to reproduce example for that?

@J-N-K
Copy link
Member Author

J-N-K commented Jul 15, 2023

@openhab/add-ons-maintainers I believe the issue is with the partial build.

@NorbertHD
Copy link

@J-N-K
This happens with MiFlora sensors over OpenMQTTGateway with homeassistant discovery format.

...
2023-07-15 16:05:30.313 [WARN ] [core.thing.internal.ThingManagerImpl] - A thing handler factory claims to support 'mqtt:homeassistant_9C9C1FC6CA0C' for thing 'mqtt:homeassistant_9C9C1FC6CA0C:c1e021c7b5:9C9C1F
C6CA0C' for more than 120s, but the thing type can't be found in the registry. This should be fixed in the binding.
...
2023-07-15 16:05:32.539 [INFO ] [ab.event.ThingStatusInfoChangedEvent] - Thing 'mqtt:homeassistant_9C9C1FCAF9C8:c1e021c7b5:9C9C1FCAF9C8' changed from UNINITIALIZED (NOT_YET_READY) to INITIALIZING
2023-07-15 16:05:32.553 [INFO ] [ab.event.ThingStatusInfoChangedEvent] - Thing 'mqtt:homeassistant_9C9C1FCAF9C8:c1e021c7b5:9C9C1FCAF9C8' changed from INITIALIZING to UNKNOWN
...

@J-N-K
Copy link
Member Author

J-N-K commented Jul 15, 2023

@NorbertHD This needs to be fixed in the home assistant part of MQTT. There are two reference implementations, but someone needs to pick this issue up.

@wborn
Copy link
Member

wborn commented Jul 17, 2023

Any MQTT PR build will fail because the ruuvigateway itest is commented and not part of the reactor:

openhab-addons/itests/pom.xml

Lines 31 to 33 in 075592f

<!-- MQTT ruuvigateway tests disabled until fixed (CoreItemFactory + NumberItem)
<module>org.openhab.binding.mqtt.ruuvigateway.tests</module>
-->

So hopefully it can be fixed soon with #15018. What will also help is to rename or delete the itest dir.

@J-N-K
[mqtt] Dynamically change accepted item-type for Number channels
bc88257 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K
Copy link
Member Author

J-N-K commented Jul 20, 2023

@openhab/add-ons-maintainers It is debatable if this is a bugfix or an enhancement. Without it UoM will not work properly with MQTT. I'm not sure if this would be a regression to 3.4.

@J-N-K
address review comments
d83d92c 
Signed-off-by: Jan N. Klug <github@klug.nrw>
jlaur
jlaur approved these changes Jul 21, 2023
View reviewed changes
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! LGTM.

@jlaur
Copy link
Contributor

jlaur commented Jul 21, 2023

It is debatable if this is a bugfix or an enhancement. Without it UoM will not work properly with MQTT. I'm not sure if this would be a regression to 3.4.

I agree with should merge for consistent UoM behavior. @openhab/add-ons-maintainers - anyone against?

@kaikreuzer
Copy link
Member

No, I'm not against merging.

@J-N-K J-N-K mentioned this pull request Jul 21, 2023
Closed
@jlaur jlaur merged commit ae14611 into openhab:main Jul 21, 2023
3 checks passed
@jlaur jlaur added this to the 4.0 milestone Jul 21, 2023
@J-N-K J-N-K deleted the mqtt branch July 21, 2023 10:04
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@J-N-K @austvik
[mqtt] Dynamically change accepted item-type for Number channels (ope…
6a68cd8 
…nhab#15114)

Signed-off-by: Jan N. Klug <github@klug.nrw>
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
Reviewers

@jlaur jlaur jlaur approved these changes

@davidgraeff davidgraeff Awaiting requested review from davidgraeff

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@J-N-K @NorbertHD @wborn @jlaur @kaikreuzer