Repository navigation
add support for linked services - #110
Conversation
|
@dfrommi thank you for your pull request. this is really most wanted/missing feature. |
|
You have a base service (just made this name up) and a linked service. And you just have to make sure that I don't have a strong opinion on how it should be added to the accessory interface, but you will probably somewhere have code like Sharing my TV-accessory doesn't help much. I have a small facade around HAP-Java to better integrate into my Project-Reactor and Kotlin environment. There's not this one small piece of code that I could share as an example. |
|
@yfre I decided to make my wrapper public. Here's a sample for a TV service |
|
@dfrommi i was trying to get it working with test client (here in sample branch) or with openhab but without success. probably the getLinkedService needs to be adapted as it always returns a new array and forget all services added via "getLinkedServices().add()". the way how you initialise HomkitAccessory in kotlin is not the way how it is typically done in java. would it be ok for you, if I take your PR, add some additional java API and submit as a new PR? |
|
@dfrommi just checked and indeed, getLinkedServices was the issue. so, only minor changes needed to make it work in java as well. i will propose them as part of PR review here, maybe you could incorporate |
| public List<Service> getLinkedServices() { | ||
| return new ArrayList<>(); | ||
| } | ||
|
|
There was a problem hiding this comment.
please make the linkedService list persistent by adding following
| private final List<Service> linkedServices = new LinkedList<>(); | |
| @Override | |
| public void addLinkedService(Service s) { linkedServices.add(s);} | |
| @Override | |
| public List<Service> getLinkedServices() { return Collections.unmodifiableList(linkedServices); } |
| * ########-####-####-####-############. | ||
| */ | ||
| String getType(); | ||
|
|
There was a problem hiding this comment.
please add
| void addLinkedService(Service s); | |
There was a problem hiding this comment.
Characteristics are handled just the same as LinkedServices in the Service interface and AbstractServiceImpl. And so I didn't add addLinkedService to the Service interface, but only to the abstract class, which is just how it's done for Characteristics as well.
Some services require linked services to work, for example, the Television service which requires each input to be a linked service. With this change, support to link services is added, though it's not used in any existing service, as they don't need linking. This change also centralizes the generation of interface ids. They are now only generated in the registry. We were previously also generating them in the accessory controller, relying on an identical processing sequence.
|
@yfre Thank you so much for reviewing my changes. |
yfre
left a comment
yfre
left a comment
There was a problem hiding this comment.
looks good for me.
i have tested the sample client
@dfrommi you are right. linkedService should be done in the same way as characteristics. |
Some services require linked services to work, for example, the Television service which requires each input to be a linked service.
With this change, support to link services is added, though it's not used in any existing service, as they don't need linking. But I verified the functionality using my own Television service, which is not yet production-ready.
This change also centralizes the generation of interface ids. They are now only generated in the registry.
We were previously also generating them in the accessory controller, relying on an identical processing sequence.
This is implementing the issue #104 and replacing PR #105.
The author seems to have abandoned the PR and I personally also think that linking accessories rather than services is not the right level.
Pull Request Checklist
Please confirm that you've done the following when opening a new pull request: