Skip to content

Linked Services #104

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

Closed
scothi opened this issue Mar 30, 2020 · 12 comments
Closed

Linked Services #104

scothi opened this issue Mar 30, 2020 · 12 comments

Comments

@scothi
Copy link

scothi commented Mar 30, 2020

Are there any plans to implement the Linked Services logic?
I'm implementing - or at least trying to - the Television service which comes with two linked services (InputSource and TelevisionSpeaker) but I am having difficulties. I tried to add this functionality but it's not clear for me what is linked to what. Is the "sub" service linked to the "main" service or the linked attribute contains the sub service's ids?

@yfre
Copy link
Contributor

yfre commented Mar 30, 2020

i have implemented it for my irrigation system which has to be linked to valve services. but it was more hack than proper implementation.
in terms what is linked to what i would say, main services has an array with linked services IDs.

@scothi
Copy link
Author

scothi commented Mar 31, 2020

Ok, thanks. Now it’s working. :)

@yfre
Copy link
Contributor

yfre commented Mar 31, 2020

cool. will you push your changes to your repository or create PR here? i would love to integrate your changes to my branch.

@scothi
Copy link
Author

scothi commented Apr 1, 2020

PR sent #105

@dfrommi
Copy link
Contributor

dfrommi commented Sep 30, 2020

@scothi So you have a working Television service?
Would you mind sharing it, esp. the missing characteristics? I tried the same but I just can't get it working.
No exceptions or errors, but the accessory is still shown as "not available" in the home app.
I suspect that something is wrong with the characteristics, but I just can't find it.

@gjvanderheiden
Copy link
Contributor

gjvanderheiden commented Dec 10, 2020

Could

public void addLinkedService(Service service)

be added to the interface Service.

I don't see how I can add a linked service when using hap-java.

AbstractServiceImpl isn't accessible. I ran into problems when adding FilterMaintenanceAccessory. The service it's linked to can't be added as a linked service now.

@gjvanderheiden
Copy link
Contributor

I would like to do this:

HomekitAccessory accessory = new MockSwitch(...);
accessory.addLinkedAccessory(new MockBatteryAccessory());

@yfre
Copy link
Contributor

yfre commented Dec 10, 2020

yes, i have proposed the addLinkedAccessory as well but decided against it in order to be consistent with Characterstics which has addCharacteristics only at AbstractServiceImpl but not in the interface.

as of now you can do this
accessory.getServices().add(new Service())

but maybe we should really introduce addLinkedService at Service interface. maybe also the addCharacteristic as well. it is more developer friendly imo.

@yfre
Copy link
Contributor

yfre commented Dec 10, 2020

@gjvanderheiden you are completely right, without adding addLinkedService to Service interface we cannot use it. it was working only with test mock client as i could overwrite getService there.
and it is also correct to add addCharacteristics to Service interface. characteristics are fixed and the developer should not add them manually one-by-one.
let me prepare a PR with changes

@dfrommi
Copy link
Contributor

dfrommi commented Dec 10, 2020

I actually thought it could used like this:

  • Some service is implementing AbstractServiceImpl, let's say TelevisionService
  • The TelevisionService would expose a method like addInputSource(InputSourceService s)
    • this would be implemented as super.addLinkedService(s)

Do you see a problem with this approach?

You can't link arbitrary services, so type-safety imho would make sense.

@yfre
Copy link
Contributor

yfre commented Dec 10, 2020

this is the approach @gjvanderheiden was also proposing - dedicated methods.

my idea was to keep it more flexible as services can be combined in many different ways and HAP does not restrict it. e.g. you can add battery service to lightbulb or switch and home app shows it correctly.
other examples are fan, slat and sensors which can be added to many other services like AirPurifier, Heating and other.
so, we would need to add many methods for almost all combinations.

@dfrommi
Copy link
Contributor

dfrommi commented Dec 10, 2020

Ah, I see.
Didn't know that you can combine arbitrary services. Thanks.

@ccutrer ccutrer closed this as completed Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants