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 #6678

Closed
wants to merge 4 commits into from
Closed

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Dec 24, 2019

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

  • complete code
  • add JavaDoc
  • binding documentation

Attention! OH3 only - requires Java 11.

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

@kaikreuzer
Copy link
Member

I don't really understand your proposal. Are you still thinking about it as a profile? If so, making it available for any Call or String channel is the most generic option we can imho have...?

@J-N-K
Copy link
Member Author

J-N-K commented Aug 30, 2020

The telegram binding has a string-type channel chatId which provides the (numeric) id of the sender of the last message. If I add an SQL database (and support for the profile by an SQL binding) with a table chatId -> realName the profile could be used used to lookup the real name and send that to the item instead of the numeric chatId.

The callmonitor of the avmfritz-binding could use the same profile with the TR-064-binding's phonebook (which essentially is nothing else than a phoneNumber->realName table) .

The unifi binding has a string-type channel ap for the access point a wireless channel is connected to. The profile could be used to do a lookup in a database to use a more human readable name (provided by whatever datasource implements support for the profile).

@kaikreuzer
Copy link
Member

I still do not understand why you talk about databases, this seems to introduce a huge unnecessary complexity to the discussion.
If the phonebook lookup profile is done as I suggested above and is "available on any channel that provides either CallItem or StringItem values", you can automatically apply it to the telegram chatId channel as well as to the avmfritz incomingCall channel. What else is needed?

@J-N-K
Copy link
Member Author

J-N-K commented Aug 30, 2020

All I want is an interface LookupDatasource with methods String lookup(String value), String lookup(String value, int matchCount) and String lookup(String value, int matchCount, boolean reverse) and a LookupProfileFactory where datasource providers (like the TR-064 binding or whoever is able to provide a datasource for a string lookup) can register their data sources (be it a phonebook or whatever table can be used to lookup a value and return another value).

The user than can apply the lookup profile (configured with the datasource and additional information like matchCount) to every channel he wants, no matter if it is a number, a MAC address, an IP address, a chatId or whatever). If it's any value, having "phonebook" in the name is not well thought through.

So I want to remove the interface declaration (and profile classes) from this binding, add them as a new contribution (probably to core, similar to the SystemOffsetProfile) and just leave the implementation of the interfaces in this binding. That way the lookup looks the same to the user, no matter where the original value came from and who provides the lookup-values.

@kaikreuzer
Copy link
Member

With this you would introduce another layer of complexity as this profile implementation would just be an empty hull and not usable if there isn't any "datasource" (which is a so far non-existing concept) available.
We already have such a rather generic profile in place (the transformation profile) which is at least based on an existing concept (the transformations). I don't think it makes sense to introduce another, fairly similar one.

Having the PhonebookLookup profile in place as discussed above is imho pretty good as it only turns up as an option if people are using this binding. I'd actually even have an improvement suggestion: Instead of requiring a Thing as a parameter, it should only be registered if a fritzbox thing is set up and a phonebook is available. Selecting a phonebook (could be a reference of fritzboxname+phonebookid) could then be the only required parameter.

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 Sep 13, 2020

Having the PhonebookLookup profile in place as discussed above is imho pretty good as it only turns up as an option if people are using this binding. I'd actually even have an improvement suggestion: Instead of requiring a Thing as a parameter, it should only be registered if a fritzbox thing is set up and a phonebook is available. Selecting a phonebook (could be a reference of fritzboxname+phonebookid) could then be the only required parameter.

Good idea. What should the profile be named then? PHONEBOOK_<UID>? Another questions: Do we need the textual configuration example in the docs in OH3?

@kaikreuzer
Copy link
Member

What should the profile be named then? PHONEBOOK_?

Should still be just "PhonebookLookup" - my idea from above was to have a single profile and only have the Thing being selected as part of the required parameter (fritzboxname+phonebookid). This keeps the profile list clean (there is just a single "PhonebookLookup" entry) and only when the user choses it, he is confronted with having to think about "which fritzbox?" and "which phonebook id?" and answer both these question by selecting one of the provided options.

Do we need the textual configuration example in the docs in OH3?

Would imho be extremely helpful, yes!

@J-N-K
Copy link
Member Author

J-N-K commented Sep 13, 2020

Is it possible to dynamically provide options for the thing-UID? A drop-down list would be nice, but I don't know how to achive that.

The textual configuration really is a PITA. It always takes me hours to figure out how the textual thing configuration works. But ok.

@kaikreuzer
Copy link
Member

Is it possible to dynamically provide options for the thing-UID? A drop-down list would be nice, but I don't know how to achive that.

Most certainly! Just the very same way as for any other config descriptions: By implementing a ConfigOptionProvider.

It always takes me hours to figure out how the textual thing configuration works. But ok.

Think of all the hours you can save the users of the binding 😄 . But yeah, especially the profile syntax isn't yet documented anywhere properly, so it might not be easy to provide this. Feel free to leave it to the end and I can try to help figuring out the right syntax for the example then.

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

With the implementation in place now, you can select the phonebook in the profile. Do you want to lookup the phone number in all phonebooks at once? We should continue that discussion in #6678

Yes, that's what I mean. It would be great to look up for numbers in all phonebooks at once. As I already wrote I use different phonebooks. The first one for private, the second one for business contacts (home office). By resolving the caller name I get notifications from my Echos using Alexa's TTS when somebody's calling. But at the moment it's working for one phonebook only.

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

It would be great to look up for numbers in all phonebooks at once.

... which could be potentially even be coming from different bindings. Imho this is too much for a profile and the better way to have it covered is through calling actions in rules. Besides the profile, I hence think that we should also have an action for any custom complex use cases.

@J-N-K
Copy link
Member Author

J-N-K commented Sep 20, 2020

All phonebooks of a thing would be easy, it‘s just one additional entry in the parameter options.

What actions do you think about? Getting the phonebook(s) or looking up numbers or both?

@kaikreuzer kaikreuzer closed this Sep 20, 2020
@kaikreuzer
Copy link
Member

Could you please re-create this PR against the main branch?
See point (4) in #8512. Thanks!

@J-N-K J-N-K deleted the fritzboxtr064 branch September 21, 2020 17:05
@wborn wborn removed the oh3 label Oct 17, 2020
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.