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

[netatmo] Make configuration link clickable in Main UI #18079

Merged
merged 2 commits into from
Jan 12, 2025
Merged

[netatmo] Make configuration link clickable in Main UI #18079

merged 2 commits into from
Jan 12, 2025

Conversation

jimtng
Copy link
Contributor

@jimtng jimtng commented Jan 10, 2025 •
edited
Loading

clinique
clinique approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@clinique clinique 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, this annoyed me for a while

jlaur
jlaur requested changes Jan 10, 2025
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.

Cool! I have added few minor annoying comments. 😉

bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/i18n/netatmo_fi.properties Outdated Show resolved Hide resolved
bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/i18n/netatmo_fr.properties Outdated Show resolved Hide resolved
bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/i18n/netatmo_it.properties Outdated Show resolved Hide resolved
...ing.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/ApiBridgeHandler.java Outdated Show resolved Hide resolved
@jimtng jimtng force-pushed the netatmo-config-link branch from 36e7e4c to 9b760b8 Compare January 10, 2025 07:54
@jlaur
Copy link
Contributor

jlaur commented Jan 10, 2025

Note to self: Check how this looks like in events.log.

jimtng added a commit to jimtng/openhab-webui that referenced this pull request Jan 10, 2025
@jimtng
Thing Details: broaden the criteria for hyperlinking thing status des…
1a15e18 
…cription

See openhab/openhab-addons#18079 (comment)

Signed-off-by: Jimmy Tanagra <[email protected]>
@jimtng jimtng mentioned this pull request Jan 10, 2025
Merged
@jlaur jlaur added awaiting other PR Depends on another PR and removed awaiting other PR Depends on another PR labels Jan 10, 2025
@jlaur jlaur mentioned this pull request Jan 10, 2025
Open
@jlaur
Copy link
Contributor

jlaur commented Jan 11, 2025

Before this PR:

image

In log:

2025-01-11 11:45:09.966 [INFO ] [ab.event.ThingStatusInfoChangedEvent] - Thing 'netatmo:account:05828bd3bf' changed from UNKNOWN to OFFLINE (CONFIGURATION_ERROR): Complete the configuration by granting the binding to Netatmo Connect: '/netatmo/connect/123'.

With this PR:

image

(and link works)

In log:

2025-01-11 11:50:33.181 [INFO ] [ab.event.ThingStatusInfoChangedEvent] - Thing 'netatmo:account:05828bd3bf' changed from UNKNOWN to OFFLINE (CONFIGURATION_ERROR): Complete the configuration by granting the binding to Netatmo Connect: http(s)://<YOUROPENHAB>:<YOURPORT>/netatmo/connect/123

Is the binding able to get hold of host and port, or is this only known by the UI? If the binding could obtain it, I guess it would be possible to provide it from binding side so that the link would also be available in the log.

Either way, it's still an improvement, since we now have a working link in the UI, and we didn't have that in UI or log previously. It's just a minor glitch that the log shows the placeholders.

@jlaur jlaur added the enhancement An enhancement or new feature for an existing add-on label Jan 11, 2025
@jimtng
Copy link
Contributor Author

jimtng commented Jan 11, 2025

Is the binding able to get hold of host and port

I haven't dug deep enough to figure this out. Doing it in the UI / javascript was easy.

If I have time I could poke around and see.

@jlaur
Copy link
Contributor

jlaur commented Jan 11, 2025

Is the binding able to get hold of host and port

I haven't dug deep enough to figure this out. Doing it in the UI / javascript was easy.

If I have time I could poke around and see.

Thanks! 👍 One last thought: How will this work with openHAB cloud? I don't use that personally, so would need to set this up to check.

@jimtng
Copy link
Contributor Author

jimtng commented Jan 11, 2025 •
edited
Loading

One last thought: How will this work with openHAB cloud? I don't use that personally, so would need to set this up to check.

Good question! I have never used myopenhab in this manner, but I've just tested it now, e.g. on amazonechocontrol
image

https://home.myopenhab.org/amazonechocontrol/aa2862ceec82/ clicking that link worked for me.

This means, this. has to be done through webui. You can't figure out this link from within the binding because it doesn't know whether it's accessed locally or via myopehab.

jlaur
jlaur approved these changes Jan 12, 2025
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.

LGTM

@jlaur jlaur merged commit 7598cf9 into openhab:main Jan 12, 2025
2 checks passed
@jlaur jlaur added this to the 5.0 milestone Jan 12, 2025
@jlaur jlaur mentioned this pull request Jan 14, 2025
Merged
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

@clinique clinique clinique approved these changes

@lolodomo lolodomo Awaiting requested review from lolodomo lolodomo is a code owner

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

Successfully merging this pull request may close these issues.
3 participants
@jimtng @jlaur @clinique