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

[tr064] Fix clearing of auth (results) #15415

Merged
merged 2 commits into from
Aug 13, 2023
Merged

[tr064] Fix clearing of auth (results) #15415

merged 2 commits into from
Aug 13, 2023

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Aug 12, 2023

In case of multiple root things the auth store (results and digests) was cleared for all clients instead of only removing the failed auth result or old authentication.

Also fixes TAM list request.

In case of multiple root things the auth store (results and digests) was cleared for all clients instead of only removing the failed auth result or old authentication.

Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K added the bug An unexpected problem or unintended behavior of an add-on label Aug 12, 2023
@J-N-K J-N-K marked this pull request as ready for review August 12, 2023 22:01
Signed-off-by: Jan N. Klug <github@klug.nrw>
@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/upgraded-to-oh4-0-tr064-binding-not-working-properly-any-more/148090/13

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 for the fixes! One minor comment for consideration.

@@ -244,7 +244,7 @@ private State processMacSignalStrength(State state, Tr064ChannelConfig channelCo
@SuppressWarnings("unused")
private State processTamListURL(State state, Tr064ChannelConfig channelConfig) throws PostProcessingException {
try {
ContentResponse response = httpClient.newRequest(state.toString()).timeout(timeout, TimeUnit.MILLISECONDS)
ContentResponse response = httpClient.newRequest(state.toString()).timeout(timeout, TimeUnit.SECONDS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps the instance variable timeout could also be renamed to timeoutSeconds or similar to avoid such mistake from being repeated?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that is necessary. It was a refactoring error when the configurable timeouts were introduced in #14468 (the fixed value was in ms).

@jlaur jlaur merged commit 196d809 into openhab:main Aug 13, 2023
@jlaur jlaur added this to the 4.1 milestone Aug 13, 2023
jlaur pushed a commit that referenced this pull request Aug 13, 2023
* [tr064] Fix clearing of auth (results)

In case of multiple root things the auth store (results and digests) was cleared for all clients instead of only removing the failed auth result or old authentication.

* fix TAM request

Signed-off-by: Jan N. Klug <github@klug.nrw>
@jlaur jlaur added the patch A PR that has been cherry-picked to a patch release branch label Aug 13, 2023
@J-N-K J-N-K deleted the tr064 branch August 13, 2023 16:57
@Intenos
Copy link

Intenos commented Aug 27, 2023

Can I still download the snapshot binding (org.openhab.binding.tr064-4.1.0-SNAPSHOT.jar) from somewhere? I don´t want to upgrade my complete openHAB to the not yet released 4.1.0 version.

@jlaur
Copy link
Contributor

jlaur commented Aug 27, 2023

@Intenos - the fix was included in 4.0.2, see https://github.com/openhab/openhab-distro/releases/tag/4.0.2.

@Intenos
Copy link

Intenos commented Aug 27, 2023

Fantastic! It works. Thanks a lot!

austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
* [tr064] Fix clearing of auth (results)

In case of multiple root things the auth store (results and digests) was cleared for all clients instead of only removing the failed auth result or old authentication.

* fix TAM request

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
Labels
bug An unexpected problem or unintended behavior of an add-on community approved patch A PR that has been cherry-picked to a patch release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants