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

Initial contribution of the Homematic binding #438

Merged
merged 1 commit into from
Apr 24, 2016
Merged

Initial contribution of the Homematic binding #438

merged 1 commit into from
Apr 24, 2016

Conversation

gerrieg
Copy link
Contributor

@gerrieg gerrieg commented Oct 18, 2015

Documentation

Open issue:

  • CCU1 autodetection may not work because i can't test it. If someone can send me a startup trace log of the binding connected to a CCU1, i can quickly implement it. Workaround: set gatewayType property to ccu

@kaikreuzer kaikreuzer added the new binding If someone has started to work on a binding. For a new binding PR. label Oct 24, 2015
@kaikreuzer kaikreuzer self-assigned this Oct 24, 2015
@kaikreuzer kaikreuzer added this to the 2.0.0 beta1 milestone Oct 24, 2015
@kaikreuzer
Copy link
Member

Wow, this is a lot of code :-) I hope I will try to review it in November.
To make sure that I do not spend any unnecessary effort: Can you tell me if and which code is taken from the openHAB 1 version of the binding (and thus has already been reviewed)? Or is it a complete rewrite?

@gerrieg
Copy link
Contributor Author

gerrieg commented Oct 25, 2015

Yes, it grew bigger and bigger :-)
The only thing from version 1 is the converter framework (slightly adjusted), everything else is a complete rewrite.
The 1.x binding required TclRega script, so only a CCU is possible as a gateway. This version uses only the Homematic API and only if available for some extra stuff TclRega scripts. Therefore i can support many different gateways like OCCU.
The internal package is completely decoupled from openHab and could in theory be extracted as own library. So future updates for OH3, OH4, ... are much easier.

@kaikreuzer
Copy link
Member

alright, thanks for the infos!

@stefanradike
Copy link

Hi,

I've compiled a current snapshot of openhab/openhab2 and integrated the homematic2 binding from gerrieg/openhab2 homematic branch. HomeMatic device discovery works fine (I am using homegear), but if I want to save a thing, I get a 500 error. More curiously, I cannot create any more groups after that so I assume, the mapdb gets somehow corrupted. I have not (yet) the necessary insight into the codebase, so could you therefore look into the problemto find out if this is a real bug or just me? @gerrieg
EDIT: just found out, that these problems do only occur with Paper UI and not with HABmin2
Here is the exception tree:

2015-11-08 00:24:48 [WARN ] [lipse.jetty.server.HttpChannel:395 ] - /rest/inbox/homematic:HG-HM-LC-Dim1TPBU-FM:a0d0f3e3:JEQ0XXXXXX/approvejavax.servlet.ServletException: javax.servlet.ServletException: org.glassfish.jersey.server.ContainerException: java.io.IOError: java.nio.channels.ClosedChannelException
at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:130)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97)
at org.eclipse.jetty.server.Server.handle(Server.java:499)
at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:310)
at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:257)
at org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:540)
at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:635)
at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:555)
at java.lang.Thread.run(Unknown Source)
Caused by: javax.servlet.ServletException: org.glassfish.jersey.server.ContainerException: java.io.IOError: java.nio.channels.ClosedChannelException
at org.glassfish.jersey.servlet.WebComponent.service(WebComponent.java:423)
at org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:386)
at org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:334)
at org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:221)
at com.eclipsesource.jaxrs.publisher.internal.ServletContainerBridge.service(ServletContainerBridge.java:76)
at org.eclipse.equinox.http.servlet.internal.ServletRegistration.service(ServletRegistration.java:61)
at org.eclipse.equinox.http.servlet.internal.ProxyServlet.processAlias(ProxyServlet.java:128)
at org.eclipse.equinox.http.servlet.internal.ProxyServlet.service(ProxyServlet.java:68)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:808)
at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:587)
Caused by: org.glassfish.jersey.server.ContainerException: java.io.IOError: java.nio.channels.ClosedChannelException
at org.glassfish.jersey.servlet.internal.ResponseWriter.rethrow(ResponseWriter.java:256)
at org.glassfish.jersey.servlet.internal.ResponseWriter.failure(ResponseWriter.java:238)
at org.glassfish.jersey.server.ServerRuntime$Responder.process(ServerRuntime.java:486)
at org.glassfish.jersey.server.ServerRuntime$2.run(ServerRuntime.java:316)
at org.glassfish.jersey.internal.Errors$1.call(Errors.java:271)
at org.glassfish.jersey.internal.Errors$1.call(Errors.java:267)
at org.glassfish.jersey.internal.Errors.process(Errors.java:315)
at org.glassfish.jersey.internal.Errors.process(Errors.java:297)
at org.glassfish.jersey.internal.Errors.process(Errors.java:267)
at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:317)
at org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:291)
Caused by: java.io.IOError: java.nio.channels.ClosedChannelException
at org.mapdb.Volume$FileChannelVol.getLong(Volume.java:818)
at org.mapdb.StoreWAL.update(StoreWAL.java:398)
at org.mapdb.Caches$HashTable.update(Caches.java:254)
at org.mapdb.EngineWrapper.update(EngineWrapper.java:63)
at org.mapdb.BTreeMap.put2(BTreeMap.java:748)
at org.mapdb.BTreeMap.put(BTreeMap.java:645)
at org.eclipse.smarthome.storage.mapdb.MapDbStorage.put(MapDbStorage.java:56)
at org.eclipse.smarthome.core.common.registry.AbstractManagedProvider.add(AbstractManagedProvider.java:60)
at org.eclipse.smarthome.core.common.registry.AbstractRegistry.add(AbstractRegistry.java:110)
at org.eclipse.smarthome.core.thing.setup.ThingSetupManager.addThingSafely(ThingSetupManager.java:648)
at org.eclipse.smarthome.core.thing.setup.ThingSetupManager.addThing(ThingSetupManager.java:609)
Caused by: java.nio.channels.ClosedChannelException: null
at sun.nio.ch.FileChannelImpl.ensureOpen(Unknown Source)
at sun.nio.ch.FileChannelImpl.read(Unknown Source)
at org.mapdb.Volume$FileChannelVol.readFully(Volume.java:787)
at org.mapdb.Volume$FileChannelVol.getLong(Volume.java:815)
at org.mapdb.StoreWAL.update(StoreWAL.java:398)
at org.mapdb.Caches$HashTable.update(Caches.java:254)
at org.mapdb.EngineWrapper.update(EngineWrapper.java:63)
at org.mapdb.BTreeMap.put2(BTreeMap.java:748)
at org.mapdb.BTreeMap.put(BTreeMap.java:645)
at org.eclipse.smarthome.storage.mapdb.MapDbStorage.put(MapDbStorage.java:56)
at org.eclipse.smarthome.core.common.registry.AbstractManagedProvider.add(AbstractManagedProvider.java:60)

@gerrieg
Copy link
Contributor Author

gerrieg commented Nov 8, 2015

Maybe your mapdb files are corrupt. Delete the three files in your openhab_home folder (userdata/mapdb) and try it again please.

@stefanradike
Copy link

I always tried with a fresh installation, wether I compiled the current master and integrated the HomeMatic binding or I compiled your branch. Therefore deleting the userdata folder would probably be only effective as long as I would not add another HomeMatic thing ;-)
But I have found a workaround: After the occurence of this issue I switched to HABmin2 UI and could add all HomeMatic things without any problems. After I while I switched back to PaperUI and it worked again. I guess, it is only a problem with paperUI under certain conditions.

PS: Thanks for getting back to me so quickly. I have worked all weekend on my openHAB2 HM2-binding setup and your work ist just great! 👍

@gerrieg
Copy link
Contributor Author

gerrieg commented Nov 9, 2015

I also think that this is a paperUI issue. Just tried it with a fresh installation and a homegear installation with 30 devices. Can't reproduce the exception.

<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
<classpathentry excluding="org/openhab/binding/homematic/internal/bridge/binrpc/" kind="src" path="src/main/java"/>
<classpathentry kind="src" path="src/main/resources"/>
<classpathentry kind="src" path="src/test/java"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove the two test entries as these paths do not exist.

@kaikreuzer
Copy link
Member

Your refresh example is for one item, i mean refreshing a whole Homematic device or all devices from the Homematic Gateway, not a single state.

I know, but I commented that above already with: "ok to refresh all channels of a thing if a REFRESH is received for any of its channels)" I guess this is what you will in any case do, right?

i have to generate multiple ThingTypes and also must include the firmware in the ThingTypeUID.

No, there is no need to do this! Your handler can simply dynamically set properties on the thing, e.g. in the initialize() method (or whenever you are able to read the firmware version and other static properties).
If the thing is defined in a things-file, this won't be persisted, but as it is reloaded on every startup, that isn't needed either (anyhow, channel states aren't persisted either).

@gerrieg
Copy link
Contributor Author

gerrieg commented Mar 28, 2016

Hi Kai!

I've implemented now Thing parameters, system channels, the refresh command, channel groups and prepared channel and parameter descriptions.

What i want to add now is Homematic IP support for the new CCU firmware. The required XML-RPC communication is already done (not yet commited). I've ordered a HM-IP device and will then implement the rest.

BR, Gerhard

@kaikreuzer
Copy link
Member

Sounds like a productive long Easter weekend, cool 👍
If you think that the current code is fine, I would actually suggest to get it merged soon.
You can still add HM-IP support with a follow-up PR, what do you think?

@gerrieg
Copy link
Contributor Author

gerrieg commented Mar 28, 2016

Yes, one day family, two days openHab 😄

OK, sounds good. So give me two more days for some tests and to update the readme.md file.

I have one problem, but i think it's a PaperUI issue. If i open the Thing parameters, all decimal type parameters are not shown. Metadata with the rest-api looks good for me. So i don't know if its my fault or that of PaperUI.

@kaikreuzer
Copy link
Member

If i open the Thing parameters, all decimal type parameters are not shown.

I just tested this by changing this line from integer to decimal and it all works nicely in the Paper UI.

If you can find a way to reproduce your problem with the Yahoo Weather binding, please enter an issue at https://github.com/eclipse/smarthome/issues

@gerrieg
Copy link
Contributor Author

gerrieg commented Mar 29, 2016

So here is my Homematic contribution for OH2.

With the Thing parameters i need your help, i'm out of ideas what i can do. PaperUI still does not show decimal type parameters. Maybe you can try the binding and give me a hint?

@kaikreuzer kaikreuzer removed the awaiting feedback Awaiting feedback from the pull request author label Mar 29, 2016
@kaikreuzer
Copy link
Member

Thanks! I will try to look at the problem soon. (Please allow me a few days as I am currently on holiday)

@kaikreuzer
Copy link
Member

As I do not have my CCU2 with me on holidays - can you give me a hint on how I can reproduce your problem with the decimal properties? Adding a Homematic thing manually is only possible for the gateway, but this does not have any decimal properties. No other thing types are offered (since they are only created once the gateway connection is established).

@sja
Copy link

sja commented Apr 1, 2016

If you're familar with docker and homegear, you can add a virtual homegear device at homegear. Maybe event the presence of homegear (>= 0.6) us sufficient.

@gerrieg
Copy link
Contributor Author

gerrieg commented Apr 1, 2016

enjoy your holidays and relax! 😎 🌴 🍹

@kaikreuzer
Copy link
Member

Ok, just returned back home and connected to my CCU2.
I actually came across quite a few exceptions and problems when trying to get started - I will need some more time to report them in a structured way; but it leaves me the impression that the current code isn't very robust yet. I assume that a few problems can also be due to ESH itself, since this binding is the first one to do a fully dynamic construction of everything - I e.g. often see placeholders on the label of discovery results and only see the correct label upon a reload.

What worries me more though is the fact that the config properties and channels that I see do not really match my expectations for a user friendly solution - the automatic creation of the thing types might be at its limits here. Let me give you an example: I have a HM-CC-SCD.
This is what I see for its configuration:
bildschirmfoto 2016-04-03 um 21 23 01
All that is meaningful to me is the Aes setting and the transmit retries - and even those should probably be hidden from a regular user. There are no descriptions for the parameters, nor are the values in the selection boxes shown as proper labels (only as keys).
What is imho worse is the list of channels:
bildschirmfoto 2016-04-03 um 21 26 02
There are 7 channels (none of which is hidden as "advanced") and two groups with the name "channel 0" and "channel 1", which does not make much sense.
From a user perspective, all I would expect to see is a channel "CO2" - but this doesn't even exist, instead there is only a "state" channel, which does not say much.

I don't really know how do address these things best, considering the fully automatic creation. But imho we should at least try to hide most configuration parameters as well as channels by marking them as "advanced". The names of the channel groups must definitely be changed as well.
I also would not try to cover all functionality that a CCU provides. E.g. I doubt that it is a use case for openHAB to allow users setting the AES key for devices; this key could therefore also simply be a (read-only) property (or not made available to openHAB at all).

@kaikreuzer kaikreuzer added the awaiting feedback Awaiting feedback from the pull request author label Apr 3, 2016
@gerrieg
Copy link
Contributor Author

gerrieg commented Apr 4, 2016

Exceptions and problems:
I'm very interested in the exceptions and problems. I have 33 devices now and the binding works for me.

Metadata:
I tried to extract as much as possible from the CCU, but descriptions are not available. A Homematic channel has a number and a type, that's all. Therefore, the ChannelGroup has the number and the type is in the description. Why does that not make sense? What should I show instead?

Config and Channel descriptions:
Currently there are only some descriptions for parameters/channels, but the functionality is available. If you look at the end of the descriptions.properties file, i wrote a short doc how to add these descriptions. I will add more over time. Currently i am trying to extract all descriptions from OCCU and from the CCU firmware. Maybe it's possible, otherwise i have to add them all by hand. And that's a lot of work.

Advanced:
Do you only want the most important datapoints per device and all others should be marked as advanced?
If so, i have to dig through the Homematic datapoint documentation and try to identify these datapoints for each device. Maybe i find a logic in the name (e.g LEVEL and STATE are never advanced). OK for you?
For the config parameters it's harder, currently i have no idea for a advanced/not advanced logic.

@kaikreuzer
Copy link
Member

the ChannelGroup has the number and the type is in the description. Why does that not make sense? What should I show instead?

It does not make sense to me because "Homematic channel" and "Thing channel" get mixed up here. "Homematic Channel 0" is then a Thing channel group with x "Thing channels".
As the current label does not really provide any info, I would use the type directly for the label and leave the description empty.

i wrote a short doc how to add these descriptions. I will add more over time.

Ok, cool, makes sense!

Do you only want the most important datapoints per device and all others should be marked as advanced?

Yes, that's the idea of the "advanced" flag. A regular user should not be flooded with options that he hardly ever needs to be aware of.

Maybe i find a logic in the name (e.g LEVEL and STATE are never advanced). OK for you?

Yes, at least for a start. On the long run, it might make sense to have some optional property file within the binding that could hold this information for different names (and maybe also a label to be used, as mentioned above "STATE" is a terribly generic and technical name for the label of the most important channel.

For the config parameters it's harder, currently i have no idea for a advanced/not advanced logic.

So a separate property file could help here as well - I would actually also suggest that it should be possible to completely ignore certain channels/parameters. Take e.g. the wall thermostat: I saw a mass of configuration properties about the scheduling thresholds, which made the configuration screen almost unusable as you had to scroll over many many pages and couldn't find anything anymore.

@kaikreuzer
Copy link
Member

Hi @gerrieg!
Do you see any of the above suggestions as feasible without too much work?
If so, it would be great if you could adapt it and we would be good to merge.

@gerrieg
Copy link
Contributor Author

gerrieg commented Apr 16, 2016

Hi @kaikreuzer !

Yes, the descriptions are already done. I found a way to extract the most descriptions directly from the CCU (see CcuMetadataExtractor). They are spread around many JavaScript files and are concatenated partly. So the most can be extracted and the missing ones can be added in the extra-descriptions.property file over time.

ChannelGroup shows the type now.

I am still working on the advanced flag. I have a concept now and i'm going to implement it in the next days.

@kaikreuzer
Copy link
Member

Sounds great, many thanks!

@gerrieg
Copy link
Contributor Author

gerrieg commented Apr 23, 2016

Hi @kaikreuzer!

I identified the standard datapoints for the most devices and marked all others with the advanced flag. I'm sure, this must be optimized for devices i don't own like a heating thermostat. But that can be done easily in the standard-datapoints.properties file.
For the parameters i didn't find any logic, but if you like i can implement a similar property file.

Descriptions are available for the most datapoints now, some are extracted from the CCU, some are added manually. There are still missing descriptions which should be added over time. At device discovery, missing descriptions are logged.

Homematic IP support is also included now (via CCU2)

I switched the execution environment to JavaSE-1.8, but this is only for the Base64 class included in this jdk. Or is it better to user JavaSE-1.7 and the Base64 class from commons-codec?

@kaikreuzer
Copy link
Member

Thanks a lot, that sounds great!
Switching to Java8 is fine, since the openHAB distro requires Java8 anyhow.

@kaikreuzer kaikreuzer removed the awaiting feedback Awaiting feedback from the pull request author label Apr 23, 2016
org.osgi.framework,
org.osgi.service.component,
org.slf4j
Service-Component: OSGI-INF/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please change this to OSGI-INF/*.xml

Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
@gerrieg
Copy link
Contributor Author

gerrieg commented Apr 24, 2016

done and squashed all commits

@kaikreuzer
Copy link
Member

Thanks again!

@kaikreuzer kaikreuzer merged commit 569b1e5 into openhab:master Apr 24, 2016
doubled-ca pushed a commit to doubled-ca/openhab-addons that referenced this pull request Aug 14, 2016
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants