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] Initial contribution #8523

Merged
merged 4 commits into from
Nov 3, 2020
Merged

[tr064] Initial contribution #8523

merged 4 commits into from
Nov 3, 2020

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Sep 21, 2020

This provides a replacement for the OH1 "fritzboxtr064" binding.

Replaces #6678

Signed-off-by: Jan N. Klug jan.n.klug@rub.de

@J-N-K J-N-K requested a review from a team as a code owner September 21, 2020 17:00
@TravisBuddy
Copy link

TravisBuddy commented Sep 21, 2020

Travis tests have failed

Hey @J-N-K,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

@Hilbrand Hilbrand added new binding If someone has started to work on a binding. For a new binding PR. oh1 migration Relates to migrating an openHAB 1 addon to openHAB 2 labels Sep 22, 2020
@J-N-K J-N-K self-assigned this Sep 25, 2020
@J-N-K J-N-K removed their assignment Sep 27, 2020
* @return the SOAPMessage answer from the remote host
* @throws Tr064CommunicationException if an error occurs during the request
*/
public synchronized SOAPMessage doSOAPRequest(SCPDServiceType service, String soapAction,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this synchronized?

Copy link
Member Author

Choose a reason for hiding this comment

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

IIRC there was a problem when several requests are executed at the same time and return 401. It was not poissble to get the re-auth working in that case.

bundles/org.openhab.binding.tr064/pom.xml Outdated Show resolved Hide resolved
@lionhe1966
Copy link
Contributor

I am not sure if this is the right place to ask. Will the binding allow connection to more than one fritzbox? My use case would be presence detection on the fritzbox in my local network and on a VPN connected remote fritzbox.

@J-N-K
Copy link
Member Author

J-N-K commented Oct 3, 2020

@lionhe1966 yes

@J-N-K
Copy link
Member Author

J-N-K commented Oct 3, 2020

Interesting. The build does not fail locally. And the error is wrong anyway, returning null is fully valid and results in an empty Optional.

@wborn
Copy link
Member

wborn commented Oct 3, 2020

You'd see this error if you are using a newer version of Eclipse or if you rebase on the current main branch.
Travis is now using an updated compiler (2 years newer) similar to that being used in current Eclipse versions (see #8636).

@J-N-K
Copy link
Member Author

J-N-K commented Oct 3, 2020

@wborn It‘s still wrong. Obviously the compiler update breaks valid code. It‘s very hard to create a work around. Maybe ist improves when we have external null annotations in place.

@wborn
Copy link
Member

wborn commented Oct 3, 2020

At least you only need to make one workaround instead of two because of the more similar compiler versions. ;-)
Doesn't adding @NonNullByDefault({}) or @Nullable fix it?

@jgantenberg
Copy link

I changed from version 1 binding to the test version of the fritzboxtr064 binding. I managed to set it up I receive valid values for example from the WAN-sub-device. I set it up via paper ui and filled in proper values for the root device for hostname, user, password etc. Also for tam index and the number of days for incoming and outgoing calls.

Expected Behavior
the filled in hostname should be used for communication with the fritzbox.
I also expect additional things to be automatically created corresponding to the configuration settings, for example a ringing indicator or missed calls thing.

Current Behavior
The configuration for hostname is stored but the name fritz.box has to be resolvable. Otherwise the binding won't start complaining fritz.box can't be resolved.
In the section for the additional values none of the values is stored and I don't see additional things for incoming calls etc.

Possible Solution
Steps to Reproduce (for Bugs)
setup new root device
reopen configuration
search for things
Context
Your Environment
Version used: openHAB 2.5.9, org.openhab.binding.fritzboxtr064-2.5.6-SNAPSHOT.jar
Fritzbox 6591 cable, 7.13
ubuntu focal x86
OpenJDK Runtime Environment Zulu11.43+21-CA (build 11.0.9+11-LTS)

@J-N-K
Copy link
Member Author

J-N-K commented Oct 25, 2020

@jgantenberg I already responded in the community forum thread. Please set the binding to TRACE logging and show the log.

@jgantenberg
Copy link

jgantenberg commented Oct 25, 2020

@jgantenberg I already responded in the community forum thread. Please set the binding to TRACE logging and show the log.
openhab.log-1.txt
hope, there are no confidential informations inside

@J-N-K
Copy link
Member Author

J-N-K commented Oct 31, 2020

@jgantenberg This happens after you configured a different hostname than fritz.box?

@J-N-K
Copy link
Member Author

J-N-K commented Oct 31, 2020

@cpmeister I think I addressed most of your comments and commented on the remaining issues.

Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Some small final comments from me - rest is fine and imho good for being merged.

Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
@J-N-K
Copy link
Member Author

J-N-K commented Oct 31, 2020

Jenkins fails on the tado-library. I wonder what is wrong there.

Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Thanks!

@jgantenberg
Copy link

@jgantenberg This happens after you configured a different hostname than fritz.box?

yes, I have configured another hostname for my fritzbox, that is configured properly in DNS. I can fill in IP or hostname, the message that fritz.box cannot be resolved remained the same.

@J-N-K
Copy link
Member Author

J-N-K commented Nov 1, 2020

@jgantenberg That must be due to the old version you are using (I'm not sure if that was hard-coded in an earlier version). I checked that it works in the current version if I assign an IP address instead of a host-name. Please try again, when this has been merged (I guess that will happen in the next days).

@cpmeister cpmeister merged commit a3d33fa into openhab:main Nov 3, 2020
@cpmeister cpmeister added this to the 3.0.0.M3 milestone Nov 3, 2020
@jgantenberg
Copy link

I have another question: is there also a ringing indicator channel as it was in the v1 binding? It was nice to have it to evaluate in rules, e.g. lowering volume of music or tv.
Thanks.

@J-N-K J-N-K deleted the tr064 branch November 4, 2020 14:11
@cweitkamp
Copy link
Contributor

@J-N-K Thanks for this great implementation. I am playing with the Phonebook Profile combined with the avmfritz Call Monitor. Do you already have an working example?

When adding the Profile via UI I am not able to see the configuration for it:
grafik

There is an Internal 500 with the following error message:

"{\"error\":{\"message\":\"stream has already been operated upon or closed\",\"http-code\":500,\"exception\":{\"class\":\"java.lang.IllegalStateException\",\"message\":\"stream has already been operated upon or closed\",\"localized-message\":\"stream has already been operated upon or closed\"}}}"

I saw you implemented a ConfigOptionProvider for the parameter phonebook using a lot of streaming stuff I assume this is somehow related. Any ideas? Disabling / Removing the ConfigOptionProvider at least eliminates the Internal 500 and lets me see the Profile configuration in the UI.

@cweitkamp
Copy link
Contributor

I found basic solution for it in #9054.

nowaterman pushed a commit to nowaterman/openhab-addons that referenced this pull request Jan 19, 2021
boehan pushed a commit to boehan/openhab-addons that referenced this pull request Apr 12, 2021
* Initial contribution

Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
* Initial contribution

Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new binding If someone has started to work on a binding. For a new binding PR. oh1 migration Relates to migrating an openHAB 1 addon to openHAB 2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants