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

Enhanced sitemap SSE event with a new boolean field informing if the #2413

Merged
merged 1 commit into from
Jun 25, 2021

Conversation

lolodomo
Copy link
Contributor

item state or command description has been updated

Listen to the new ChannelDescriptionChangedEvent

Signed-off-by: Laurent Garnier lg.hc@free.fr

@lolodomo lolodomo requested a review from a team as a code owner June 24, 2021 19:04
@lolodomo
Copy link
Contributor Author

@cweitkamp @kaikreuzer : that would be really cool if you could do a very fast review & merge.

This is the core part of the changes necessary to trigger the handling of updated dynamic state/command description in any UI handling sitemaps.
I just added a new field in the SSE message. This will be with no impact on unmodified UI app.
I will then update Basic UI to read this new field and I will trigger a page reload when the field is set to true.

@lolodomo
Copy link
Contributor Author

@mueller-ma @maniac103 : for your information in case you would like to handle that in the Android app.

@lolodomo
Copy link
Contributor Author

@timbms : for iOS app (in case the iOS app is based on the sitemap SSE events)

@lolodomo lolodomo force-pushed the sitemap_sse_descrchanged branch from 2227a33 to d49b8cb Compare June 24, 2021 20:51
item state or command description has been updated

Listen to the new ChannelDescriptionChangedEvent

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo lolodomo force-pushed the sitemap_sse_descrchanged branch from d49b8cb to 230b4ae Compare June 24, 2021 20:58
@cweitkamp cweitkamp added enhancement An enhancement or new feature of the Core sitemap labels Jun 25, 2021
Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@cweitkamp cweitkamp added this to the 3.1 milestone Jun 25, 2021
@cweitkamp cweitkamp merged commit 2d71afe into openhab:main Jun 25, 2021
@lolodomo
Copy link
Contributor Author

Excellent, thank you. I will work on Basic UI enhancement today.

@kaikreuzer : Please trigger again a new core rebuild for next snapshots.

@lolodomo lolodomo deleted the sitemap_sse_descrchanged branch June 25, 2021 05:28
@maniac103
Copy link
Contributor

@lolodomo Do apps need changing at all? I'd think the new description values should be part of the event sent out, shouldn't they?
(And BTW, handling this in the server is a very good idea, thank you for that!)

@lolodomo
Copy link
Contributor Author

@lolodomo Do apps need changing at all? I'd think the new description values should be part of the event sent out, shouldn't they?

Yes, the new state description / command description is part of the sent event. If you are already updating the command/state options when receiving such an event, that is perfect and you have nothing more to do.
So I will test if it already works in Android app.
In BasicUI, it is not yet handled.

@lolodomo
Copy link
Contributor Author

@maniac103 : excellent, I just tested the Android app and it works well. You have nothing to do ;)

@timbms : can you give us a feedback for the iOS app ? Something to change or not ?

@kaikreuzer
Copy link
Member

@kaikreuzer : Please trigger again a new core rebuild for next snapshots.

@lolodomo Did so: https://ci.openhab.org/job/openHAB-Core/797/

I will work on Basic UI enhancement today.

Please note that we are officially now already in code freeze and I planned to do the RC1 build right now.

If you think the Basic UI changes are straight forward and riskless, I'd give you until 1pm and postpone the RC1 build until then. Otherwise, I'd suggest to live without those changes for 3.1.

@maniac103
Copy link
Contributor

A bit OT:

Please note that we are officially now already in code freeze and I planned to do the RC1 build right now.

Does this also affect addons? I have one addition I'd really love to see in 3.1, but I'm not sure how to expedite code review there (or, for starters, getting this into the 3.1 milestone marker)

@lolodomo
Copy link
Contributor Author

If you think the Basic UI changes are straight forward and riskless, I'd give you until 1pm and postpone the RC1 build until then.

I will try for 1pm French hour.

@kaikreuzer
Copy link
Member

Does this also affect addons?

@maniac103 Yes, it does. But as it is just 1pm, I have merged it for you :-)

@lolodomo
Copy link
Contributor Author

Too late for me, I encounter a difficulty to detect my new field, probably something stupid but I will find the solution only this evening.

@kaikreuzer
Copy link
Member

Ok, then I will start the RC1 build now - nonetheless, thanks for trying!

lolodomo added a commit to lolodomo/openhab-webui that referenced this pull request Jun 25, 2021
…n the page

was updated

Related to openhab/openhab-core#2413

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo
Copy link
Contributor Author

@kaikreuzer : my PR is now submitted (just 4 lines of code).

@maniac103
Copy link
Contributor

Yes, it does. But as it is just 1pm, I have merged it for you :-)

Great, thank you!

kaikreuzer pushed a commit to openhab/openhab-webui that referenced this pull request Jun 26, 2021
…n the page (#1120)

Related to openhab/openhab-core#2413

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
ghys pushed a commit to ghys/openhab-core that referenced this pull request Sep 9, 2021
…ng if the (openhab#2413)

item state or command description has been updated

Listen to the new ChannelDescriptionChangedEvent

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 11, 2023
…ng if the (openhab#2413)

item state or command description has been updated

Listen to the new ChannelDescriptionChangedEvent

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
GitOrigin-RevId: 2d71afe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core sitemap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants