hap-java / HAP-Java

Java implementation of the HomeKit Accessory Protocol
MIT License
152 stars 83 forks source link

Linked Services #104

Closed scothi closed 3 years ago

scothi commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

yfre commented 4 years ago

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

scothi commented 4 years ago

PR sent #105

dfrommi commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

I would like to do this:

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

yfre commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

I actually thought it could used like this:

Do you see a problem with this approach?

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

yfre commented 3 years ago

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 commented 3 years ago

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