hap-java / HAP-Java

Java implementation of the HomeKit Accessory Protocol
MIT License
153 stars 82 forks source link

We can now specify service name #17

Closed gdombiak closed 8 years ago

gdombiak commented 8 years ago

Andy,

Here are 2 small improvements:

  1. It is now possible to specify service name. Still defaults to accessory label
  2. Log IOExceptions using DEBUG level. Before it was only one type of timeout.

I have an accessory that offers 2 services and both services were appearing with the same name (the name of the accessory). I added a new constructor to ServicesImpl so that we can pass a serviceName. Old constructor is still around and defaults to accessory label. Fine-prints: 1) Had to remove the deprecated constructor in AbstractServiceImpl. I think you were ok with this. 2) Changed constructor of Name to accept a String instead of a HomekitAccessory.

Noticed a few more "noisy" entries in the log around IOExceptions so went ahead and change code to log with DEBUG all IOExceptions instead of just "Connection timed out".

Let me know if you are ok with these changes. Thanks, Gaston

gdombiak commented 8 years ago

Added 2 more things to the PR.

1) new Light Sensor. tested & working 2) small touch. Mirrored existing logging when adding sensor to print when removing a sensor.

gdombiak commented 8 years ago

Found a NPE after an accessory was removed from the bridge. This was a silent error since iOS apps are still removing the accessor from the UI.

java.lang.NullPointerException: null at java.util.Collections$UnmodifiableMap.(Collections.java:1446) ~[na:1.8.0_91] at java.util.Collections.unmodifiableMap(Collections.java:1433) ~[na:1.8.0_91] at com.beowulfe.hap.impl.HomekitRegistry.getCharacteristics(HomekitRegistry.java:75) ~[hap-1.1.4-SNAPSHOT.jar!/:na] at com.beowulfe.hap.impl.json.CharacteristicsController.get(CharacteristicsController.java:52) ~[hap-1.1.4-SNAPSHOT.jar!/:na] at com.beowulfe.hap.impl.connections.HttpSession.handleAuthenticatedRequest(HttpSession.java:87) ~[hap-1.1.4-SNAPSHOT.jar!/:na] at com.beowulfe.hap.impl.connections.ConnectionImpl.doHandleRequest(ConnectionImpl.java:51) [hap-1.1.4-SNAPSHOT.jar!/:na] at com.beowulfe.hap.impl.connections.ConnectionImpl.handleRequest(ConnectionImpl.java:46) [hap-1.1.4-SNAPSHOT.jar!/:na] at com.beowulfe.hap.impl.http.impl.AccessoryHandler.channelRead0(AccessoryHandler.java:50) [hap-1.1.4-SNAPSHOT.jar!/:na] at com.beowulfe.hap.impl.http.impl.AccessoryHandler.channelRead0(AccessoryHandler.java:18) [hap-1.1.4-SNAPSHOT.jar!/:na] at io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:105) [netty-all-4.0.32.Final.jar!/:4.0.32.Final] at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:308) [netty-all-4.0.32.Final.jar!/:4.0.32.Final] at io.netty.channel.AbstractChannelHandlerContext.access$600(AbstractChannelHandlerContext.java:32) [netty-all-4.0.32.Final.jar!/:4.0.32.Final] at io.netty.channel.AbstractChannelHandlerContext$7.run(AbstractChannelHandlerContext.java:299) [netty-all-4.0.32.Final.jar!/:4.0.32.Final] at io.netty.util.concurrent.DefaultEventExecutor.run(DefaultEventExecutor.java:36) [netty-all-4.0.32.Final.jar!/:4.0.32.Final] at io.netty.util.concurrent.SingleThreadEventExecutor$2.run(SingleThreadEventExecutor.java:112) [netty-all-4.0.32.Final.jar!/:4.0.32.Final] at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:137) [netty-all-4.0.32.Final.jar!/:4.0.32.Final] at java.lang.Thread.run(Thread.java:745) [na:1.8.0_91]

This PR includes a fix for this. Logs now print something like this instead of the NPE:

2016-08-06 15:32:08.219 WARN 12567 --- [ecutorGroup-4-3] c.b.h.i.json.CharacteristicsController : Accessory 35 has no characteristics or does not exist. Request: /characteristics?id=35.2 2016-08-06 15:32:08.471 WARN 12567 --- [ecutorGroup-4-3] c.b.h.i.json.CharacteristicsController : Accessory 35 has no characteristics or does not exist. Request: /characteristics?id=35.3 2016-08-06 15:32:08.779 WARN 12567 --- [ecutorGroup-4-3] c.b.h.i.json.CharacteristicsController : Accessory 35 has no characteristics or does not exist. Request: /characteristics?id=35.4 2016-08-06 15:32:09.103 WARN 12567 --- [ecutorGroup-4-3] c.b.h.i.json.CharacteristicsController : Accessory 35 has no characteristics or does not exist. Request: /characteristics?id=35.5 2016-08-06 15:32:09.425 WARN 12567 --- [ecutorGroup-4-3] c.b.h.i.json.CharacteristicsController : Accessory 35 has no characteristics or does not exist. Request: /characteristics?id=35.8 2016-08-06 15:32:09.771 WARN 12567 --- [ecutorGroup-4-3] c.b.h.i.json.CharacteristicsController : Accessory 35 has no characteristics or does not exist. Request: /characteristics?id=35.9

andylintner commented 8 years ago

@gdombiak - awesome work! One small change and then I'll merge.

gdombiak commented 8 years ago

Hey Andy,

I added back the deprecated constructor. Things should work fine even when using that constructor.

Let me know if you see anything else.

Thanks, Gaston

andylintner commented 8 years ago

Thanks for another great contribution!